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: b014e96d1abb ("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>

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 94069fc35e912f2d06cade921581deb40f014175..a3e8edd62a0493e9344f8b8ceb571404c02cdebe 100644 (file)
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5058,39 +5058,42 @@ unlock:
        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;
 
        list_for_each_entry(dev, &bus->devices, bus_list) {
-               pci_dev_lock(dev);
                pci_dev_save_and_disable(dev);
-               pci_dev_unlock(dev);
                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
- * restored before we can get to subordinate devices.
+ * Restore devices from top of the tree down while holding @dev mutex lock
+ * 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;
 
        list_for_each_entry(dev, &bus->devices, bus_list) {
-               pci_dev_lock(dev);
                pci_dev_restore(dev);
-               pci_dev_unlock(dev);
                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;
 
@@ -5099,26 +5102,25 @@ static void pci_slot_save_and_disable(struct pci_slot *slot)
                        continue;
                pci_dev_save_and_disable(dev);
                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
- * restored before we can get to subordinate devices.
+ * Restore devices from top of the tree down while holding @dev mutex lock
+ * 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;
 
        list_for_each_entry(dev, &slot->bus->devices, bus_list) {
                if (!dev->slot || dev->slot != slot)
                        continue;
-               pci_dev_lock(dev);
                pci_dev_restore(dev);
-               pci_dev_unlock(dev);
                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)
                return rc;
 
-       pci_slot_save_and_disable(slot);
-
        if (pci_slot_trylock(slot)) {
+               pci_slot_save_and_disable_locked(slot);
                might_sleep();
                rc = pci_reset_hotplug_slot(slot->hotplug, 0);
+               pci_slot_restore_locked(slot);
                pci_slot_unlock(slot);
        } else
                rc = -EAGAIN;
 
-       pci_slot_restore(slot);
-
        return rc;
 }
 
@@ -5273,17 +5273,15 @@ static int __pci_reset_bus(struct pci_bus *bus)
        if (rc)
                return rc;
 
-       pci_bus_save_and_disable(bus);
-
        if (pci_bus_trylock(bus)) {
+               pci_bus_save_and_disable_locked(bus);
                might_sleep();
                rc = pci_bridge_secondary_bus_reset(bus->self);
+               pci_bus_restore_locked(bus);
                pci_bus_unlock(bus);
        } else
                rc = -EAGAIN;
 
-       pci_bus_restore(bus);
-
        return rc;
 }