[PATCH v2] PCI: Adjust visibility of boot_display attribute instead of creation

[PATCH v2] PCI: Adjust visibility of boot_display attribute instead of creation

[Mario Limonciello]

From: Mario Limonciello <mario.limonciello@amd.com>

There is a desire to avoid creating new sysfs files late, so instead
of dynamically deciding to create the boot_display attribute, make
it static and use sysfs_update_group() to adjust visibility on the
applicable devices.

This also fixes a compilation warning when compiled without
CONFIG_VIDEO on sparc.

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Closes: https://lore.kernel.org/linux-next/20250718224118.5b3f22b0@canb.auug.org.au/
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v2:
 * Change to sysfs_update_group() instead
---
 drivers/pci/pci-sysfs.c | 59 +++++++++++++++++------------------------
 1 file changed, 25 insertions(+), 34 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 6b1a0ae254d3a..462a90d13eb87 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1059,37 +1059,6 @@ void pci_remove_legacy_files(struct pci_bus *b)
 }
 #endif /* HAVE_PCI_LEGACY */
 
-/**
- * pci_create_boot_display_file - create a file in sysfs for @dev
- * @pdev: dev in question
- *
- * Creates a file `boot_display` in sysfs for the PCI device @pdev
- * if it is the boot display device.
- */
-static int pci_create_boot_display_file(struct pci_dev *pdev)
-{
-#ifdef CONFIG_VIDEO
-	if (video_is_primary_device(&pdev->dev))
-		return sysfs_create_file(&pdev->dev.kobj, &dev_attr_boot_display.attr);
-#endif
-	return 0;
-}
-
-/**
- * pci_remove_boot_display_file - remove the boot display file for @dev
- * @pdev: dev in question
- *
- * Removes the file `boot_display` in sysfs for the PCI device @pdev
- * if it is the boot display device.
- */
-static void pci_remove_boot_display_file(struct pci_dev *pdev)
-{
-#ifdef CONFIG_VIDEO
-	if (video_is_primary_device(&pdev->dev))
-		sysfs_remove_file(&pdev->dev.kobj, &dev_attr_boot_display.attr);
-#endif
-}
-
 #if defined(HAVE_PCI_MMAP) || defined(ARCH_GENERIC_PCI_MMAP_RESOURCE)
 /**
  * pci_mmap_resource - map a PCI resource into user memory space
@@ -1691,6 +1660,29 @@ static const struct attribute_group pci_dev_resource_resize_group = {
 	.is_visible = resource_resize_is_visible,
 };
 
+static struct attribute *pci_display_attrs[] = {
+	&dev_attr_boot_display.attr,
+	NULL,
+};
+
+static umode_t pci_boot_display_visible(struct kobject *kobj,
+					struct attribute *a, int n)
+{
+#ifdef CONFIG_VIDEO
+	struct device *dev = kobj_to_dev(kobj);
+	struct pci_dev *pdev = to_pci_dev(dev);
+
+	if (a == &dev_attr_boot_display.attr && video_is_primary_device(&pdev->dev))
+		return a->mode;
+#endif
+	return 0;
+}
+
+static const struct attribute_group pci_display_attr_group = {
+	.attrs = pci_display_attrs,
+	.is_visible = pci_boot_display_visible,
+};
+
 int __must_check pci_create_sysfs_dev_files(struct pci_dev *pdev)
 {
 	int retval;
@@ -1698,7 +1690,7 @@ int __must_check pci_create_sysfs_dev_files(struct pci_dev *pdev)
 	if (!sysfs_initialized)
 		return -EACCES;
 
-	retval = pci_create_boot_display_file(pdev);
+	retval = sysfs_update_group(&pdev->dev.kobj, &pci_display_attr_group);
 	if (retval)
 		return retval;
 
@@ -1716,7 +1708,6 @@ void pci_remove_sysfs_dev_files(struct pci_dev *pdev)
 	if (!sysfs_initialized)
 		return;
 
-	pci_remove_boot_display_file(pdev);
 	pci_remove_resource_files(pdev);
 }
 
@@ -1755,7 +1746,6 @@ static umode_t pci_dev_attrs_are_visible(struct kobject *kobj,
 
 	if (a == &dev_attr_boot_vga.attr && pci_is_vga(pdev))
 		return a->mode;
-
 	return 0;
 }
 
@@ -1845,6 +1835,7 @@ static const struct attribute_group pcie_dev_attr_group = {
 
 const struct attribute_group *pci_dev_attr_groups[] = {
 	&pci_dev_attr_group,
+	&pci_display_attr_group,
 	&pci_dev_hp_attr_group,
 #ifdef CONFIG_PCI_IOV
 	&sriov_pf_dev_attr_group,
-- 
2.43.0

Re: [PATCH v2] PCI: Adjust visibility of boot_display attribute instead of creation

[Lukas Wunner]

On Sun, Jul 20, 2025 at 10:15:08AM -0500, Mario Limonciello wrote:
> There is a desire to avoid creating new sysfs files late, so instead
> of dynamically deciding to create the boot_display attribute, make
> it static and use sysfs_update_group() to adjust visibility on the
> applicable devices.
[...]
> @@ -1698,7 +1690,7 @@ int __must_check pci_create_sysfs_dev_files(struct pci_dev *pdev)
>  	if (!sysfs_initialized)
>  		return -EACCES;
>  
> -	retval = pci_create_boot_display_file(pdev);
> +	retval = sysfs_update_group(&pdev->dev.kobj, &pci_display_attr_group);
>  	if (retval)
>  		return retval;
>

pci_create_sysfs_dev_files() is called from two places, pci_bus_add_device()
and the late_initcall pci_sysfs_init().

Do you really need to update the group in both or would one of them suffice?
If the latter, please update it only in that single place.  Better yet,
if you know where visibility of the attribute may change (e.g. after certain
function calls in graphics drivers), add the call there instead of in the
PCI core.

We should probably add a code comment to pci_create_sysfs_dev_files() that
it is slated for removal (once the resource files have been converted to
static as well) and that no new attributes are allowed to be added here.

Thanks,

Lukas

Re: [PATCH v2] PCI: Adjust visibility of boot_display attribute instead of creation

[Manivannan Sadhasivam]

On Sun, Jul 20, 2025 at 05:34:44PM GMT, Lukas Wunner wrote:
> On Sun, Jul 20, 2025 at 10:15:08AM -0500, Mario Limonciello wrote:
> > There is a desire to avoid creating new sysfs files late, so instead
> > of dynamically deciding to create the boot_display attribute, make
> > it static and use sysfs_update_group() to adjust visibility on the
> > applicable devices.
> [...]
> > @@ -1698,7 +1690,7 @@ int __must_check pci_create_sysfs_dev_files(struct pci_dev *pdev)
> >  	if (!sysfs_initialized)
> >  		return -EACCES;
> >  
> > -	retval = pci_create_boot_display_file(pdev);
> > +	retval = sysfs_update_group(&pdev->dev.kobj, &pci_display_attr_group);
> >  	if (retval)
> >  		return retval;
> >
> 
> pci_create_sysfs_dev_files() is called from two places, pci_bus_add_device()
> and the late_initcall pci_sysfs_init().
> 

Separate question: Do we really need to call pci_create_sysfs_dev_files() from
pci_sysfs_init()? It is already getting called from pci_bus_add_device() and it
looks redundant to me. More importantly, it can also lead to a race (though
won't happen practically) [1].

Same goes for pci_proc_attach_device(). I was tempted to submit a patch removing
both these calls from pci_sysfs_init() and pci_proc_init(), but wanted to check
first.

[1] https://lore.kernel.org/linux-pci/r7fgb5xrn6ocstq6ctq4q7r4o2esgh4rqr44c3u234kcep6thk@bge2vzl33ptb/

- Mani 

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

Re: [PATCH v2] PCI: Adjust visibility of boot_display attribute instead of creation

[Jonathan Cameron]

On Sun, 20 Jul 2025 10:15:08 -0500
Mario Limonciello <superm1@kernel.org> wrote:

> From: Mario Limonciello <mario.limonciello@amd.com>
> 
> There is a desire to avoid creating new sysfs files late, so instead
> of dynamically deciding to create the boot_display attribute, make
> it static and use sysfs_update_group() to adjust visibility on the
> applicable devices.
> 
> This also fixes a compilation warning when compiled without
> CONFIG_VIDEO on sparc.
> 
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Closes: https://lore.kernel.org/linux-next/20250718224118.5b3f22b0@canb.auug.org.au/
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Leaving aside the 'where to call it' discussions, a few bits of feedback on the
code. See inline.


> ---
> v2:
>  * Change to sysfs_update_group() instead
> ---
>  drivers/pci/pci-sysfs.c | 59 +++++++++++++++++------------------------
>  1 file changed, 25 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 6b1a0ae254d3a..462a90d13eb87 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1059,37 +1059,6 @@ void pci_remove_legacy_files(struct pci_bus *b)
>  }
>  #endif /* HAVE_PCI_LEGACY */
>  
> -/**
> - * pci_create_boot_display_file - create a file in sysfs for @dev
> - * @pdev: dev in question
> - *
> - * Creates a file `boot_display` in sysfs for the PCI device @pdev
> - * if it is the boot display device.
> - */
> -static int pci_create_boot_display_file(struct pci_dev *pdev)
> -{
> -#ifdef CONFIG_VIDEO
> -	if (video_is_primary_device(&pdev->dev))
> -		return sysfs_create_file(&pdev->dev.kobj, &dev_attr_boot_display.attr);
> -#endif
> -	return 0;
> -}
> -
> -/**
> - * pci_remove_boot_display_file - remove the boot display file for @dev
> - * @pdev: dev in question
> - *
> - * Removes the file `boot_display` in sysfs for the PCI device @pdev
> - * if it is the boot display device.
> - */
> -static void pci_remove_boot_display_file(struct pci_dev *pdev)
> -{
> -#ifdef CONFIG_VIDEO
> -	if (video_is_primary_device(&pdev->dev))
> -		sysfs_remove_file(&pdev->dev.kobj, &dev_attr_boot_display.attr);
> -#endif
> -}
> -
>  #if defined(HAVE_PCI_MMAP) || defined(ARCH_GENERIC_PCI_MMAP_RESOURCE)
>  /**
>   * pci_mmap_resource - map a PCI resource into user memory space
> @@ -1691,6 +1660,29 @@ static const struct attribute_group pci_dev_resource_resize_group = {
>  	.is_visible = resource_resize_is_visible,
>  };
>  
> +static struct attribute *pci_display_attrs[] = {
> +	&dev_attr_boot_display.attr,
> +	NULL,

Trivial but I'd drop the comma after the NULL. It's at terminating entry and
we don't want it to be easy to add stuff after it!

> +};
> +
> +static umode_t pci_boot_display_visible(struct kobject *kobj,
> +					struct attribute *a, int n)
> +{
> +#ifdef CONFIG_VIDEO
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +
> +	if (a == &dev_attr_boot_display.attr && video_is_primary_device(&pdev->dev))
Is video_is_primary_device() stubbed out on !CONFIG_VIDEO?

There seems to be a stub in include/asm-generic/video.h

If it always is, drop the ifdef guards whilst you are here as the compiler
can see that and remove the code anyway.


> +		return a->mode;
> +#endif
> +	return 0;
> +}
> +
> +static const struct attribute_group pci_display_attr_group = {
> +	.attrs = pci_display_attrs,
> +	.is_visible = pci_boot_display_visible,
> +};
> +
>  int __must_check pci_create_sysfs_dev_files(struct pci_dev *pdev)
>  {
>  	int retval;
> @@ -1698,7 +1690,7 @@ int __must_check pci_create_sysfs_dev_files(struct pci_dev *pdev)
>  	if (!sysfs_initialized)
>  		return -EACCES;
>  
> -	retval = pci_create_boot_display_file(pdev);
> +	retval = sysfs_update_group(&pdev->dev.kobj, &pci_display_attr_group);
>  	if (retval)
>  		return retval;
>  
> @@ -1716,7 +1708,6 @@ void pci_remove_sysfs_dev_files(struct pci_dev *pdev)
>  	if (!sysfs_initialized)
>  		return;
>  
> -	pci_remove_boot_display_file(pdev);
>  	pci_remove_resource_files(pdev);
>  }
>  
> @@ -1755,7 +1746,6 @@ static umode_t pci_dev_attrs_are_visible(struct kobject *kobj,
>  
>  	if (a == &dev_attr_boot_vga.attr && pci_is_vga(pdev))
>  		return a->mode;
> -
Unrelated change.

>  	return 0;
>  }
>  
> @@ -1845,6 +1835,7 @@ static const struct attribute_group pcie_dev_attr_group = {
>  
>  const struct attribute_group *pci_dev_attr_groups[] = {
>  	&pci_dev_attr_group,
> +	&pci_display_attr_group,
>  	&pci_dev_hp_attr_group,
>  #ifdef CONFIG_PCI_IOV
>  	&sriov_pf_dev_attr_group,

Re: [PATCH v2] PCI: Adjust visibility of boot_display attribute instead of creation

[Mario Limonciello]

On 7/21/25 2:15 AM, Manivannan Sadhasivam wrote:
> On Sun, Jul 20, 2025 at 05:34:44PM GMT, Lukas Wunner wrote:
>> On Sun, Jul 20, 2025 at 10:15:08AM -0500, Mario Limonciello wrote:
>>> There is a desire to avoid creating new sysfs files late, so instead
>>> of dynamically deciding to create the boot_display attribute, make
>>> it static and use sysfs_update_group() to adjust visibility on the
>>> applicable devices.
>> [...]
>>> @@ -1698,7 +1690,7 @@ int __must_check pci_create_sysfs_dev_files(struct pci_dev *pdev)
>>>   	if (!sysfs_initialized)
>>>   		return -EACCES;
>>>   
>>> -	retval = pci_create_boot_display_file(pdev);
>>> +	retval = sysfs_update_group(&pdev->dev.kobj, &pci_display_attr_group);
>>>   	if (retval)
>>>   		return retval;
>>>
>>
>> pci_create_sysfs_dev_files() is called from two places, pci_bus_add_device()
>> and the late_initcall pci_sysfs_init().
>>
> 
> Separate question: Do we really need to call pci_create_sysfs_dev_files() from
> pci_sysfs_init()? It is already getting called from pci_bus_add_device() and it
> looks redundant to me. More importantly, it can also lead to a race (though
> won't happen practically) [1].
> 
> Same goes for pci_proc_attach_device(). I was tempted to submit a patch removing
> both these calls from pci_sysfs_init() and pci_proc_init(), but wanted to check
> first.
> 
> [1] https://lore.kernel.org/linux-pci/r7fgb5xrn6ocstq6ctq4q7r4o2esgh4rqr44c3u234kcep6thk@bge2vzl33ptb/
> 
> - Mani
> 

Thanks!  It's totally valid.
Lukas raised the same concern.  I have respun as a v3 with a fix.

https://lore.kernel.org/linux-pci/20250721023818.2410062-1-superm1@kernel.org/T/#u

Thanks,

Re: [PATCH v2] PCI: Adjust visibility of boot_display attribute instead of creation

[Mario Limonciello]

On 7/21/25 4:57 AM, Jonathan Cameron wrote:
> On Sun, 20 Jul 2025 10:15:08 -0500
> Mario Limonciello <superm1@kernel.org> wrote:
> 
>> From: Mario Limonciello <mario.limonciello@amd.com>
>>
>> There is a desire to avoid creating new sysfs files late, so instead
>> of dynamically deciding to create the boot_display attribute, make
>> it static and use sysfs_update_group() to adjust visibility on the
>> applicable devices.
>>
>> This also fixes a compilation warning when compiled without
>> CONFIG_VIDEO on sparc.
>>
>> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
>> Closes: https://lore.kernel.org/linux-next/20250718224118.5b3f22b0@canb.auug.org.au/
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> 
> Leaving aside the 'where to call it' discussions, a few bits of feedback on the
> code. See inline.
> 

Thanks!

> 
>> ---
>> v2:
>>   * Change to sysfs_update_group() instead
>> ---
>>   drivers/pci/pci-sysfs.c | 59 +++++++++++++++++------------------------
>>   1 file changed, 25 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
>> index 6b1a0ae254d3a..462a90d13eb87 100644
>> --- a/drivers/pci/pci-sysfs.c
>> +++ b/drivers/pci/pci-sysfs.c
>> @@ -1059,37 +1059,6 @@ void pci_remove_legacy_files(struct pci_bus *b)
>>   }
>>   #endif /* HAVE_PCI_LEGACY */
>>   
>> -/**
>> - * pci_create_boot_display_file - create a file in sysfs for @dev
>> - * @pdev: dev in question
>> - *
>> - * Creates a file `boot_display` in sysfs for the PCI device @pdev
>> - * if it is the boot display device.
>> - */
>> -static int pci_create_boot_display_file(struct pci_dev *pdev)
>> -{
>> -#ifdef CONFIG_VIDEO
>> -	if (video_is_primary_device(&pdev->dev))
>> -		return sysfs_create_file(&pdev->dev.kobj, &dev_attr_boot_display.attr);
>> -#endif
>> -	return 0;
>> -}
>> -
>> -/**
>> - * pci_remove_boot_display_file - remove the boot display file for @dev
>> - * @pdev: dev in question
>> - *
>> - * Removes the file `boot_display` in sysfs for the PCI device @pdev
>> - * if it is the boot display device.
>> - */
>> -static void pci_remove_boot_display_file(struct pci_dev *pdev)
>> -{
>> -#ifdef CONFIG_VIDEO
>> -	if (video_is_primary_device(&pdev->dev))
>> -		sysfs_remove_file(&pdev->dev.kobj, &dev_attr_boot_display.attr);
>> -#endif
>> -}
>> -
>>   #if defined(HAVE_PCI_MMAP) || defined(ARCH_GENERIC_PCI_MMAP_RESOURCE)
>>   /**
>>    * pci_mmap_resource - map a PCI resource into user memory space
>> @@ -1691,6 +1660,29 @@ static const struct attribute_group pci_dev_resource_resize_group = {
>>   	.is_visible = resource_resize_is_visible,
>>   };
>>   
>> +static struct attribute *pci_display_attrs[] = {
>> +	&dev_attr_boot_display.attr,
>> +	NULL,
> 
> Trivial but I'd drop the comma after the NULL. It's at terminating entry and
> we don't want it to be easy to add stuff after it!

Thanks!
Sounds sensible.  If I spin a v4 keeping it in pci-sysfs.c I'll drop it.

> 
>> +};
>> +
>> +static umode_t pci_boot_display_visible(struct kobject *kobj,
>> +					struct attribute *a, int n)
>> +{
>> +#ifdef CONFIG_VIDEO
>> +	struct device *dev = kobj_to_dev(kobj);
>> +	struct pci_dev *pdev = to_pci_dev(dev);
>> +
>> +	if (a == &dev_attr_boot_display.attr && video_is_primary_device(&pdev->dev))
> Is video_is_primary_device() stubbed out on !CONFIG_VIDEO?
> 
> There seems to be a stub in include/asm-generic/video.h
> 
> If it always is, drop the ifdef guards whilst you are here as the compiler
> can see that and remove the code anyway.
> 

I feel like there was a reason for this from earlier in the versions of 
the series introducing boot_display, but with how things are now I think 
you're right.

I'll do some test builds without CONFIG_VIDEO set to convince myself.

> 
>> +		return a->mode;
>> +#endif
>> +	return 0;
>> +}
>> +
>> +static const struct attribute_group pci_display_attr_group = {
>> +	.attrs = pci_display_attrs,
>> +	.is_visible = pci_boot_display_visible,
>> +};
>> +
>>   int __must_check pci_create_sysfs_dev_files(struct pci_dev *pdev)
>>   {
>>   	int retval;
>> @@ -1698,7 +1690,7 @@ int __must_check pci_create_sysfs_dev_files(struct pci_dev *pdev)
>>   	if (!sysfs_initialized)
>>   		return -EACCES;
>>   
>> -	retval = pci_create_boot_display_file(pdev);
>> +	retval = sysfs_update_group(&pdev->dev.kobj, &pci_display_attr_group);
>>   	if (retval)
>>   		return retval;
>>   
>> @@ -1716,7 +1708,6 @@ void pci_remove_sysfs_dev_files(struct pci_dev *pdev)
>>   	if (!sysfs_initialized)
>>   		return;
>>   
>> -	pci_remove_boot_display_file(pdev);
>>   	pci_remove_resource_files(pdev);
>>   }
>>   
>> @@ -1755,7 +1746,6 @@ static umode_t pci_dev_attrs_are_visible(struct kobject *kobj,
>>   
>>   	if (a == &dev_attr_boot_vga.attr && pci_is_vga(pdev))
>>   		return a->mode;
>> -
> Unrelated change.
Thanks, that snuck in when I was rearranging and I totally missed it.
I dropped it in v3.

> 
>>   	return 0;
>>   }
>>   
>> @@ -1845,6 +1835,7 @@ static const struct attribute_group pcie_dev_attr_group = {
>>   
>>   const struct attribute_group *pci_dev_attr_groups[] = {
>>   	&pci_dev_attr_group,
>> +	&pci_display_attr_group,
>>   	&pci_dev_hp_attr_group,
>>   #ifdef CONFIG_PCI_IOV
>>   	&sriov_pf_dev_attr_group,
> 

Re: [External] Re: [PATCH v2] PCI: Adjust visibility of boot_display attribute instead of creation

[Shuan He]

Hi Mani,

>Separate question: Do we really need to call pci_create_sysfs_dev_files() from
>pci_sysfs_init()? It is already getting called from pci_bus_add_device() and it
>looks redundant to me. More importantly, it can also lead to a race (though
>won't happen practically) [1].

>Same goes for pci_proc_attach_device(). I was tempted to submit a patch removing
>both these calls from pci_sysfs_init() and pci_proc_init(), but wanted to check
>first.
Thanks for pointing this out.
Yes, please do this! I was also considering deleting those unprotected calls,
pci_proc_attach_device and pci_create_sysfs_dev_files respectively.
Just wondering what needs to be checked before removing them.

Plus, now I am testing the following changes on my developing environment and
it works well so far.

--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1722,18 +1722,9 @@ void pci_remove_sysfs_dev_files(struct pci_dev *pdev)
 static int __init pci_sysfs_init(void)
 {
-       struct pci_dev *pdev = NULL;
        struct pci_bus *pbus = NULL;
-       int retval;
        sysfs_initialized = 1;
-       for_each_pci_dev(pdev) {
-               retval = pci_create_sysfs_dev_files(pdev);
-               if (retval) {
-                       pci_dev_put(pdev);
-                       return retval;
-               }
-       }
        while ((pbus = pci_find_next_bus(pbus)))
                pci_create_legacy_files(pbus);
diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
index 9348a0fb8084..b78286afe18e 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;
 }

Bests,
Shuan


On Mon, Jul 21, 2025 at 3:16 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
>
> On Sun, Jul 20, 2025 at 05:34:44PM GMT, Lukas Wunner wrote:
> > On Sun, Jul 20, 2025 at 10:15:08AM -0500, Mario Limonciello wrote:
> > > There is a desire to avoid creating new sysfs files late, so instead
> > > of dynamically deciding to create the boot_display attribute, make
> > > it static and use sysfs_update_group() to adjust visibility on the
> > > applicable devices.
> > [...]
> > > @@ -1698,7 +1690,7 @@ int __must_check pci_create_sysfs_dev_files(struct pci_dev *pdev)
> > >     if (!sysfs_initialized)
> > >             return -EACCES;
> > >
> > > -   retval = pci_create_boot_display_file(pdev);
> > > +   retval = sysfs_update_group(&pdev->dev.kobj, &pci_display_attr_group);
> > >     if (retval)
> > >             return retval;
> > >
> >
> > pci_create_sysfs_dev_files() is called from two places, pci_bus_add_device()
> > and the late_initcall pci_sysfs_init().
> >
>
> Separate question: Do we really need to call pci_create_sysfs_dev_files() from
> pci_sysfs_init()? It is already getting called from pci_bus_add_device() and it
> looks redundant to me. More importantly, it can also lead to a race (though
> won't happen practically) [1].
>
> Same goes for pci_proc_attach_device(). I was tempted to submit a patch removing
> both these calls from pci_sysfs_init() and pci_proc_init(), but wanted to check
> first.
>
> [1] https://lore.kernel.org/linux-pci/r7fgb5xrn6ocstq6ctq4q7r4o2esgh4rqr44c3u234kcep6thk@bge2vzl33ptb/
>
> - Mani
>
> --
> மணிவண்ணன் சதாசிவம்
>