[PATCH 1/4] PCI: Move declaration of pci_rescan_remove_lock into public pci.h

Benjamin Block posted 4 patches 1 month ago
There is a newer version of this series
[PATCH 1/4] PCI: Move declaration of pci_rescan_remove_lock into public pci.h
Posted by Benjamin Block 1 month ago
So far it is possible to use and call the functions
pci_lock_rescan_remove() and pci_unlock_rescan_remove() from any PCI
code, including modules and architecture code; but the lock
`pci_rescan_remove_lock` itself is private to objects residing in
`drivers/pci/` via the header `drivers/pci/pci.h`.

With that setup it is not possible to use lockdep annotations such as
lockdep_assert_held(), or sparse annotations such as __must_hold() in
modules or architecture code for PCI.

Since it is useful for `pci_rescan_remove_lock` to have such
annotations, move the variable declaration into `include/linux/pci.h`.

Signed-off-by: Benjamin Block <bblock@linux.ibm.com>
---
 drivers/pci/pci.h   | 2 --
 drivers/pci/probe.c | 1 +
 include/linux/pci.h | 2 ++
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 13d998fbacce..6d611523420f 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -110,8 +110,6 @@ struct pcie_tlp_log;
 extern const unsigned char pcie_link_speed[];
 extern bool pci_early_dump;
 
-extern struct mutex pci_rescan_remove_lock;
-
 bool pcie_cap_has_lnkctl(const struct pci_dev *dev);
 bool pcie_cap_has_lnkctl2(const struct pci_dev *dev);
 bool pcie_cap_has_rtctl(const struct pci_dev *dev);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index bccc7a4bdd79..e5b12878e972 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -3509,6 +3509,7 @@ EXPORT_SYMBOL_GPL(pci_rescan_bus);
  * routines should always be executed under this mutex.
  */
 DEFINE_MUTEX(pci_rescan_remove_lock);
+EXPORT_SYMBOL_GPL(pci_rescan_remove_lock);
 
 void pci_lock_rescan_remove(void)
 {
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 1c270f1d5123..fd7a962a64ef 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -39,6 +39,7 @@
 #include <linux/io.h>
 #include <linux/resource_ext.h>
 #include <linux/msi_api.h>
+#include <linux/mutex.h>
 #include <uapi/linux/pci.h>
 
 #include <linux/pci_ids.h>
@@ -1533,6 +1534,7 @@ void set_pcie_hotplug_bridge(struct pci_dev *pdev);
 
 /* Functions for PCI Hotplug drivers to use */
 unsigned int pci_rescan_bus(struct pci_bus *bus);
+extern struct mutex pci_rescan_remove_lock;
 void pci_lock_rescan_remove(void);
 void pci_unlock_rescan_remove(void);
 
-- 
2.53.0
Re: [PATCH 1/4] PCI: Move declaration of pci_rescan_remove_lock into public pci.h
Posted by Keith Busch 1 month ago
On Fri, Mar 06, 2026 at 05:49:13PM +0100, Benjamin Block wrote:
> So far it is possible to use and call the functions
> pci_lock_rescan_remove() and pci_unlock_rescan_remove() from any PCI
> code, including modules and architecture code; but the lock
> `pci_rescan_remove_lock` itself is private to objects residing in
> `drivers/pci/` via the header `drivers/pci/pci.h`.
> 
> With that setup it is not possible to use lockdep annotations such as
> lockdep_assert_held(), or sparse annotations such as __must_hold() in
> modules or architecture code for PCI.
> 
> Since it is useful for `pci_rescan_remove_lock` to have such
> annotations, move the variable declaration into `include/linux/pci.h`.

This big lock for pci scanning is way to easy to misuse to create
deadlocks, many of which still exist today, so I'm not sure making it
easier to access is the right direction.
Re: [PATCH 1/4] PCI: Move declaration of pci_rescan_remove_lock into public pci.h
Posted by Benjamin Block 1 month ago
On Fri, Mar 06, 2026 at 10:31:45AM -0700, Keith Busch wrote:
> On Fri, Mar 06, 2026 at 05:49:13PM +0100, Benjamin Block wrote:
> > So far it is possible to use and call the functions
> > pci_lock_rescan_remove() and pci_unlock_rescan_remove() from any PCI
> > code, including modules and architecture code; but the lock
> > `pci_rescan_remove_lock` itself is private to objects residing in
> > `drivers/pci/` via the header `drivers/pci/pci.h`.
> > 
> > With that setup it is not possible to use lockdep annotations such as
> > lockdep_assert_held(), or sparse annotations such as __must_hold() in
> > modules or architecture code for PCI.
> > 
> > Since it is useful for `pci_rescan_remove_lock` to have such
> > annotations, move the variable declaration into `include/linux/pci.h`.
> 
> This big lock for pci scanning is way to easy to misuse to create
> deadlocks, many of which still exist today, 

I can fully appreciate that, having had to deal with bug reports that
stem from it's use for several months at this point.

> so I'm not sure making it easier to access is the right direction.

The thing is, I really would love to be able to annotate our
architecture code where appropriate, and I didn't see any other option.
Explicit annotations in the code highlight far better than putting a
comment in the function/-header.

-- 
Best Regards und Beste Grüße, Benjamin Block
               PGP KeyID: 9610 2BB8 2E17 6F65 2362  6DF2 46E0 4E05 67A3 2E9E