| Message ID | 171711746953.1628941.4692125082286867825.stgit@dwillia2-xfh.jf.intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 0d1a1c6745a891f3ae7154e96d1179d129400b05 |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI: Revert / replace the cfg_access_lock lockdep mechanism | expand |
On 5/30/24 6:04 PM, Dan Williams wrote: > The recent adventure with adding lockdep tracking for cfg_access_lock, > while it yielded many false positives [1], it did catch a true positive in > the pci_reset_bus() path [2]. > > So, while lockdep is difficult to deploy, open coding a check that > cfg_access_lock is held during the reset is feasible. > > While this does not offer a full backtrace, it should be sufficient to > implicate the caller of pci_bridge_secondary_bus_reset() as a path that > needs investigation. > > Link: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_134186v1/shard-dg2-1/igt@device_reset@unbind-reset-rebind.html [1] > Link: http://lore.kernel.org/r/cfb50601-5d2a-4676-a958-1bd3f1b06654@intel.com [2] > Cc: Dave Jiang <dave.jiang@intel.com> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> Reviewed-by: Dave Jiang <dave.jiang@intel.com> > --- > drivers/pci/pci.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 35fb1f17a589..8df32a3a0979 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -4883,6 +4883,9 @@ void __weak pcibios_reset_secondary_bus(struct pci_dev *dev) > */ > int pci_bridge_secondary_bus_reset(struct pci_dev *dev) > { > + if (!dev->block_cfg_access) > + pci_warn_once(dev, "unlocked secondary bus reset via: %pS\n", > + __builtin_return_address(0)); > pcibios_reset_secondary_bus(dev); > > return pci_bridge_wait_for_secondary_bus(dev, "bus reset"); >
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 35fb1f17a589..8df32a3a0979 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -4883,6 +4883,9 @@ void __weak pcibios_reset_secondary_bus(struct pci_dev *dev) */ int pci_bridge_secondary_bus_reset(struct pci_dev *dev) { + if (!dev->block_cfg_access) + pci_warn_once(dev, "unlocked secondary bus reset via: %pS\n", + __builtin_return_address(0)); pcibios_reset_secondary_bus(dev); return pci_bridge_wait_for_secondary_bus(dev, "bus reset");
The recent adventure with adding lockdep tracking for cfg_access_lock, while it yielded many false positives [1], it did catch a true positive in the pci_reset_bus() path [2]. So, while lockdep is difficult to deploy, open coding a check that cfg_access_lock is held during the reset is feasible. While this does not offer a full backtrace, it should be sufficient to implicate the caller of pci_bridge_secondary_bus_reset() as a path that needs investigation. Link: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_134186v1/shard-dg2-1/igt@device_reset@unbind-reset-rebind.html [1] Link: http://lore.kernel.org/r/cfb50601-5d2a-4676-a958-1bd3f1b06654@intel.com [2] Cc: Dave Jiang <dave.jiang@intel.com> Cc: Bjorn Helgaas <bhelgaas@google.com> Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- drivers/pci/pci.c | 3 +++ 1 file changed, 3 insertions(+)