linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC 0/1] PCI: Fix pci devices double register WARNING in the kernel starting process
@ 2025-07-02 15:51 Shuan He
  2025-07-02 15:51 ` [PATCH] PCI: Fix pci devices double register WARN " Shuan He
  2025-07-03  6:35 ` [RFC 0/1] PCI: Fix pci devices double register WARNING " Sunil V L
  0 siblings, 2 replies; 7+ messages in thread
From: Shuan He @ 2025-07-02 15:51 UTC (permalink / raw)
  To: bhelgaas, cuiyunhui; +Cc: linux-pci, linux-kernel, heshuan, sunilvl

Hi All.
I encountered a WARNING printed out during the kernel starting process
on my developing environment.
(with RISC-V arch, 6.12 kernel, and Debian 13 OS).

WARN Trace:
[    0.518993] proc_dir_entry '000c:00/00.0' already registered
[    0.519187] WARNING: CPU: 2 PID: 179 at fs/proc/generic.c:375 proc_register+0xf6/0x180
[    0.519214] [<ffffffff804055a6>] proc_register+0xf6/0x180
[    0.519217] [<ffffffff80405a9e>] proc_create_data+0x3e/0x60
[    0.519220] [<ffffffff80616e44>] pci_proc_attach_device+0x74/0x130
[    0.509991] [<ffffffff805f1af2>] pci_bus_add_device+0x42/0x100
[    0.509997] [<ffffffff805f1c76>] pci_bus_add_devices+0xc6/0x110
[    0.519230] [<ffffffff8066763c>] acpi_pci_root_add+0x54c/0x810
[    0.519233] [<ffffffff8065d206>] acpi_bus_attach+0x196/0x2f0
[    0.519234] [<ffffffff8065d390>] acpi_scan_clear_dep_fn+0x30/0x70
[    0.519236] [<ffffffff800468fa>] process_one_work+0x19a/0x390
[    0.519239] [<ffffffff80047a6e>] worker_thread+0x2be/0x420
[    0.519241] [<ffffffff80050dc4>] kthread+0xc4/0xf0
[    0.519243] [<ffffffff80ad6ad2>] ret_from_fork+0xe/0x1c

After digging into this issue a little bit, I find the double-register
of PCIe devices occurs in the following logic:

Early:
static int __init pci_proc_init(void)
{
...
    for_each_pci_dev(dev)
        pci_proc_attach_device(dev);
        //000c:00:00.0 will be registered here for the first time (succeeded).
...
}

Later:
acpi_pci_root_add
-> pci_bus_add_devices
  -> pci_bus_add_device
    -> pci_proc_attach_device
    //try to register 000c:00:00.0 here for the second time
      (failed and triggered the WARNING trace);

I tried to add two more steps in the pci_proc_init function
(shown in the attached patch).
1st is to prevent the concurrent issue by holding the
pci_rescan_remove_lock.
2nd is to correctly update the device's status after
it's been successfully registered (by adding the
PCI_DEV_ADDED bit to the device's flag).

Then the WARNING disappeared and the system worked well. 

I understand that the device_initcall(pci_proc_init) function
stays there already for 20 years (time of the initiliaztion of
repo), and it hasn't really changed since then.
So I wonder if my patch is the RIGHT way to fix this WARNING issue?
I am not positive about this. :(

Any suggestions?

As a beginner Linux programmer, I am not sure whether I have included
all related reviews/maintainers in my email list, but sincerely seeking
your help and any comments are very welcome.

Warmly regards,
Shuan

Shuan He (1):
  PCI: Fix pci devices double register WARN in the kernel starting
    process

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

-- 
2.39.5 (Apple Git-154)


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] PCI: Fix pci devices double register WARN in the kernel starting process
  2025-07-02 15:51 [RFC 0/1] PCI: Fix pci devices double register WARNING in the kernel starting process Shuan He
@ 2025-07-02 15:51 ` Shuan He
  2025-07-03 15:09   ` kernel test robot
  2025-07-03  6:35 ` [RFC 0/1] PCI: Fix pci devices double register WARNING " Sunil V L
  1 sibling, 1 reply; 7+ messages in thread
From: Shuan He @ 2025-07-02 15:51 UTC (permalink / raw)
  To: bhelgaas, cuiyunhui; +Cc: linux-pci, linux-kernel, heshuan, sunilvl

To avoid the double register of pcie device (otherwise a WARNING will
be triggered), two more steps are added in the pci_proc_init function.
1. The pci_rescan_remove_lock is held to prevent the concurrent issue
with pci_bus_add_devices.
2. The PCI_DEV_ADDED bit is added to dev->priv_flags after the device is
successfully registered to procfs.

WARN Trace:
[    0.518993] proc_dir_entry '000c:00/00.0' already registered
[    0.519187] WARNING: CPU: 2 PID: 179 at fs/proc/generic.c:375 proc_register+0xf6/0x180
[    0.519214] [<ffffffff804055a6>] proc_register+0xf6/0x180
[    0.519217] [<ffffffff80405a9e>] proc_create_data+0x3e/0x60
[    0.519220] [<ffffffff80616e44>] pci_proc_attach_device+0x74/0x130
[    0.509991] [<ffffffff805f1af2>] pci_bus_add_device+0x42/0x100
[    0.509997] [<ffffffff805f1c76>] pci_bus_add_devices+0xc6/0x110
[    0.519230] [<ffffffff8066763c>] acpi_pci_root_add+0x54c/0x810
[    0.519233] [<ffffffff8065d206>] acpi_bus_attach+0x196/0x2f0
[    0.519234] [<ffffffff8065d390>] acpi_scan_clear_dep_fn+0x30/0x70
[    0.519236] [<ffffffff800468fa>] process_one_work+0x19a/0x390
[    0.519239] [<ffffffff80047a6e>] worker_thread+0x2be/0x420
[    0.519241] [<ffffffff80050dc4>] kthread+0xc4/0xf0
[    0.519243] [<ffffffff80ad6ad2>] ret_from_fork+0xe/0x1c

Signed-off-by: Shuan He <heshuan@bytedance.com>
---
 drivers/pci/proc.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
index 9348a0fb8084..6506316c8a31 100644
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -468,9 +468,14 @@ static int __init pci_proc_init(void)
 	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);
-
+	pci_lock_rescan_remove();
+	for_each_pci_dev(dev) {
+		if (pci_dev_is_added(dev))
+			continue;
+		if (!pci_proc_attach_device(dev))
+			pci_dev_assign_added(dev, true);
+	}
+	pci_unlock_rescan_remove();
 	return 0;
 }
 device_initcall(pci_proc_init);
-- 
2.39.5 (Apple Git-154)


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [RFC 0/1] PCI: Fix pci devices double register WARNING in the kernel starting process
  2025-07-02 15:51 [RFC 0/1] PCI: Fix pci devices double register WARNING in the kernel starting process Shuan He
  2025-07-02 15:51 ` [PATCH] PCI: Fix pci devices double register WARN " Shuan He
@ 2025-07-03  6:35 ` Sunil V L
  2025-07-03 11:31   ` [External] " 拴何
  1 sibling, 1 reply; 7+ messages in thread
From: Sunil V L @ 2025-07-03  6:35 UTC (permalink / raw)
  To: Shuan He; +Cc: bhelgaas, cuiyunhui, linux-pci, linux-kernel

On Wed, Jul 02, 2025 at 11:51:11PM +0800, Shuan He wrote:
> Hi All.
> I encountered a WARNING printed out during the kernel starting process
> on my developing environment.
> (with RISC-V arch, 6.12 kernel, and Debian 13 OS).
> 
> WARN Trace:
> [    0.518993] proc_dir_entry '000c:00/00.0' already registered
> [    0.519187] WARNING: CPU: 2 PID: 179 at fs/proc/generic.c:375 proc_register+0xf6/0x180
> [    0.519214] [<ffffffff804055a6>] proc_register+0xf6/0x180
> [    0.519217] [<ffffffff80405a9e>] proc_create_data+0x3e/0x60
> [    0.519220] [<ffffffff80616e44>] pci_proc_attach_device+0x74/0x130
> [    0.509991] [<ffffffff805f1af2>] pci_bus_add_device+0x42/0x100
> [    0.509997] [<ffffffff805f1c76>] pci_bus_add_devices+0xc6/0x110
> [    0.519230] [<ffffffff8066763c>] acpi_pci_root_add+0x54c/0x810
> [    0.519233] [<ffffffff8065d206>] acpi_bus_attach+0x196/0x2f0
> [    0.519234] [<ffffffff8065d390>] acpi_scan_clear_dep_fn+0x30/0x70
> [    0.519236] [<ffffffff800468fa>] process_one_work+0x19a/0x390
> [    0.519239] [<ffffffff80047a6e>] worker_thread+0x2be/0x420
> [    0.519241] [<ffffffff80050dc4>] kthread+0xc4/0xf0
> [    0.519243] [<ffffffff80ad6ad2>] ret_from_fork+0xe/0x1c
> 
This should not happen. I suspect some issue in ACPI namespace/_PRT. Can
you reproduce this on qemu virt machine?

Regards
Sunil


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [External] Re: [RFC 0/1] PCI: Fix pci devices double register WARNING in the kernel starting process
  2025-07-03  6:35 ` [RFC 0/1] PCI: Fix pci devices double register WARNING " Sunil V L
@ 2025-07-03 11:31   ` 拴何
  2025-07-03 12:48     ` Manivannan Sadhasivam
  0 siblings, 1 reply; 7+ messages in thread
From: 拴何 @ 2025-07-03 11:31 UTC (permalink / raw)
  To: Sunil V L; +Cc: bhelgaas, cuiyunhui, linux-pci, linux-kernel

Hi Sunil,
Thanks for your reply! (really appricate it).
This WARNING truly occurred. Through the added debug info, I found
that the device was registered to proc via pci_proc_init and
acpi_pci_root_add paths respectively, which ultimately triggered
the warning message.
Let me try to reproduce it on qemu first. I'll keep you updated.
Thanks again.

Regards,
Shuan

On Thu, Jul 3, 2025 at 2:36 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
>
> On Wed, Jul 02, 2025 at 11:51:11PM +0800, Shuan He wrote:
> > Hi All.
> > I encountered a WARNING printed out during the kernel starting process
> > on my developing environment.
> > (with RISC-V arch, 6.12 kernel, and Debian 13 OS).
> >
> > WARN Trace:
> > [    0.518993] proc_dir_entry '000c:00/00.0' already registered
> > [    0.519187] WARNING: CPU: 2 PID: 179 at fs/proc/generic.c:375 proc_register+0xf6/0x180
> > [    0.519214] [<ffffffff804055a6>] proc_register+0xf6/0x180
> > [    0.519217] [<ffffffff80405a9e>] proc_create_data+0x3e/0x60
> > [    0.519220] [<ffffffff80616e44>] pci_proc_attach_device+0x74/0x130
> > [    0.509991] [<ffffffff805f1af2>] pci_bus_add_device+0x42/0x100
> > [    0.509997] [<ffffffff805f1c76>] pci_bus_add_devices+0xc6/0x110
> > [    0.519230] [<ffffffff8066763c>] acpi_pci_root_add+0x54c/0x810
> > [    0.519233] [<ffffffff8065d206>] acpi_bus_attach+0x196/0x2f0
> > [    0.519234] [<ffffffff8065d390>] acpi_scan_clear_dep_fn+0x30/0x70
> > [    0.519236] [<ffffffff800468fa>] process_one_work+0x19a/0x390
> > [    0.519239] [<ffffffff80047a6e>] worker_thread+0x2be/0x420
> > [    0.519241] [<ffffffff80050dc4>] kthread+0xc4/0xf0
> > [    0.519243] [<ffffffff80ad6ad2>] ret_from_fork+0xe/0x1c
> >
> This should not happen. I suspect some issue in ACPI namespace/_PRT. Can
> you reproduce this on qemu virt machine?
>
> Regards
> Sunil
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [External] Re: [RFC 0/1] PCI: Fix pci devices double register WARNING in the kernel starting process
  2025-07-03 11:31   ` [External] " 拴何
@ 2025-07-03 12:48     ` Manivannan Sadhasivam
  2025-07-04  7:41       ` He Shuan
  0 siblings, 1 reply; 7+ messages in thread
From: Manivannan Sadhasivam @ 2025-07-03 12:48 UTC (permalink / raw)
  To: 拴何; +Cc: Sunil V L, bhelgaas, cuiyunhui, linux-pci, linux-kernel

On Thu, Jul 03, 2025 at 07:31:10PM GMT, 拴何 wrote:
> Hi Sunil,
> Thanks for your reply! (really appricate it).
> This WARNING truly occurred. Through the added debug info, I found
> that the device was registered to proc via pci_proc_init and
> acpi_pci_root_add paths respectively, which ultimately triggered
> the warning message.
> Let me try to reproduce it on qemu first. I'll keep you updated.
> Thanks again.
> 

I think you have uncovered a valid bug. There is nothing preventing (except the
blessings of the initcall order) the occurence of the race between
pci_proc_init() and pci_bus_add_device(). I think it went mostly unnoticed
because, pci_proc_init() gets called very early before any PCI devices were
registered. So for_each_pci_dev() loop never gets executed.

But in your case, looks like the PCI device is available somehow before
pci_proc_init() gets executed. Now, it is not very clear to me how the device
becomes available at this point. It might be due to some other issue. But in
anycase, I think we need to get rid of calling pci_proc_attach_device() from
pci_proc_init() as I don't see a reason to call this function from two
different places. pci_bus_add_device() should be the one calling this function
as it is the one adding the PCI device.

Ironically, I do see a similar pattern for sysfs also. Maybe there is (or was) a
reason to create these files from two different places?

- Mani

-- 
மணிவண்ணன் சதாசிவம்

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] PCI: Fix pci devices double register WARN in the kernel starting process
  2025-07-02 15:51 ` [PATCH] PCI: Fix pci devices double register WARN " Shuan He
@ 2025-07-03 15:09   ` kernel test robot
  0 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2025-07-03 15:09 UTC (permalink / raw)
  To: Shuan He, bhelgaas, cuiyunhui
  Cc: oe-kbuild-all, linux-pci, linux-kernel, heshuan, sunilvl

Hi Shuan,

kernel test robot noticed the following build errors:

[auto build test ERROR on pci/next]
[also build test ERROR on pci/for-linus linus/master v6.16-rc4 next-20250703]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Shuan-He/PCI-Fix-pci-devices-double-register-WARN-in-the-kernel-starting-process/20250703-035442
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:    https://lore.kernel.org/r/20250702155112.40124-2-heshuan%40bytedance.com
patch subject: [PATCH] PCI: Fix pci devices double register WARN in the kernel starting process
config: alpha-allnoconfig (https://download.01.org/0day-ci/archive/20250703/202507032253.sXSgz4lH-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250703/202507032253.sXSgz4lH-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507032253.sXSgz4lH-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/pci/proc.c: In function 'pci_proc_init':
>> drivers/pci/proc.c:476:25: error: too many arguments to function 'pci_dev_assign_added'; expected 1, have 2
     476 |                         pci_dev_assign_added(dev, true);
         |                         ^~~~~~~~~~~~~~~~~~~~      ~~~~
   In file included from drivers/pci/proc.c:18:
   drivers/pci/pci.h:577:20: note: declared here
     577 | static inline void pci_dev_assign_added(struct pci_dev *dev)
         |                    ^~~~~~~~~~~~~~~~~~~~


vim +/pci_dev_assign_added +476 drivers/pci/proc.c

   463	
   464	static int __init pci_proc_init(void)
   465	{
   466		struct pci_dev *dev = NULL;
   467		proc_bus_pci_dir = proc_mkdir("bus/pci", NULL);
   468		proc_create_seq("devices", 0, proc_bus_pci_dir,
   469			    &proc_bus_pci_devices_op);
   470		proc_initialized = 1;
   471		pci_lock_rescan_remove();
   472		for_each_pci_dev(dev) {
   473			if (pci_dev_is_added(dev))
   474				continue;
   475			if (!pci_proc_attach_device(dev))
 > 476				pci_dev_assign_added(dev, true);

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [External] Re: [RFC 0/1] PCI: Fix pci devices double register WARNING in the kernel starting process
  2025-07-03 12:48     ` Manivannan Sadhasivam
@ 2025-07-04  7:41       ` He Shuan
  0 siblings, 0 replies; 7+ messages in thread
From: He Shuan @ 2025-07-04  7:41 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Sunil V L, bhelgaas, cuiyunhui, linux-pci, linux-kernel

Hi Mani,

Thanks for your comments.

>But in your case, looks like the PCI device is available somehow before
>pci_proc_init() gets executed. Now, it is not very clear to me how the device
>becomes available at this point. It might be due to some other issue. But in
>anycase, I think we need to get rid of calling pci_proc_attach_device() from
>pci_proc_init() as I don't see a reason to call this function from two
>different places. pci_bus_add_device() should be the one calling this function
>as it is the one adding the PCI device.
Got it. I need to figure out why the PCI device is available already before
pci_proc_init() is executed. (Actually I didn't change too much source code yet,
basically running my test based on the upstream code).

>Ironically, I do see a similar pattern for sysfs also. Maybe there is (or was) a
>reason to create these files from two different places?
Yes, I see the sysfs register confusion (pci_create_sysfs_dev_files) from
pci_sysfs_init() and pci_bus_add_device() as well. There do have concurrence
protection through pci_bus_add_device() paths, so I agree that function
pci_proc_attach_device and pci_create_sysfs_dev_files should be called
from pci_bus_add_devices().

Anyway, it appears there is a great deal of work/effort needed before
making this part clear. :(

Bests,
Shuan

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-07-04  7:41 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-02 15:51 [RFC 0/1] PCI: Fix pci devices double register WARNING in the kernel starting process Shuan He
2025-07-02 15:51 ` [PATCH] PCI: Fix pci devices double register WARN " Shuan He
2025-07-03 15:09   ` kernel test robot
2025-07-03  6:35 ` [RFC 0/1] PCI: Fix pci devices double register WARNING " Sunil V L
2025-07-03 11:31   ` [External] " 拴何
2025-07-03 12:48     ` Manivannan Sadhasivam
2025-07-04  7:41       ` He Shuan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).