On Sat, 11 Apr 2026, Krzysztof Wilczyński wrote:

> Currently, the PCI resource files (resourceN, resourceN_wc) are
> dynamically created by pci_create_sysfs_dev_files(), called from
> both pci_bus_add_device() and the pci_sysfs_init() late_initcall,
> with only a sysfs_initialized flag for synchronisation.  This has
> caused "duplicate filename" warnings and boot panics when both
> paths race on the same device.
> 
> This is especially likely on Devicetree-based platforms, where the
> PCI host controllers are platform drivers that probe via the driver
> model, which can happen during or after the late_initcall.  As such,
> pci_bus_add_device() and pci_sysfs_init() are more likely to overlap.
> 
> Thus, convert to static const attributes with three attribute groups
> (I/O, UC, WC), each with an .is_bin_visible callback that checks
> resource flags, BAR length, and non_mappable_bars.  A .bin_size
> callback provides pci_resource_len() to the kernfs node for correct
> stat and lseek behaviour.
> 
> As part of this conversion:
> 
>   - Rename pci_read_resource_io() and pci_write_resource_io() to
>     pci_read_resource() and pci_write_resource() since the callbacks
>     are no longer I/O-specific in the static attribute context.
> 
>   - Remove pci_create_resource_files(), pci_remove_resource_files(),
>     and pci_create_attr() which are no longer needed.
> 
>   - Move the __weak stubs outside the #if guard so they remain
>     available for callers converted in subsequent commits.
> 
> Platforms that do not define the HAVE_PCI_MMAP macro or the
> ARCH_GENERIC_PCI_MMAP_RESOURCE macro, such as Alpha architecture,
> continue using their platform-specific resource file creation.
> 
> For reference, the dynamic creation dates back to the pre-Git era:
> 
>   
> https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/drivers/pci/pci-sysfs.c?id=42298be0eeb5ae98453b3374c36161b05a46c5dc
> 
> The write-combine support was added in commit 45aec1ae72fc ("x86: PAT
> export resource_wc in pci sysfs").
> 
> Signed-off-by: Krzysztof Wilczyński <[email protected]>
> ---
>  drivers/pci/pci-sysfs.c | 242 +++++++++++++++++++++-------------------
>  include/linux/pci.h     |   2 -
>  2 files changed, 127 insertions(+), 117 deletions(-)
> 
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index ca0074674acc..2922905ff070 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1200,14 +1200,14 @@ static ssize_t pci_resource_io(struct file *filp, 
> struct kobject *kobj,
>  #endif
>  }
>  
> -static ssize_t pci_read_resource_io(struct file *filp, struct kobject *kobj,
> +static ssize_t pci_read_resource(struct file *filp, struct kobject *kobj,
>                                   const struct bin_attribute *attr, char *buf,
>                                   loff_t off, size_t count)
>  {
>       return pci_resource_io(filp, kobj, attr, buf, off, count, false);
>  }
>  
> -static ssize_t pci_write_resource_io(struct file *filp, struct kobject *kobj,
> +static ssize_t pci_write_resource(struct file *filp, struct kobject *kobj,
>                                    const struct bin_attribute *attr, char 
> *buf,
>                                    loff_t off, size_t count)
>  {
> @@ -1261,129 +1261,136 @@ static const struct bin_attribute 
> dev_resource##_bar##_wc_attr = {          \
>       .mmap = pci_mmap_resource_wc,                                           
> \
>  }
>  
> -/**
> - * pci_remove_resource_files - cleanup resource files
> - * @pdev: dev to cleanup
> - *
> - * If we created resource files for @pdev, remove them from sysfs and
> - * free their resources.
> - */
> -static void pci_remove_resource_files(struct pci_dev *pdev)
> +static inline umode_t
> +__pci_resource_attr_is_visible(struct kobject *kobj,
> +                            const struct bin_attribute *a,
> +                            int bar, bool write_combine,
> +                            unsigned long flags)
>  {
> -     int i;
> +     struct pci_dev *pdev = to_pci_dev(kobj_to_dev(kobj));
>  
> -     for (i = 0; i < PCI_STD_NUM_BARS; i++) {
> -             struct bin_attribute *res_attr;
> -
> -             res_attr = pdev->res_attr[i];
> -             if (res_attr) {
> -                     sysfs_remove_bin_file(&pdev->dev.kobj, res_attr);
> -                     kfree(res_attr);
> -             }
> -
> -             res_attr = pdev->res_attr_wc[i];
> -             if (res_attr) {
> -                     sysfs_remove_bin_file(&pdev->dev.kobj, res_attr);
> -                     kfree(res_attr);
> -             }
> -     }
> -}
> -
> -static int pci_create_attr(struct pci_dev *pdev, int num, int write_combine)
> -{
> -     /* allocate attribute structure, piggyback attribute name */
> -     int name_len = write_combine ? 13 : 10;
> -     struct bin_attribute *res_attr;
> -     char *res_attr_name;
> -     int retval;
> -
> -     res_attr = kzalloc(sizeof(*res_attr) + name_len, GFP_ATOMIC);
> -     if (!res_attr)
> -             return -ENOMEM;
> -
> -     res_attr_name = (char *)(res_attr + 1);
> -
> -     sysfs_bin_attr_init(res_attr);
> -     if (write_combine) {
> -             sprintf(res_attr_name, "resource%d_wc", num);
> -             res_attr->mmap = pci_mmap_resource_wc;
> -     } else {
> -             sprintf(res_attr_name, "resource%d", num);
> -             if (pci_resource_flags(pdev, num) & IORESOURCE_IO) {
> -                     res_attr->read = pci_read_resource_io;
> -                     res_attr->write = pci_write_resource_io;
> -                     if (arch_can_pci_mmap_io())
> -                             res_attr->mmap = pci_mmap_resource_uc;
> -             } else {
> -                     res_attr->mmap = pci_mmap_resource_uc;
> -             }
> -     }
> -     if (res_attr->mmap) {
> -             res_attr->f_mapping = iomem_get_mapping;
> -             /*
> -              * generic_file_llseek() consults f_mapping->host to determine
> -              * the file size. As iomem_inode knows nothing about the
> -              * attribute, it's not going to work, so override it as well.
> -              */
> -             res_attr->llseek = pci_llseek_resource;
> -     }
> -     res_attr->attr.name = res_attr_name;
> -     res_attr->attr.mode = 0600;
> -     res_attr->size = pci_resource_len(pdev, num);
> -     res_attr->private = (void *)(unsigned long)num;
> -     retval = sysfs_create_bin_file(&pdev->dev.kobj, res_attr);
> -     if (retval) {
> -             kfree(res_attr);
> -             return retval;
> -     }
> -
> -     if (write_combine)
> -             pdev->res_attr_wc[num] = res_attr;
> -     else
> -             pdev->res_attr[num] = res_attr;
> -
> -     return 0;
> -}
> -
> -/**
> - * pci_create_resource_files - create resource files in sysfs for @dev
> - * @pdev: dev in question
> - *
> - * Walk the resources in @pdev creating files for each resource available.
> - */
> -static int pci_create_resource_files(struct pci_dev *pdev)
> -{
> -     int i;
> -     int retval;
> -
> -     /* Skip devices with non-mappable BARs */
>       if (pdev->non_mappable_bars)
>               return 0;
>  
> -     /* Expose the PCI resources from this device as files */
> -     for (i = 0; i < PCI_STD_NUM_BARS; i++) {
> +     if (!pci_resource_len(pdev, bar))
> +             return 0;

Hi,

Did you accidently forget to address some of the comments as I thought you 
were agreeing to changing this to resource_assigned() but I found no 
resource_assigned() from entire series?

-- 
 i.

>  
> -             /* skip empty resources */
> -             if (!pci_resource_len(pdev, i))
> -                     continue;
> +     if ((pci_resource_flags(pdev, bar) & flags) != flags)
> +             return 0;
>  
> -             retval = pci_create_attr(pdev, i, 0);
> -             /* for prefetchable resources, create a WC mappable file */
> -             if (!retval && arch_can_pci_mmap_wc() &&
> -                 pci_resource_flags(pdev, i) & IORESOURCE_PREFETCH)
> -                     retval = pci_create_attr(pdev, i, 1);
> -             if (retval) {
> -                     pci_remove_resource_files(pdev);
> -                     return retval;
> -             }
> -     }
> -     return 0;
> +     if (write_combine && !arch_can_pci_mmap_wc())
> +             return 0;
> +
> +     return a->attr.mode;
>  }
> -#else /* !(defined(HAVE_PCI_MMAP) || 
> defined(ARCH_GENERIC_PCI_MMAP_RESOURCE)) */
> -int __weak pci_create_resource_files(struct pci_dev *dev) { return 0; }
> -void __weak pci_remove_resource_files(struct pci_dev *dev) { return; }
> +
> +static umode_t pci_dev_resource_io_is_visible(struct kobject *kobj,
> +                                           const struct bin_attribute *a,
> +                                           int n)
> +{
> +     return __pci_resource_attr_is_visible(kobj, a, n, false,
> +                                           IORESOURCE_IO);
> +}
> +
> +static umode_t pci_dev_resource_uc_is_visible(struct kobject *kobj,
> +                                           const struct bin_attribute *a,
> +                                           int n)
> +{
> +     return __pci_resource_attr_is_visible(kobj, a, n, false,
> +                                           IORESOURCE_MEM);
> +}
> +
> +static umode_t pci_dev_resource_wc_is_visible(struct kobject *kobj,
> +                                           const struct bin_attribute *a,
> +                                           int n)
> +{
> +     return __pci_resource_attr_is_visible(kobj, a, n, true,
> +                                           IORESOURCE_MEM | 
> IORESOURCE_PREFETCH);
> +}
> +
> +static size_t pci_dev_resource_bin_size(struct kobject *kobj,
> +                                     const struct bin_attribute *a,
> +                                     int n)
> +{
> +     struct pci_dev *pdev = to_pci_dev(kobj_to_dev(kobj));
> +
> +     return pci_resource_len(pdev, n);
> +}
> +
> +pci_dev_resource_io_attr(0);
> +pci_dev_resource_io_attr(1);
> +pci_dev_resource_io_attr(2);
> +pci_dev_resource_io_attr(3);
> +pci_dev_resource_io_attr(4);
> +pci_dev_resource_io_attr(5);
> +
> +pci_dev_resource_uc_attr(0);
> +pci_dev_resource_uc_attr(1);
> +pci_dev_resource_uc_attr(2);
> +pci_dev_resource_uc_attr(3);
> +pci_dev_resource_uc_attr(4);
> +pci_dev_resource_uc_attr(5);
> +
> +pci_dev_resource_wc_attr(0);
> +pci_dev_resource_wc_attr(1);
> +pci_dev_resource_wc_attr(2);
> +pci_dev_resource_wc_attr(3);
> +pci_dev_resource_wc_attr(4);
> +pci_dev_resource_wc_attr(5);
> +
> +static const struct bin_attribute *const pci_dev_resource_io_attrs[] = {
> +     &dev_resource0_io_attr,
> +     &dev_resource1_io_attr,
> +     &dev_resource2_io_attr,
> +     &dev_resource3_io_attr,
> +     &dev_resource4_io_attr,
> +     &dev_resource5_io_attr,
> +     NULL,
> +};
> +
> +static const struct bin_attribute *const pci_dev_resource_uc_attrs[] = {
> +     &dev_resource0_uc_attr,
> +     &dev_resource1_uc_attr,
> +     &dev_resource2_uc_attr,
> +     &dev_resource3_uc_attr,
> +     &dev_resource4_uc_attr,
> +     &dev_resource5_uc_attr,
> +     NULL,
> +};
> +
> +static const struct bin_attribute *const pci_dev_resource_wc_attrs[] = {
> +     &dev_resource0_wc_attr,
> +     &dev_resource1_wc_attr,
> +     &dev_resource2_wc_attr,
> +     &dev_resource3_wc_attr,
> +     &dev_resource4_wc_attr,
> +     &dev_resource5_wc_attr,
> +     NULL,
> +};
> +
> +static const struct attribute_group pci_dev_resource_io_attr_group = {
> +     .bin_attrs = pci_dev_resource_io_attrs,
> +     .is_bin_visible = pci_dev_resource_io_is_visible,
> +     .bin_size = pci_dev_resource_bin_size,
> +};
> +
> +static const struct attribute_group pci_dev_resource_uc_attr_group = {
> +     .bin_attrs = pci_dev_resource_uc_attrs,
> +     .is_bin_visible = pci_dev_resource_uc_is_visible,
> +     .bin_size = pci_dev_resource_bin_size,
> +};
> +
> +static const struct attribute_group pci_dev_resource_wc_attr_group = {
> +     .bin_attrs = pci_dev_resource_wc_attrs,
> +     .is_bin_visible = pci_dev_resource_wc_is_visible,
> +     .bin_size = pci_dev_resource_bin_size,
> +};
> +
>  #endif
>  
> +int __weak pci_create_resource_files(struct pci_dev *dev) { return 0; }
> +void __weak pci_remove_resource_files(struct pci_dev *dev) { }
> +
>  /**
>   * pci_write_rom - used to enable access to the PCI ROM display
>   * @filp: sysfs file
> @@ -1861,6 +1868,11 @@ static const struct attribute_group pci_dev_group = {
>  
>  const struct attribute_group *pci_dev_groups[] = {
>       &pci_dev_group,
> +#if defined(HAVE_PCI_MMAP) || defined(ARCH_GENERIC_PCI_MMAP_RESOURCE)
> +     &pci_dev_resource_io_attr_group,
> +     &pci_dev_resource_uc_attr_group,
> +     &pci_dev_resource_wc_attr_group,
> +#endif
>       &pci_dev_config_attr_group,
>       &pci_dev_rom_attr_group,
>       &pci_dev_reset_attr_group,
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 05aceb7f1f37..9c0782899ef9 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -2526,10 +2526,8 @@ int pcibios_alloc_irq(struct pci_dev *dev);
>  void pcibios_free_irq(struct pci_dev *dev);
>  resource_size_t pcibios_default_alignment(void);
>  
> -#if !defined(HAVE_PCI_MMAP) && !defined(ARCH_GENERIC_PCI_MMAP_RESOURCE)
>  extern int pci_create_resource_files(struct pci_dev *dev);
>  extern void pci_remove_resource_files(struct pci_dev *dev);
> -#endif
>  
>  #if defined(CONFIG_PCI_MMCONFIG) || defined(CONFIG_ACPI_MCFG)
>  void __init pci_mmcfg_early_init(void);
> 

Reply via email to