Re: [PATCH 05/11] fs/resctrl: Use accurate type for rdt_resource::rid

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Tony,

On 3/3/26 11:54 AM, Luck, Tony wrote:
> On Tue, Mar 03, 2026 at 11:06:52AM -0800, Reinette Chatre wrote:
>> Hi Tony,
>>> +int rdt_num_resources = ARRAY_SIZE(rdt_resources_all);
>>> +
>>
>> ... and this proposes to let the *architecture* initialize how many
>> resources resctrl fs supports?
> 
> Not exactly. The file system is free to support as many resources as
> it wants to.
> 
> The architecture just provides the largest value that will provide
> any useful result from resctrl_arch_get_resource() so that this filesystem macro
> works without redundant iterations for unimplemented resources at the high
> end of the enum range:

I am seeing three different values being considered:
- The number of resources supported by resctrl fs.
- The number of resources supported by architecture.
- The maximum resource id supported by architecture.

At this time RDT_NUM_RESOURCES is used for all three on x86 but these three
numbers can be different.

If I understand correctly you are not actually proposing to "Replace the
RDT_NUM_RESOURCES #define with a variable initialized to
ARRAY_SIZE(rdt_resources_all)." but instead you are proposing
to drop RDT_NUM_RESOURCES entirely and introduce a new variable that 
captures the maximum resource ID supported by the architecture (which can
be different from the number of resources supported by the architecture).

> /* Walk all possible resources, with variants for only controls or monitors. */
> #define for_each_rdt_resource(_r)						\
> 	for ((_r) = resctrl_arch_get_resource(0);				\
> 	     (_r) && (_r)->rid < rdt_num_resources;				\
> 	     (_r) = resctrl_arch_get_resource((_r)->rid + 1))

Letting rdt_num_resources be the maximum resource ID supported by the architecture
will not avoid that resctrl_arch_get_resource() be called for a resource that is
not supported by the architecture nor does it avoid resctrl fs calling 
resctrl_arch_get_resource() for resource IDs larger than the maximum set by
the architecture.

Consider all the other places where resctrl fs call resctrl_arch_get_resource()
directly. There are several places where resctrl_arch_get_resource() is called
directly for a resource. Introducing a new max that architecture can set with
the expectation that  resctrl_arch_get_resource() will not be called for an id
larger than that is not correct and a solution that implements such contract
between resctrl fs and the architecture needs to look further than the
for_each_rdt_resource() loop.

>> This implies that all architectures need to initialize this on behalf of
>> resctrl fs. resctrl fs does not force an architecture to use an array nor does
>> it require an architecture to support all resources. What if an architecture
>> decides to not use an array and does not support all the resources resctrl fs
>> supports? How should it initialize rdt_num_resources?
> 
> Yes. Each architecture would have to provide a value. Each architecture
> must support the resctrl_arch_get_resource(enum resctrl_res_level l)
> function. Regardless of whether this is backed up with an array of
> resources, a list, or some other exotic structure this function must
> return a valid "struct rdt_resource *" pointer that can be dereferenced
> for all values [0 ... RDT_NUM_RESOURCES).

To support the loop the architecture only needs to provide valid pointers
(with an initialized rdt_resource::rid) up to the maximum resource ID supported
by it. This proposal does not change this requirement.

The expectation is that for any resource known to resctrl the architecture
will return a "not-capable" resource. For a comprehensive solution resctrl
fs should either never call resctrl_arch_get_resource() on a non-capable
resource or be able to handle NULL returned from any resctrl_arch_get_resource()
at all other call sites.

This may be where this proposal is headed but in its current form it
creates a fragmented contract between resctrl fs and the architecture.

> Allowing the architecture to define the upper limit of supported resource
> numbers doesn't constrain the file system.

What is the purpose of defining an upper limit when file system still
expects architecture to handle requests that exceed the limit?

>> I see the number of resources supported by resctrl fs as a resctrl fs property,
>> not something it should depend on the architecture to initialize.
> 
> Having resctrl fs define the number means that an architecture that uses
> an array must pad out that array.

Not for the for_each_rdt_resource() loop but it does still require padding the
unsupported resources with IDs less than the max that this proposal does not address.
Will need to survey resctrl for the other call sites that are not covered by this
proposal anyway. If the goal is to avoid padding then the solution needs to be
more comprehensive.

Reinette