Re: [PATCH v4 07/24] PCI/sysfs: Convert PCI resource files to static attributes

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3133 bytes --]

On Fri, 17 Apr 2026, Krzysztof Wilczyński wrote:

> Hello,
> 
> > > -	/* 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;
> [...]
> > 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 have not.  Sorry for late reply here.
> 
> When testing resource_assigned() as replacement for pci_resource_len(),
> none of the resource files would be made visible.
> 
> The resource_assigned() checks res->parent, but the static .is_bin_visible
> callback runs at device_add() time, called from pci_device_add() during bus
> enumeration, so before pci_assign_unassigned_bus_resources() runs and inserts
> resources into the resource tree via pci_claim_resource(), etc.
> 
> The call sequence in pci_host_probe() is:
> 
>   pci_scan_root_bus_bridge()
>     pci_scan_child_bus()
>       pci_scan_slot()
>         pci_scan_single_device()
>           pci_scan_device()
>             pci_setup_device()
>               pci_read_bases()        <- res->start/end set from BARs
>           pci_device_add()
>             device_add()              <- is_bin_visible() runs here
>                                          res->parent still NULL
> 
>   pci_assign_unassigned_root_bus_resources()
>     pci_claim_resource()
>       request_resource_conflict()
>         __request_resource()          <- res->parent set here
> 
> At that point, the pci_resource_len() would return a non-zero value as
> res->start and res->end would already be set, but the res->parent is
> still NULL (not yet assigned to tree).
> 
> The old dynamic code ran from pci_sysfs_init() as a late_initcall (after
> assignment), where resource_assigned() would have worked.
> 
> As such, we can't really use resource_assigned() together with static
> sysfs attributes, at least not without solving the resources evaluation
> order here.
> 
> Thank you!

Okay, understood (I already saw you coverletter as well).

This however implies there's no guarantee the resource space range is even 
available for the BAR if the check is not based on ->parent. (Hit a 
problem like this kind of highlights how using pci_resource_len() is 
definitely hacky.)

I suppose it would be more proper to always recalculate the sysfs
visibilities after resources have been rearranged (released or assigned).
But as with the BAR resize, there's the race window when releasing BARs
so solving it may be non-trivial to get the ordering right (without mass
removing sysfs files prior to running the resource fitting and assignment
algorithm which doesn't seem wise either).

So to conclude, IMO, this series is a major improvement to state of
things as is and solving this pci_resource_len() vs resource_assigned() 
within the context of this series doesn't seem worth the extra delay and 
complexity. Thus, I'm fine with using pci_resource_len() for now.

-- 
 i.