Re: [PATCH v5 17/40] x86/resctrl: Rewrite and move the for_each_*_rdt_resource() walkers

From: Tony Luck
Date: Mon Oct 07 2024 - 20:00:28 EST


On Fri, Oct 04, 2024 at 06:03:24PM +0000, James Morse wrote:
> The for_each_*_rdt_resource() helpers walk the architecture's array
> of structures, using the resctrl visible part as an iterator. These
> became over-complex when the structures were split into a
> filesystem and architecture-specific struct. This approach avoided
> the need to touch every call site, and was done before there was a
> helper to retrieve a resource by rid.
>
> Once the filesystem parts of resctrl are moved to /fs/, both the
> architecture's resource array, and the definition of those structures
> is no longer accessible. To support resctrl, each architecture would
> have to provide equally complex macros.
>
> Rewrite the macro to make use of resctrl_arch_get_resource(), and
> move these to the core header so existing x86 arch code continues
> to use them.

Apologies if this comment was suggested against earlier versions
of this series.

Did you consider replacing rdt_resources_all[] a list (in the filesystem
code) instead of an array (in the architecture code)?

List would start empty. Architecture init code would enumerate features
and add entries to the list for those that exist and are to be enabled.

The "for_each" macros then walk the list (variants for all entries,
for "alloc_capable" and for "mon_capable"). Note that only enabled
entries appear on the lists.

There are currently a bunch of places in filesystem code that
do:
r = resctrl_arch_get_resource(RDT_RESOURCE_MBA);
or
r = resctrl_arch_get_resource(RDT_RESOURCE_L3);

those could become:

r = resctrl_arch_get_mba_resource();

r = resctrl_arch_get_l3_resource();

Then the whole "enum resctrl_res_level" and ->rid field in
struct rdt_resource could go away? Remaining uses look like
distinguishing MBA from SMBA. Perhaps better done with a
flags word?

Advantage of doing this would be to avoid the generic
enum resctrl_res_level having to be a superset of all
features across all architectures. E.g. ARM might want
to add L4/L5 resources, X86 may have some that ARM will
never need. RiscV may also follow some divergent path.

If this v5 series is close to being applied then I don't
want to derail with a re-write at this late stage.
All of this could be done as a cleanup after this series
has been applied.

-Tony