[PATCH v2] PCI: Remove redudant call to pci_proc_attach_device()

[PATCH v2] PCI: Remove redudant call to pci_proc_attach_device()

[Manivannan Sadhasivam]

pci_proc_init() should just initialize the top level procfs directory for
PCI devices and let pci_bus_add_device() add the device specific procfs
attributes later.

But it is also calling pci_proc_attach_device() for each PCI device. This
causes a race condition with pci_bus_add_device(), which may run in
parallel, calling the same API, leading to the below warning spat:

 proc_dir_entry '000c:00/00.0' already registered
 WARNING: CPU: 2 PID: 179 at fs/proc/generic.c:375 proc_register+0xf6/0x180
  proc_register+0xf6/0x180
  proc_create_data+0x3e/0x60
  pci_proc_attach_device+0x74/0x130
  pci_bus_add_device+0x42/0x100
  pci_bus_add_devices+0xc6/0x110

Hence, remove the call to pci_proc_attach_device() from pci_proc_init().

Reported-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
Reported-by: Shuan He <heshuan@bytedance.com>
Closes: https://lore.kernel.org/linux-pci/20250702155112.40124-1-heshuan@bytedance.com
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---

Changes in v2:

* Dropped the sysfs change as it is supposed to be replaced by static resources
  change.

NOTE: I don't know if there is any reason to call pci_proc_attach_device()
before pci_bus_add_device(), but it definitely causes a race.

 drivers/pci/proc.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
index ce36e35681e8..de894ae70e9a 100644
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -463,13 +463,10 @@ int pci_proc_detach_bus(struct pci_bus *bus)
 
 static int __init pci_proc_init(void)
 {
-	struct pci_dev *dev = NULL;
 	proc_bus_pci_dir = proc_mkdir("bus/pci", NULL);
 	proc_create_seq("devices", 0, proc_bus_pci_dir,
 		    &proc_bus_pci_devices_op);
 	proc_initialized = 1;
-	for_each_pci_dev(dev)
-		pci_proc_attach_device(dev);
 
 	return 0;
 }
-- 
2.51.0

Re: [PATCH v2] PCI: Remove redudant call to pci_proc_attach_device()

[Bjorn Helgaas]

On Sat, Feb 28, 2026 at 12:51:29PM +0530, Manivannan Sadhasivam wrote:
> pci_proc_init() should just initialize the top level procfs directory for
> PCI devices and let pci_bus_add_device() add the device specific procfs
> attributes later.
> 
> But it is also calling pci_proc_attach_device() for each PCI device. This
> causes a race condition with pci_bus_add_device(), which may run in
> parallel, calling the same API, leading to the below warning spat:
> 
>  proc_dir_entry '000c:00/00.0' already registered
>  WARNING: CPU: 2 PID: 179 at fs/proc/generic.c:375 proc_register+0xf6/0x180
>   proc_register+0xf6/0x180
>   proc_create_data+0x3e/0x60
>   pci_proc_attach_device+0x74/0x130
>   pci_bus_add_device+0x42/0x100
>   pci_bus_add_devices+0xc6/0x110
> 
> Hence, remove the call to pci_proc_attach_device() from pci_proc_init().

Seems plausible, but given that this code has been this way basically
forever, I'd sure like to figure out what's different about Shuan and
Lorenzo's system that makes this issue show up now.

> Reported-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
> Reported-by: Shuan He <heshuan@bytedance.com>
> Closes: https://lore.kernel.org/linux-pci/20250702155112.40124-1-heshuan@bytedance.com
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
> 
> Changes in v2:
> 
> * Dropped the sysfs change as it is supposed to be replaced by static resources
>   change.

v1: https://lore.kernel.org/linux-pci/20250723111124.13694-1-manivannan.sadhasivam@oss.qualcomm.com/

> NOTE: I don't know if there is any reason to call pci_proc_attach_device()
> before pci_bus_add_device(), but it definitely causes a race.
> 
>  drivers/pci/proc.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
> index ce36e35681e8..de894ae70e9a 100644
> --- a/drivers/pci/proc.c
> +++ b/drivers/pci/proc.c
> @@ -463,13 +463,10 @@ int pci_proc_detach_bus(struct pci_bus *bus)
>  
>  static int __init pci_proc_init(void)
>  {
> -	struct pci_dev *dev = NULL;
>  	proc_bus_pci_dir = proc_mkdir("bus/pci", NULL);
>  	proc_create_seq("devices", 0, proc_bus_pci_dir,
>  		    &proc_bus_pci_devices_op);
>  	proc_initialized = 1;
> -	for_each_pci_dev(dev)
> -		pci_proc_attach_device(dev);
>  
>  	return 0;
>  }
> -- 
> 2.51.0
> 

Re: [PATCH v2] PCI: Remove redudant call to pci_proc_attach_device()

[Krzysztof Wilczyński]

Hello,

> > Hence, remove the call to pci_proc_attach_device() from pci_proc_init().
> 
> Seems plausible, but given that this code has been this way basically
> forever, I'd sure like to figure out what's different about Shuan and
> Lorenzo's system that makes this issue show up now.

Sadly, removing the for_each_pci_dev() loop breaks x86/ACPI systems
where it's the only mechanism creating per-device procfs entries.

On x86 with ACPI, PCI enumeration happens at subsys_initcall (level 4),
two levels before pci_proc_init() runs at device_initcall (level 6).

When the pci_bus_add_device() calls pci_proc_attach_device() during
enumeration, proc_initialized is still 0, so the call returns -EACCES
and no entry is created:

  subsys_initcall (level 4):                                                                                                                                                                                      
    acpi_init()                                                                                                                                                                                                 
      acpi_scan_init()                                                                                                                                                                                            
        acpi_bus_scan()                                                                                                                                                                                           
          acpi_pci_root_add()                                                                                                                                                                                     
            pci_acpi_scan_root()             <- devices are discovered                                                                                                                                              
            pci_bus_add_devices()
              pci_bus_add_device()
                pci_proc_attach_device()
                  if (!proc_initialized)     <- variable is not yet set
                    return -EACCES

  device_initcall (level 6):
    pci_proc_init()
      proc_initialized = 1
      for_each_pci_dev()                     <- iterates existing devices
        pci_proc_attach_device()             <- creates the procfs entries

Without that loop, the /proc/bus/pci/XX/YY.Z entries are never created
on these systems.  This can be verified under QEMU q35 with ACPI, where
removing the loop results in an empty /proc/bus/pci directory.

On platforms using Devicetree, the loop is usually not a problem, as host
bridges probe at the same device_initcall level, and with the current link
order, the pci_proc_init() runs first, so the loop finds zero new devices
and then pci_bus_add_device() creates the entries inline.  The race happens
when async probing causes both paths to overlap.

The fix needs to keep the loop but make it safe.  I have a patch that
wraps it with pci_lock_rescan_remove() and adds a simple dev->procent
idempotency check to pci_proc_attach_device(), see:

  https://lore.kernel.org/linux-pci/20260430003542.455198-1-kwilczynski@kernel.org

The lock serialises against concurrent pci_bus_add_device() calls,
and the dev->procent check makes the function idempotent so that
if both paths reach the same device, the second one returns early.
This also keeps code symmetric with pci_proc_detach_device(),
which already clears dev->procent.

Let me know what do you think, and if you have some other ideas.  Perhaps
there is a better way to do this.  Sadly, no static procfs attributes
exist for procfs, so this avenue is not available to us.

Thank you!

	Krzysztof