[PATCH] PCI: Adjust pci_sysfs_init() to device_initcall

[PATCH] PCI: Adjust pci_sysfs_init() to device_initcall

[Ian]

From: Ian Ding <4dark@outlook.com>

When the controller driver uses async probe
(.probe_type = PROBE_PREFER_ASYNCHRONOUS), pci_host_probe() is not
guaranteed to run before pci_sysfs_init(), kernel may call
pci_create_sysfs_dev_files() twice in pci_sysfs_init() and
pci_host_probe() -> pci_bus_add_device(), and dump stack:

    sysfs: cannot create duplicate filename

Signed-off-by: Ian Ding <4dark@outlook.com>
---
 drivers/pci/pci-sysfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index dd0d9d9bc..bef25fecb 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1534,7 +1534,7 @@ static int __init pci_sysfs_init(void)
 
 	return 0;
 }
-late_initcall(pci_sysfs_init);
+device_initcall(pci_sysfs_init);
 
 static struct attribute *pci_dev_dev_attrs[] = {
 	&dev_attr_boot_vga.attr,
-- 
2.25.1

Re: [PATCH] PCI: Adjust pci_sysfs_init() to device_initcall

[Bjorn Helgaas]

[+cc Krzysztof]

On Wed, Aug 07, 2024 at 06:34:34PM +0800, Ian wrote:
> From: Ian Ding <4dark@outlook.com>
> 
> When the controller driver uses async probe
> (.probe_type = PROBE_PREFER_ASYNCHRONOUS), pci_host_probe() is not
> guaranteed to run before pci_sysfs_init(), kernel may call
> pci_create_sysfs_dev_files() twice in pci_sysfs_init() and
> pci_host_probe() -> pci_bus_add_device(), and dump stack:
> 
>     sysfs: cannot create duplicate filename
> 
> Signed-off-by: Ian Ding <4dark@outlook.com>
> ---
>  drivers/pci/pci-sysfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index dd0d9d9bc..bef25fecb 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1534,7 +1534,7 @@ static int __init pci_sysfs_init(void)
>  
>  	return 0;
>  }
> -late_initcall(pci_sysfs_init);
> +device_initcall(pci_sysfs_init);

This is certainly a problem that needs to be solved, but I don't think
this approach is necessarily safe.

There's a long discussion about some of the issues at
https://lore.kernel.org/linux-pci/20200716110423.xtfyb3n6tn5ixedh@pali/t/#u

The goal is to remove pci_sysfs_init() completely, but we haven't
quite got there yet.

Since that thread, Krzysztof has made great progress by converting
most sysfs files to static attributes, e.g.,

  506140f9c06b ("PCI/sysfs: Convert "index", "acpi_index", "label" to static attributes")
  d93f8399053d ("PCI/sysfs: Convert "vpd" to static attribute")
  f42c35ea3b13 ("PCI/sysfs: Convert "reset" to static attribute")
  527139d738d7 ("PCI/sysfs: Convert "rom" to static attribute")
  e1d3f3268b0e ("PCI/sysfs: Convert "config" to static attribute")

but there are still a couple things left.

>  static struct attribute *pci_dev_dev_attrs[] = {
>  	&dev_attr_boot_vga.attr,
> -- 
> 2.25.1
>