From: Huang Ying Date: Wed, 24 Oct 2012 06:54:13 +0000 (+0800) Subject: PCI/PM: Fix deadlock when unbinding device if parent in D3cold X-Git-Tag: v3.6.11~18 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=50439c4394f0731dca76ef29916ce36abc093ad9;p=thirdparty%2Fkernel%2Fstable.git PCI/PM: Fix deadlock when unbinding device if parent in D3cold commit 90b5c1d7c45eeb622302680ff96ed30c1a2b6f0e upstream. If a PCI device and its parents are put into D3cold, unbinding the device will trigger deadlock as follow: - driver_unbind - device_release_driver - device_lock(dev) <--- previous lock here - __device_release_driver - pm_runtime_get_sync ... - rpm_resume(dev) - rpm_resume(dev->parent) ... - pci_pm_runtime_resume ... - pci_set_power_state - __pci_start_power_transition - pci_wakeup_bus(dev->parent->subordinate) - pci_walk_bus - device_lock(dev) <--- deadlock here If we do not do device_lock in pci_walk_bus, we can avoid deadlock. Device_lock in pci_walk_bus is introduced in commit: d71374dafbba7ec3f67371d3b7e9f6310a588808, corresponding email thread is: https://lkml.org/lkml/2006/5/26/38. The patch author Zhang Yanmin said device_lock is added to pci_walk_bus because: Some error handling functions call pci_walk_bus. For example, PCIe aer. Here we lock the device, so the driver wouldn't detach from the device, as the cb might call driver's callback function. So I fixed the deadlock as follows: - remove device_lock from pci_walk_bus - add device_lock into callback if callback will call driver's callback I checked pci_walk_bus users one by one, and found only PCIe aer needs device lock. Signed-off-by: Huang Ying Signed-off-by: Bjorn Helgaas Acked-by: Rafael J. Wysocki CC: stable@vger.kernel.org # v3.6+ CC: Zhang Yanmin --- diff --git a/arch/powerpc/platforms/pseries/eeh_driver.c b/arch/powerpc/platforms/pseries/eeh_driver.c index 041e28d0e702c..1f12c241cc252 100644 --- a/arch/powerpc/platforms/pseries/eeh_driver.c +++ b/arch/powerpc/platforms/pseries/eeh_driver.c @@ -164,17 +164,18 @@ static int eeh_report_error(struct pci_dev *dev, void *userdata) enum pci_ers_result rc, *res = userdata; struct pci_driver *driver; + device_lock(&dev->dev); dev->error_state = pci_channel_io_frozen; driver = eeh_pcid_get(dev); - if (!driver) return 0; + if (!driver) goto out; eeh_disable_irq(dev); if (!driver->err_handler || !driver->err_handler->error_detected) { eeh_pcid_put(dev); - return 0; + goto out; } rc = driver->err_handler->error_detected(dev, pci_channel_io_frozen); @@ -184,6 +185,8 @@ static int eeh_report_error(struct pci_dev *dev, void *userdata) if (*res == PCI_ERS_RESULT_NONE) *res = rc; eeh_pcid_put(dev); +out: + device_unlock(&dev->dev); return 0; } @@ -201,13 +204,14 @@ static int eeh_report_mmio_enabled(struct pci_dev *dev, void *userdata) enum pci_ers_result rc, *res = userdata; struct pci_driver *driver; + device_lock(&dev->dev); driver = eeh_pcid_get(dev); - if (!driver) return 0; + if (!driver) goto out; if (!driver->err_handler || !driver->err_handler->mmio_enabled) { eeh_pcid_put(dev); - return 0; + goto out; } rc = driver->err_handler->mmio_enabled(dev); @@ -217,6 +221,8 @@ static int eeh_report_mmio_enabled(struct pci_dev *dev, void *userdata) if (*res == PCI_ERS_RESULT_NONE) *res = rc; eeh_pcid_put(dev); +out: + device_unlock(&dev->dev); return 0; } @@ -235,17 +241,18 @@ static int eeh_report_reset(struct pci_dev *dev, void *userdata) enum pci_ers_result rc, *res = userdata; struct pci_driver *driver; + device_lock(&dev->dev); dev->error_state = pci_channel_io_normal; driver = eeh_pcid_get(dev); - if (!driver) return 0; + if (!driver) goto out; eeh_enable_irq(dev); if (!driver->err_handler || !driver->err_handler->slot_reset) { eeh_pcid_put(dev); - return 0; + goto out; } rc = driver->err_handler->slot_reset(dev); @@ -255,6 +262,8 @@ static int eeh_report_reset(struct pci_dev *dev, void *userdata) rc == PCI_ERS_RESULT_NEED_RESET) *res = rc; eeh_pcid_put(dev); +out: + device_unlock(&dev->dev); return 0; } @@ -271,22 +280,25 @@ static int eeh_report_resume(struct pci_dev *dev, void *userdata) { struct pci_driver *driver; + device_lock(&dev->dev); dev->error_state = pci_channel_io_normal; driver = eeh_pcid_get(dev); - if (!driver) return 0; + if (!driver) goto out; eeh_enable_irq(dev); if (!driver->err_handler || !driver->err_handler->resume) { eeh_pcid_put(dev); - return 0; + goto out; } driver->err_handler->resume(dev); eeh_pcid_put(dev); +out: + device_unlock(&dev->dev); return 0; } @@ -302,22 +314,25 @@ static int eeh_report_failure(struct pci_dev *dev, void *userdata) { struct pci_driver *driver; + device_lock(&dev->dev); dev->error_state = pci_channel_io_perm_failure; driver = eeh_pcid_get(dev); - if (!driver) return 0; + if (!driver) goto out; eeh_disable_irq(dev); if (!driver->err_handler || !driver->err_handler->error_detected) { eeh_pcid_put(dev); - return 0; + goto out; } driver->err_handler->error_detected(dev, pci_channel_io_perm_failure); eeh_pcid_put(dev); +out: + device_unlock(&dev->dev); return 0; } diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c index 4b0970b46e0b8..e7b5fd2b6ec85 100644 --- a/drivers/pci/bus.c +++ b/drivers/pci/bus.c @@ -316,10 +316,7 @@ void pci_walk_bus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *), } else next = dev->bus_list.next; - /* Run device routines with the device locked */ - device_lock(&dev->dev); retval = cb(dev, userdata); - device_unlock(&dev->dev); if (retval) break; } diff --git a/drivers/pci/pcie/aer/aerdrv_core.c b/drivers/pci/pcie/aer/aerdrv_core.c index 0ca053538146b..1b7d05de0209d 100644 --- a/drivers/pci/pcie/aer/aerdrv_core.c +++ b/drivers/pci/pcie/aer/aerdrv_core.c @@ -244,6 +244,7 @@ static int report_error_detected(struct pci_dev *dev, void *data) struct aer_broadcast_data *result_data; result_data = (struct aer_broadcast_data *) data; + device_lock(&dev->dev); dev->error_state = result_data->state; if (!dev->driver || @@ -262,12 +263,14 @@ static int report_error_detected(struct pci_dev *dev, void *data) dev->driver ? "no AER-aware driver" : "no driver"); } - return 0; + goto out; } err_handler = dev->driver->err_handler; vote = err_handler->error_detected(dev, result_data->state); result_data->result = merge_result(result_data->result, vote); +out: + device_unlock(&dev->dev); return 0; } @@ -278,14 +281,17 @@ static int report_mmio_enabled(struct pci_dev *dev, void *data) struct aer_broadcast_data *result_data; result_data = (struct aer_broadcast_data *) data; + device_lock(&dev->dev); if (!dev->driver || !dev->driver->err_handler || !dev->driver->err_handler->mmio_enabled) - return 0; + goto out; err_handler = dev->driver->err_handler; vote = err_handler->mmio_enabled(dev); result_data->result = merge_result(result_data->result, vote); +out: + device_unlock(&dev->dev); return 0; } @@ -296,14 +302,17 @@ static int report_slot_reset(struct pci_dev *dev, void *data) struct aer_broadcast_data *result_data; result_data = (struct aer_broadcast_data *) data; + device_lock(&dev->dev); if (!dev->driver || !dev->driver->err_handler || !dev->driver->err_handler->slot_reset) - return 0; + goto out; err_handler = dev->driver->err_handler; vote = err_handler->slot_reset(dev); result_data->result = merge_result(result_data->result, vote); +out: + device_unlock(&dev->dev); return 0; } @@ -311,15 +320,18 @@ static int report_resume(struct pci_dev *dev, void *data) { struct pci_error_handlers *err_handler; + device_lock(&dev->dev); dev->error_state = pci_channel_io_normal; if (!dev->driver || !dev->driver->err_handler || !dev->driver->err_handler->resume) - return 0; + goto out; err_handler = dev->driver->err_handler; err_handler->resume(dev); +out: + device_unlock(&dev->dev); return 0; }