PCI: Fix "try" semantics of bus and slot reset

The commit referenced below introduced device locking around save and
restore of state for each device during a PCI bus "try" reset, making it
decidely non-"try" and prone to deadlock in the event that a device is
already locked.  Restore __pci_reset_bus() and __pci_reset_slot() to their
advertised locking semantics by pushing the save and restore functions into
the branch where the entire tree is already locked.  Extend the helper
function names with "_locked" and update the comment to reflect this
calling requirement.

Fixes: b014e96d1a ("PCI: Protect pci_error_handlers->reset_notify() usage with device_lock()")
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Sinan Kaya <okaya@kernel.org>
This commit is contained in:
Alex Williamson 2019-02-18 12:46:46 -07:00 committed by Bjorn Helgaas
parent 20a796a969
commit ddefc033ee

View File

@ -5058,39 +5058,42 @@ static int pci_slot_trylock(struct pci_slot *slot)
return 0; return 0;
} }
/* Save and disable devices from the top of the tree down */ /*
static void pci_bus_save_and_disable(struct pci_bus *bus) * Save and disable devices from the top of the tree down while holding
* the @dev mutex lock for the entire tree.
*/
static void pci_bus_save_and_disable_locked(struct pci_bus *bus)
{ {
struct pci_dev *dev; struct pci_dev *dev;
list_for_each_entry(dev, &bus->devices, bus_list) { list_for_each_entry(dev, &bus->devices, bus_list) {
pci_dev_lock(dev);
pci_dev_save_and_disable(dev); pci_dev_save_and_disable(dev);
pci_dev_unlock(dev);
if (dev->subordinate) if (dev->subordinate)
pci_bus_save_and_disable(dev->subordinate); pci_bus_save_and_disable_locked(dev->subordinate);
} }
} }
/* /*
* Restore devices from top of the tree down - parent bridges need to be * Restore devices from top of the tree down while holding @dev mutex lock
* restored before we can get to subordinate devices. * for the entire tree. Parent bridges need to be restored before we can
* get to subordinate devices.
*/ */
static void pci_bus_restore(struct pci_bus *bus) static void pci_bus_restore_locked(struct pci_bus *bus)
{ {
struct pci_dev *dev; struct pci_dev *dev;
list_for_each_entry(dev, &bus->devices, bus_list) { list_for_each_entry(dev, &bus->devices, bus_list) {
pci_dev_lock(dev);
pci_dev_restore(dev); pci_dev_restore(dev);
pci_dev_unlock(dev);
if (dev->subordinate) if (dev->subordinate)
pci_bus_restore(dev->subordinate); pci_bus_restore_locked(dev->subordinate);
} }
} }
/* Save and disable devices from the top of the tree down */ /*
static void pci_slot_save_and_disable(struct pci_slot *slot) * Save and disable devices from the top of the tree down while holding
* the @dev mutex lock for the entire tree.
*/
static void pci_slot_save_and_disable_locked(struct pci_slot *slot)
{ {
struct pci_dev *dev; struct pci_dev *dev;
@ -5099,26 +5102,25 @@ static void pci_slot_save_and_disable(struct pci_slot *slot)
continue; continue;
pci_dev_save_and_disable(dev); pci_dev_save_and_disable(dev);
if (dev->subordinate) if (dev->subordinate)
pci_bus_save_and_disable(dev->subordinate); pci_bus_save_and_disable_locked(dev->subordinate);
} }
} }
/* /*
* Restore devices from top of the tree down - parent bridges need to be * Restore devices from top of the tree down while holding @dev mutex lock
* restored before we can get to subordinate devices. * for the entire tree. Parent bridges need to be restored before we can
* get to subordinate devices.
*/ */
static void pci_slot_restore(struct pci_slot *slot) static void pci_slot_restore_locked(struct pci_slot *slot)
{ {
struct pci_dev *dev; struct pci_dev *dev;
list_for_each_entry(dev, &slot->bus->devices, bus_list) { list_for_each_entry(dev, &slot->bus->devices, bus_list) {
if (!dev->slot || dev->slot != slot) if (!dev->slot || dev->slot != slot)
continue; continue;
pci_dev_lock(dev);
pci_dev_restore(dev); pci_dev_restore(dev);
pci_dev_unlock(dev);
if (dev->subordinate) if (dev->subordinate)
pci_bus_restore(dev->subordinate); pci_bus_restore_locked(dev->subordinate);
} }
} }
@ -5177,17 +5179,15 @@ static int __pci_reset_slot(struct pci_slot *slot)
if (rc) if (rc)
return rc; return rc;
pci_slot_save_and_disable(slot);
if (pci_slot_trylock(slot)) { if (pci_slot_trylock(slot)) {
pci_slot_save_and_disable_locked(slot);
might_sleep(); might_sleep();
rc = pci_reset_hotplug_slot(slot->hotplug, 0); rc = pci_reset_hotplug_slot(slot->hotplug, 0);
pci_slot_restore_locked(slot);
pci_slot_unlock(slot); pci_slot_unlock(slot);
} else } else
rc = -EAGAIN; rc = -EAGAIN;
pci_slot_restore(slot);
return rc; return rc;
} }
@ -5273,17 +5273,15 @@ static int __pci_reset_bus(struct pci_bus *bus)
if (rc) if (rc)
return rc; return rc;
pci_bus_save_and_disable(bus);
if (pci_bus_trylock(bus)) { if (pci_bus_trylock(bus)) {
pci_bus_save_and_disable_locked(bus);
might_sleep(); might_sleep();
rc = pci_bridge_secondary_bus_reset(bus->self); rc = pci_bridge_secondary_bus_reset(bus->self);
pci_bus_restore_locked(bus);
pci_bus_unlock(bus); pci_bus_unlock(bus);
} else } else
rc = -EAGAIN; rc = -EAGAIN;
pci_bus_restore(bus);
return rc; return rc;
} }