From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Dan Williams <dan.j.williams@intel.com>,
Imre Deak <imre.deak@intel.com>, Keith Busch <kbusch@kernel.org>,
Bjorn Helgaas <bhelgaas@google.com>,
Hans de Goede <hdegoede@redhat.com>,
Kalle Valo <kvalo@kernel.org>, Dave Jiang <dave.jiang@intel.com>,
Sasha Levin <sashal@kernel.org>,
linux-pci@vger.kernel.org
Subject: [PATCH AUTOSEL 5.15 12/13] PCI: Add missing bridge lock to pci_bus_lock()
Date: Sun, 28 Jul 2024 12:08:54 -0400 [thread overview]
Message-ID: <20240728160907.2053634-12-sashal@kernel.org> (raw)
In-Reply-To: <20240728160907.2053634-1-sashal@kernel.org>
From: Dan Williams <dan.j.williams@intel.com>
[ Upstream commit a4e772898f8bf2e7e1cf661a12c60a5612c4afab ]
One of the true positives that the cfg_access_lock lockdep effort
identified is this sequence:
WARNING: CPU: 14 PID: 1 at drivers/pci/pci.c:4886 pci_bridge_secondary_bus_reset+0x5d/0x70
RIP: 0010:pci_bridge_secondary_bus_reset+0x5d/0x70
Call Trace:
<TASK>
? __warn+0x8c/0x190
? pci_bridge_secondary_bus_reset+0x5d/0x70
? report_bug+0x1f8/0x200
? handle_bug+0x3c/0x70
? exc_invalid_op+0x18/0x70
? asm_exc_invalid_op+0x1a/0x20
? pci_bridge_secondary_bus_reset+0x5d/0x70
pci_reset_bus+0x1d8/0x270
vmd_probe+0x778/0xa10
pci_device_probe+0x95/0x120
Where pci_reset_bus() users are triggering unlocked secondary bus resets.
Ironically pci_bus_reset(), several calls down from pci_reset_bus(), uses
pci_bus_lock() before issuing the reset which locks everything *but* the
bridge itself.
For the same motivation as adding:
bridge = pci_upstream_bridge(dev);
if (bridge)
pci_dev_lock(bridge);
to pci_reset_function() for the "bus" and "cxl_bus" reset cases, add
pci_dev_lock() for @bus->self to pci_bus_lock().
Link: https://lore.kernel.org/r/171711747501.1628941.15217746952476635316.stgit@dwillia2-xfh.jf.intel.com
Reported-by: Imre Deak <imre.deak@intel.com>
Closes: http://lore.kernel.org/r/6657833b3b5ae_14984b29437@dwillia2-xfh.jf.intel.com.notmuch
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
[bhelgaas: squash in recursive locking deadlock fix from Keith Busch:
https://lore.kernel.org/r/20240711193650.701834-1-kbusch@meta.com]
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Tested-by: Hans de Goede <hdegoede@redhat.com>
Tested-by: Kalle Valo <kvalo@kernel.org>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/pci/pci.c | 35 +++++++++++++++++++++--------------
1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 67216f4ea2151..84c1300afb951 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5491,10 +5491,12 @@ static void pci_bus_lock(struct pci_bus *bus)
{
struct pci_dev *dev;
+ pci_dev_lock(bus->self);
list_for_each_entry(dev, &bus->devices, bus_list) {
- pci_dev_lock(dev);
if (dev->subordinate)
pci_bus_lock(dev->subordinate);
+ else
+ pci_dev_lock(dev);
}
}
@@ -5506,8 +5508,10 @@ static void pci_bus_unlock(struct pci_bus *bus)
list_for_each_entry(dev, &bus->devices, bus_list) {
if (dev->subordinate)
pci_bus_unlock(dev->subordinate);
- pci_dev_unlock(dev);
+ else
+ pci_dev_unlock(dev);
}
+ pci_dev_unlock(bus->self);
}
/* Return 1 on successful lock, 0 on contention */
@@ -5515,15 +5519,15 @@ static int pci_bus_trylock(struct pci_bus *bus)
{
struct pci_dev *dev;
+ if (!pci_dev_trylock(bus->self))
+ return 0;
+
list_for_each_entry(dev, &bus->devices, bus_list) {
- if (!pci_dev_trylock(dev))
- goto unlock;
if (dev->subordinate) {
- if (!pci_bus_trylock(dev->subordinate)) {
- pci_dev_unlock(dev);
+ if (!pci_bus_trylock(dev->subordinate))
goto unlock;
- }
- }
+ } else if (!pci_dev_trylock(dev))
+ goto unlock;
}
return 1;
@@ -5531,8 +5535,10 @@ static int pci_bus_trylock(struct pci_bus *bus)
list_for_each_entry_continue_reverse(dev, &bus->devices, bus_list) {
if (dev->subordinate)
pci_bus_unlock(dev->subordinate);
- pci_dev_unlock(dev);
+ else
+ pci_dev_unlock(dev);
}
+ pci_dev_unlock(bus->self);
return 0;
}
@@ -5564,9 +5570,10 @@ static void pci_slot_lock(struct pci_slot *slot)
list_for_each_entry(dev, &slot->bus->devices, bus_list) {
if (!dev->slot || dev->slot != slot)
continue;
- pci_dev_lock(dev);
if (dev->subordinate)
pci_bus_lock(dev->subordinate);
+ else
+ pci_dev_lock(dev);
}
}
@@ -5592,14 +5599,13 @@ static int pci_slot_trylock(struct pci_slot *slot)
list_for_each_entry(dev, &slot->bus->devices, bus_list) {
if (!dev->slot || dev->slot != slot)
continue;
- if (!pci_dev_trylock(dev))
- goto unlock;
if (dev->subordinate) {
if (!pci_bus_trylock(dev->subordinate)) {
pci_dev_unlock(dev);
goto unlock;
}
- }
+ } else if (!pci_dev_trylock(dev))
+ goto unlock;
}
return 1;
@@ -5610,7 +5616,8 @@ static int pci_slot_trylock(struct pci_slot *slot)
continue;
if (dev->subordinate)
pci_bus_unlock(dev->subordinate);
- pci_dev_unlock(dev);
+ else
+ pci_dev_unlock(dev);
}
return 0;
}
--
2.43.0
next prev parent reply other threads:[~2024-07-28 16:09 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-28 16:08 [PATCH AUTOSEL 5.15 01/13] PCI: Add ACS quirk for Broadcom BCM5760X NIC Sasha Levin
2024-07-28 16:08 ` [PATCH AUTOSEL 5.15 02/13] PCI: Use preserve_config in place of pci_flags Sasha Levin
2024-07-28 16:08 ` [PATCH AUTOSEL 5.15 03/13] usb: cdns3: Add quirk flag to enable suspend residency Sasha Levin
2024-07-28 16:08 ` [PATCH AUTOSEL 5.15 04/13] usb: dwc2: gadget: Don't write invalid mapped sg entries into dma_desc with iommu enabled Sasha Levin
2024-07-28 16:08 ` [PATCH AUTOSEL 5.15 05/13] usb: typec: ucsi: Fix null pointer dereference in trace Sasha Levin
2024-07-28 16:08 ` [PATCH AUTOSEL 5.15 06/13] MIPS: Loongson64: DTS: Fix msi node for ls7a Sasha Levin
2024-07-28 16:08 ` [PATCH AUTOSEL 5.15 07/13] MIPS: Loongson64: DTS: Fix PCIe port nodes " Sasha Levin
2024-07-28 16:08 ` [PATCH AUTOSEL 5.15 08/13] PCI/AER: Disable AER service on suspend Sasha Levin
2024-07-28 16:08 ` Sasha Levin
2024-07-28 16:08 ` [PATCH AUTOSEL 5.15 09/13] ASoC: Intel: sof_sdw: fix jack detection on ADL-N variant RVP Sasha Levin
2024-07-28 16:08 ` [PATCH AUTOSEL 5.15 10/13] PCI: keystone: Add workaround for Errata #i2037 (AM65x SR 1.0) Sasha Levin
2024-07-28 16:08 ` [PATCH AUTOSEL 5.15 11/13] PCI: vmd: Create domain symlink before pci_bus_add_devices() Sasha Levin
2024-07-28 16:08 ` Sasha Levin [this message]
2024-07-28 16:08 ` [PATCH AUTOSEL 5.15 13/13] ALSA: usb: Fix UBSAN warning in parse_audio_unit() Sasha Levin
2024-12-17 6:24 ` Harshit Mogalapalli
2024-12-17 19:15 ` Sasha Levin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240728160907.2053634-12-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=bhelgaas@google.com \
--cc=dan.j.williams@intel.com \
--cc=dave.jiang@intel.com \
--cc=hdegoede@redhat.com \
--cc=imre.deak@intel.com \
--cc=kbusch@kernel.org \
--cc=kvalo@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.