openpower/linux/0002-powerpc-eeh-Fix-race-with-driver-un-bind.patch - premsjha/op-build - Gitiles

 From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
 From: Michael Neuling <mikey@neuling.org>
 Date: Mon, 26 Mar 2018 15:17:07 +1100
 Subject: [PATCH 2/4] powerpc/eeh: Fix race with driver un/bind

 The current EEH callbacks can race with a driver unbind. This can
 result in a backtraces like this:

   EEH: Frozen PHB#0-PE#1fc detected
   EEH: PE location: S000009, PHB location: N/A
   CPU: 2 PID: 2312 Comm: kworker/u258:3 Not tainted 4.15.6-openpower1 #2
   Workqueue: nvme-wq nvme_reset_work [nvme]
   Call Trace:
     dump_stack+0x9c/0xd0 (unreliable)
     eeh_dev_check_failure+0x420/0x470
     eeh_check_failure+0xa0/0xa4
     nvme_reset_work+0x138/0x1414 [nvme]
     process_one_work+0x1ec/0x328
     worker_thread+0x2e4/0x3a8
     kthread+0x14c/0x154
     ret_from_kernel_thread+0x5c/0xc8
   nvme nvme1: Removing after probe failure status: -19
   <snip>
   cpu 0x23: Vector: 300 (Data Access) at [c000000ff50f3800]
       pc: c0080000089a0eb0: nvme_error_detected+0x4c/0x90 [nvme]
       lr: c000000000026564: eeh_report_error+0xe0/0x110
       sp: c000000ff50f3a80
      msr: 9000000000009033
      dar: 400
    dsisr: 40000000
     current = 0xc000000ff507c000
     paca    = 0xc00000000fdc9d80   softe: 0        irq_happened: 0x01
       pid   = 782, comm = eehd
   Linux version 4.15.6-openpower1 (smc@smc-desktop) (gcc version 6.4.0 (Buildroot 2017.11.2-00008-g4b6188e)) #2 SM                                             P Tue Feb 27 12:33:27 PST 2018
   enter ? for help
     eeh_report_error+0xe0/0x110
     eeh_pe_dev_traverse+0xc0/0xdc
     eeh_handle_normal_event+0x184/0x4c4
     eeh_handle_event+0x30/0x288
     eeh_event_handler+0x124/0x170
     kthread+0x14c/0x154
     ret_from_kernel_thread+0x5c/0xc8

 The first part is an EEH (on boot), the second half is the resulting
 crash. nvme probe starts the nvme_reset_work() worker thread. This
 worker thread starts touching the device which see a device error
 (EEH) and hence queues up an event in the powerpc EEH worker
 thread. nvme_reset_work() then continues and runs
 nvme_remove_dead_ctrl_work() which results in unbinding the driver
 from the device and hence releases all resources. At the same time,
 the EEH worker thread starts doing the EEH .error_detected() driver
 callback, which no longer works since the resources have been freed.

 This fixes the problem in the same way the generic PCIe AER code (in
 drivers/pci/pcie/aer/aerdrv_core.c) does. It makes the EEH code hold
 the device_lock() while performing the driver EEH callbacks and
 associated code. This ensures either the callbacks are no longer
 register, or if they are registered the driver will not be removed
 from underneath us.

 This has been broken forever. The EEH call backs were first introduced
 in 2005 (in 77bd7415610) but it's not clear if a lock was needed back
 then.

 Fixes: 77bd74156101 ("[PATCH] powerpc: PCI Error Recovery: PPC64 core recovery routines")
 Cc: stable@vger.kernel.org # v2.6.16+
 Signed-off-by: Michael Neuling <mikey@neuling.org>
 Reviewed-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
 Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
 (cherry picked from commit f0295e047fcf52ccb42561fb7de6942f5201b676)
 Signed-off-by: Joel Stanley <joel@jms.id.au>
 ---
  arch/powerpc/kernel/eeh_driver.c | 68 ++++++++++++++++++++------------
  1 file changed, 42 insertions(+), 26 deletions(-)

 diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
 index 0c0b66fc5bfb..295ba833846e 100644
 --- a/arch/powerpc/kernel/eeh_driver.c
 +++ b/arch/powerpc/kernel/eeh_driver.c
 @@ -207,18 +207,18 @@ static void *eeh_report_error(void *data, void *userdata)

  	if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
  		return NULL;
 +
 +	device_lock(&dev->dev);
  	dev->error_state = pci_channel_io_frozen;

  	driver = eeh_pcid_get(dev);
 -	if (!driver) return NULL;
 +	if (!driver) goto out_no_dev;

  	eeh_disable_irq(dev);

  	if (!driver->err_handler ||
 -	    !driver->err_handler->error_detected) {
 -		eeh_pcid_put(dev);
 -		return NULL;
 -	}
 +	    !driver->err_handler->error_detected)
 +		goto out;

  	rc = driver->err_handler->error_detected(dev, pci_channel_io_frozen);

 @@ -227,8 +227,12 @@ static void *eeh_report_error(void *data, void *userdata)
  	if (*res == PCI_ERS_RESULT_NONE) *res = rc;

  	edev->in_error = true;
 -	eeh_pcid_put(dev);
  	pci_uevent_ers(dev, PCI_ERS_RESULT_NONE);
 +
 +out:
 +	eeh_pcid_put(dev);
 +out_no_dev:
 +	device_unlock(&dev->dev);
  	return NULL;
  }

 @@ -251,15 +255,14 @@ static void *eeh_report_mmio_enabled(void *data, void *userdata)
  	if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
  		return NULL;

 +	device_lock(&dev->dev);
  	driver = eeh_pcid_get(dev);
 -	if (!driver) return NULL;
 +	if (!driver) goto out_no_dev;

  	if (!driver->err_handler ||
  	    !driver->err_handler->mmio_enabled ||
 -	    (edev->mode & EEH_DEV_NO_HANDLER)) {
 -		eeh_pcid_put(dev);
 -		return NULL;
 -	}
 +	    (edev->mode & EEH_DEV_NO_HANDLER))
 +		goto out;

  	rc = driver->err_handler->mmio_enabled(dev);

 @@ -267,7 +270,10 @@ static void *eeh_report_mmio_enabled(void *data, void *userdata)
  	if (rc == PCI_ERS_RESULT_NEED_RESET) *res = rc;
  	if (*res == PCI_ERS_RESULT_NONE) *res = rc;

 +out:
  	eeh_pcid_put(dev);
 +out_no_dev:
 +	device_unlock(&dev->dev);
  	return NULL;
  }

 @@ -290,20 +296,20 @@ static void *eeh_report_reset(void *data, void *userdata)

  	if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
  		return NULL;
 +
 +	device_lock(&dev->dev);
  	dev->error_state = pci_channel_io_normal;

  	driver = eeh_pcid_get(dev);
 -	if (!driver) return NULL;
 +	if (!driver) goto out_no_dev;

  	eeh_enable_irq(dev);

  	if (!driver->err_handler ||
  	    !driver->err_handler->slot_reset ||
  	    (edev->mode & EEH_DEV_NO_HANDLER) ||
 -	    (!edev->in_error)) {
 -		eeh_pcid_put(dev);
 -		return NULL;
 -	}
 +	    (!edev->in_error))
 +		goto out;

  	rc = driver->err_handler->slot_reset(dev);
  	if ((*res == PCI_ERS_RESULT_NONE) ||
 @@ -311,7 +317,10 @@ static void *eeh_report_reset(void *data, void *userdata)
  	if (*res == PCI_ERS_RESULT_DISCONNECT &&
  	     rc == PCI_ERS_RESULT_NEED_RESET) *res = rc;

 +out:
  	eeh_pcid_put(dev);
 +out_no_dev:
 +	device_unlock(&dev->dev);
  	return NULL;
  }

 @@ -362,10 +371,12 @@ static void *eeh_report_resume(void *data, void *userdata)

  	if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
  		return NULL;
 +
 +	device_lock(&dev->dev);
  	dev->error_state = pci_channel_io_normal;

  	driver = eeh_pcid_get(dev);
 -	if (!driver) return NULL;
 +	if (!driver) goto out_no_dev;

  	was_in_error = edev->in_error;
  	edev->in_error = false;
 @@ -375,18 +386,20 @@ static void *eeh_report_resume(void *data, void *userdata)
  	    !driver->err_handler->resume ||
  	    (edev->mode & EEH_DEV_NO_HANDLER) || !was_in_error) {
  		edev->mode &= ~EEH_DEV_NO_HANDLER;
 -		eeh_pcid_put(dev);
 -		return NULL;
 +		goto out;
  	}

  	driver->err_handler->resume(dev);

 -	eeh_pcid_put(dev);
  	pci_uevent_ers(dev, PCI_ERS_RESULT_RECOVERED);
 +out:
 +	eeh_pcid_put(dev);
  #ifdef CONFIG_PCI_IOV
  	if (eeh_ops->notify_resume && eeh_dev_to_pdn(edev))
  		eeh_ops->notify_resume(eeh_dev_to_pdn(edev));
  #endif
 +out_no_dev:
 +	device_unlock(&dev->dev);
  	return NULL;
  }

 @@ -406,23 +419,26 @@ static void *eeh_report_failure(void *data, void *userdata)

  	if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
  		return NULL;
 +
 +	device_lock(&dev->dev);
  	dev->error_state = pci_channel_io_perm_failure;

  	driver = eeh_pcid_get(dev);
 -	if (!driver) return NULL;
 +	if (!driver) goto out_no_dev;

  	eeh_disable_irq(dev);

  	if (!driver->err_handler ||
 -	    !driver->err_handler->error_detected) {
 -		eeh_pcid_put(dev);
 -		return NULL;
 -	}
 +	    !driver->err_handler->error_detected)
 +		goto out;

  	driver->err_handler->error_detected(dev, pci_channel_io_perm_failure);

 -	eeh_pcid_put(dev);
  	pci_uevent_ers(dev, PCI_ERS_RESULT_DISCONNECT);
 +out:
 +	eeh_pcid_put(dev);
 +out_no_dev:
 +	device_unlock(&dev->dev);
  	return NULL;
  }
	From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
	From: Michael Neuling <mikey@neuling.org>
	Date: Mon, 26 Mar 2018 15:17:07 +1100
	Subject: [PATCH 2/4] powerpc/eeh: Fix race with driver un/bind

	The current EEH callbacks can race with a driver unbind. This can
	result in a backtraces like this:

	EEH: Frozen PHB#0-PE#1fc detected
	EEH: PE location: S000009, PHB location: N/A
	CPU: 2 PID: 2312 Comm: kworker/u258:3 Not tainted 4.15.6-openpower1 #2
	Workqueue: nvme-wq nvme_reset_work [nvme]
	Call Trace:
	dump_stack+0x9c/0xd0 (unreliable)
	eeh_dev_check_failure+0x420/0x470
	eeh_check_failure+0xa0/0xa4
	nvme_reset_work+0x138/0x1414 [nvme]
	process_one_work+0x1ec/0x328
	worker_thread+0x2e4/0x3a8
	kthread+0x14c/0x154
	ret_from_kernel_thread+0x5c/0xc8
	nvme nvme1: Removing after probe failure status: -19
	<snip>
	cpu 0x23: Vector: 300 (Data Access) at [c000000ff50f3800]
	pc: c0080000089a0eb0: nvme_error_detected+0x4c/0x90 [nvme]
	lr: c000000000026564: eeh_report_error+0xe0/0x110
	sp: c000000ff50f3a80
	msr: 9000000000009033
	dar: 400
	dsisr: 40000000
	current = 0xc000000ff507c000
	paca = 0xc00000000fdc9d80 softe: 0 irq_happened: 0x01
	pid = 782, comm = eehd
	Linux version 4.15.6-openpower1 (smc@smc-desktop) (gcc version 6.4.0 (Buildroot 2017.11.2-00008-g4b6188e)) #2 SM P Tue Feb 27 12:33:27 PST 2018
	enter ? for help
	eeh_report_error+0xe0/0x110
	eeh_pe_dev_traverse+0xc0/0xdc
	eeh_handle_normal_event+0x184/0x4c4
	eeh_handle_event+0x30/0x288
	eeh_event_handler+0x124/0x170
	kthread+0x14c/0x154
	ret_from_kernel_thread+0x5c/0xc8

	The first part is an EEH (on boot), the second half is the resulting
	crash. nvme probe starts the nvme_reset_work() worker thread. This
	worker thread starts touching the device which see a device error
	(EEH) and hence queues up an event in the powerpc EEH worker
	thread. nvme_reset_work() then continues and runs
	nvme_remove_dead_ctrl_work() which results in unbinding the driver
	from the device and hence releases all resources. At the same time,
	the EEH worker thread starts doing the EEH .error_detected() driver
	callback, which no longer works since the resources have been freed.

	This fixes the problem in the same way the generic PCIe AER code (in
	drivers/pci/pcie/aer/aerdrv_core.c) does. It makes the EEH code hold
	the device_lock() while performing the driver EEH callbacks and
	associated code. This ensures either the callbacks are no longer
	register, or if they are registered the driver will not be removed
	from underneath us.

	This has been broken forever. The EEH call backs were first introduced
	in 2005 (in 77bd7415610) but it's not clear if a lock was needed back
	then.

	Fixes: 77bd74156101 ("[PATCH] powerpc: PCI Error Recovery: PPC64 core recovery routines")
	Cc: stable@vger.kernel.org # v2.6.16+
	Signed-off-by: Michael Neuling <mikey@neuling.org>
	Reviewed-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
	Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
	(cherry picked from commit f0295e047fcf52ccb42561fb7de6942f5201b676)
	Signed-off-by: Joel Stanley <joel@jms.id.au>
	---
	arch/powerpc/kernel/eeh_driver.c \| 68 ++++++++++++++++++++------------
	1 file changed, 42 insertions(+), 26 deletions(-)

	diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
	index 0c0b66fc5bfb..295ba833846e 100644
	--- a/arch/powerpc/kernel/eeh_driver.c
	+++ b/arch/powerpc/kernel/eeh_driver.c
	@@ -207,18 +207,18 @@ static void eeh_report_error(void data, void *userdata)

	if (!dev \|\| eeh_dev_removed(edev) \|\| eeh_pe_passed(edev->pe))
	return NULL;
	+
	+ device_lock(&dev->dev);
	dev->error_state = pci_channel_io_frozen;

	driver = eeh_pcid_get(dev);
	- if (!driver) return NULL;
	+ if (!driver) goto out_no_dev;

	eeh_disable_irq(dev);

	if (!driver->err_handler \|\|
	- !driver->err_handler->error_detected) {
	- eeh_pcid_put(dev);
	- return NULL;
	- }
	+ !driver->err_handler->error_detected)
	+ goto out;

	rc = driver->err_handler->error_detected(dev, pci_channel_io_frozen);

	@@ -227,8 +227,12 @@ static void eeh_report_error(void data, void *userdata)
	if (res == PCI_ERS_RESULT_NONE) res = rc;

	edev->in_error = true;
	- eeh_pcid_put(dev);
	pci_uevent_ers(dev, PCI_ERS_RESULT_NONE);
	+
	+out:
	+ eeh_pcid_put(dev);
	+out_no_dev:
	+ device_unlock(&dev->dev);
	return NULL;
	}

	@@ -251,15 +255,14 @@ static void eeh_report_mmio_enabled(void data, void *userdata)
	if (!dev \|\| eeh_dev_removed(edev) \|\| eeh_pe_passed(edev->pe))
	return NULL;

	+ device_lock(&dev->dev);
	driver = eeh_pcid_get(dev);
	- if (!driver) return NULL;
	+ if (!driver) goto out_no_dev;

	if (!driver->err_handler \|\|
	!driver->err_handler->mmio_enabled \|\|
	- (edev->mode & EEH_DEV_NO_HANDLER)) {
	- eeh_pcid_put(dev);
	- return NULL;
	- }
	+ (edev->mode & EEH_DEV_NO_HANDLER))
	+ goto out;

	rc = driver->err_handler->mmio_enabled(dev);

	@@ -267,7 +270,10 @@ static void eeh_report_mmio_enabled(void data, void *userdata)
	if (rc == PCI_ERS_RESULT_NEED_RESET) *res = rc;
	if (res == PCI_ERS_RESULT_NONE) res = rc;

	+out:
	eeh_pcid_put(dev);
	+out_no_dev:
	+ device_unlock(&dev->dev);
	return NULL;
	}

	@@ -290,20 +296,20 @@ static void eeh_report_reset(void data, void *userdata)

	if (!dev \|\| eeh_dev_removed(edev) \|\| eeh_pe_passed(edev->pe))
	return NULL;
	+
	+ device_lock(&dev->dev);
	dev->error_state = pci_channel_io_normal;

	driver = eeh_pcid_get(dev);
	- if (!driver) return NULL;
	+ if (!driver) goto out_no_dev;

	eeh_enable_irq(dev);

	if (!driver->err_handler \|\|
	!driver->err_handler->slot_reset \|\|
	(edev->mode & EEH_DEV_NO_HANDLER) \|\|
	- (!edev->in_error)) {
	- eeh_pcid_put(dev);
	- return NULL;
	- }
	+ (!edev->in_error))
	+ goto out;

	rc = driver->err_handler->slot_reset(dev);
	if ((*res == PCI_ERS_RESULT_NONE) \|\|
	@@ -311,7 +317,10 @@ static void eeh_report_reset(void data, void *userdata)
	if (*res == PCI_ERS_RESULT_DISCONNECT &&
	rc == PCI_ERS_RESULT_NEED_RESET) *res = rc;

	+out:
	eeh_pcid_put(dev);
	+out_no_dev:
	+ device_unlock(&dev->dev);
	return NULL;
	}

	@@ -362,10 +371,12 @@ static void eeh_report_resume(void data, void *userdata)

	if (!dev \|\| eeh_dev_removed(edev) \|\| eeh_pe_passed(edev->pe))
	return NULL;
	+
	+ device_lock(&dev->dev);
	dev->error_state = pci_channel_io_normal;

	driver = eeh_pcid_get(dev);
	- if (!driver) return NULL;
	+ if (!driver) goto out_no_dev;

	was_in_error = edev->in_error;
	edev->in_error = false;
	@@ -375,18 +386,20 @@ static void eeh_report_resume(void data, void *userdata)
	!driver->err_handler->resume \|\|
	(edev->mode & EEH_DEV_NO_HANDLER) \|\| !was_in_error) {
	edev->mode &= ~EEH_DEV_NO_HANDLER;
	- eeh_pcid_put(dev);
	- return NULL;
	+ goto out;
	}

	driver->err_handler->resume(dev);

	- eeh_pcid_put(dev);
	pci_uevent_ers(dev, PCI_ERS_RESULT_RECOVERED);
	+out:
	+ eeh_pcid_put(dev);
	#ifdef CONFIG_PCI_IOV
	if (eeh_ops->notify_resume && eeh_dev_to_pdn(edev))
	eeh_ops->notify_resume(eeh_dev_to_pdn(edev));
	#endif
	+out_no_dev:
	+ device_unlock(&dev->dev);
	return NULL;
	}

	@@ -406,23 +419,26 @@ static void eeh_report_failure(void data, void *userdata)

	if (!dev \|\| eeh_dev_removed(edev) \|\| eeh_pe_passed(edev->pe))
	return NULL;
	+
	+ device_lock(&dev->dev);
	dev->error_state = pci_channel_io_perm_failure;

	driver = eeh_pcid_get(dev);
	- if (!driver) return NULL;
	+ if (!driver) goto out_no_dev;

	eeh_disable_irq(dev);

	if (!driver->err_handler \|\|
	- !driver->err_handler->error_detected) {
	- eeh_pcid_put(dev);
	- return NULL;
	- }
	+ !driver->err_handler->error_detected)
	+ goto out;

	driver->err_handler->error_detected(dev, pci_channel_io_perm_failure);

	- eeh_pcid_put(dev);
	pci_uevent_ers(dev, PCI_ERS_RESULT_DISCONNECT);
	+out:
	+ eeh_pcid_put(dev);
	+out_no_dev:
	+ device_unlock(&dev->dev);
	return NULL;
	}