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

[Niklas Schnelle]

On Wed, 2026-03-11 at 14:27 +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 variable
> `pci_rescan_remove_lock` itself is private to objects residing in
> `drivers/pci/` via the header `drivers/pci/pci.h`.
> 
> This makes it possible to use the lock - lock it, unlock it - from
> anywhere, but 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 to make the usage more safe.
> 
> 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@xxxxxxxxxxxxx>
> ---
>  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);

This has a (rather trivial) merge conflict with Ionut's patch which at
the same time is a prerequisite for this series. Sadly since that isn't
in linux-next yet I'm not sure how to best handle this. Maybe it would
make sense to just include it in this series? @Ionut would that be ok
for you?

>  
>  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);
>  

I do see Keith's argument that proliferation of the rescan/remove lock
is to be minimized. That said, since user's of this header can already
lock/unlock I don't think this patch makes matters worse. In fact we
want this patch to be able to add better lockdep asserts so it will
help against misuse.

With that feel free to add:

Reviewed-by: Niklas Schnelle <schnelle@xxxxxxxxxxxxx>

Thanks,
Niklas