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 > --- > 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); >