PCI: qcom: Fix potential deadlock when enabling ASPM
authorJohan Hovold <johan+linaro@kernel.org>
Tue, 28 Nov 2023 08:15:09 +0000 (09:15 +0100)
committerBjorn Helgaas <bhelgaas@google.com>
Mon, 11 Dec 2023 18:08:43 +0000 (12:08 -0600)
The qcom_pcie_enable_aspm() helper is called from pci_walk_bus() during
host init to enable ASPM.

Since pci_walk_bus() already holds a pci_bus_sem read lock, use
pci_enable_link_state_locked() to enable link states in order to avoid a
potential deadlock (e.g. in case someone takes a write lock before
reacquiring the read lock).

This issue was reported by lockdep:

  ============================================
  WARNING: possible recursive locking detected
  6.7.0-rc1 #4 Not tainted
  --------------------------------------------
  kworker/u16:6/147 is trying to acquire lock:
  ffffbf3ff9d2cfa0 (pci_bus_sem){++++}-{3:3}, at: pci_enable_link_state+0x74/0x1e8

  but task is already holding lock:
  ffffbf3ff9d2cfa0 (pci_bus_sem){++++}-{3:3}, at: pci_walk_bus+0x34/0xbc

  other info that might help us debug this:
   Possible unsafe locking scenario:

         CPU0
         ----
    lock(pci_bus_sem);
    lock(pci_bus_sem);

   *** DEADLOCK ***

Fixes: 9f4f3dfad8cf ("PCI: qcom: Enable ASPM for platforms supporting 1.9.0 ops")
Link: https://lore.kernel.org/r/20231128081512.19387-4-johan+linaro@kernel.org
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
[bhelgaas: add "potential" in subject since the deadlock has only been
reported by lockdep, include helper name in commit log]
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
drivers/pci/controller/dwc/pcie-qcom.c

index 6902e97719d1366d754c20f3d28a0482db48500e..13dbdfac98f2fd576b58068b623546fcef88223e 100644 (file)
@@ -970,7 +970,7 @@ static int qcom_pcie_enable_aspm(struct pci_dev *pdev, void *userdata)
 {
        /* Downstream devices need to be in D0 state before enabling PCI PM substates */
        pci_set_power_state(pdev, PCI_D0);
-       pci_enable_link_state(pdev, PCIE_LINK_STATE_ALL);
+       pci_enable_link_state_locked(pdev, PCIE_LINK_STATE_ALL);
 
        return 0;
 }