diff options
author | Huang Ying <ying.huang@intel.com> | 2012-10-24 08:54:13 +0200 |
---|---|---|
committer | Bjorn Helgaas <bhelgaas@google.com> | 2012-11-02 17:07:17 +0100 |
commit | 90b5c1d7c45eeb622302680ff96ed30c1a2b6f0e (patch) | |
tree | 0c299a4e317c781a2c5ef85227b06ee0b9c6a24e | |
parent | Linux 3.7-rc3 (diff) | |
download | linux-90b5c1d7c45eeb622302680ff96ed30c1a2b6f0e.tar.xz linux-90b5c1d7c45eeb622302680ff96ed30c1a2b6f0e.zip |
PCI/PM: Fix deadlock when unbinding device if parent in D3cold
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 <ying.huang@intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
CC: stable@vger.kernel.org # v3.6+
CC: Zhang Yanmin <yanmin.zhang@intel.com>
-rw-r--r-- | drivers/pci/bus.c | 3 | ||||
-rw-r--r-- | drivers/pci/pcie/aer/aerdrv_core.c | 20 |
2 files changed, 16 insertions, 7 deletions
diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c index 6241fd05bd41..a543746fb354 100644 --- a/drivers/pci/bus.c +++ b/drivers/pci/bus.c @@ -320,10 +320,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 06bad96af415..af4e31cd3a3b 100644 --- a/drivers/pci/pcie/aer/aerdrv_core.c +++ b/drivers/pci/pcie/aer/aerdrv_core.c @@ -213,6 +213,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 || @@ -231,12 +232,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; } @@ -247,14 +250,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; } @@ -265,14 +271,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; } @@ -280,15 +289,18 @@ static int report_resume(struct pci_dev *dev, void *data) { const 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; } |