RE: [PATCH v18 06/17] x86/resctrl: Introduce snc_nodes_per_l3_cache

Luck, Tony
2024-05-23T19:05:10Z

> > return (is_mbm_local_enabled() &&
> > - r->alloc_capable && is_mba_linear());
> > + r->alloc_capable && is_mba_linear() &&
> > + snc_nodes_per_l3_cache == 1);
> > }
> >
> > /*
>
> Since the software controller is a filesystem feature the above
> now requires that snc_nodes_per_l3_cache becomes part of the resctrl
> filesystem code and every architecture will need to set snc_nodes_per_l3_cache.
> Every architecture will thus need to interpret what "SNC" means for them
> using the term introduced here. That may be ok ... but the term "SNC"
> will then surely not identify an Intel feature and Intel needs to be ok
> that any architecture calls their "similar to SNC but not quite identical"
> "SNC".
>
> I assume now that as part of the fs/arch split there needs to be
> a new helper that allows different architectures to set this
> filesystem variable?

I can change this check to better reflect the underlying reason to
disable the software controller. Which is that the MBM monitor scope
does not match the MBA control scope. This seems like an architecture
neutral expression.

So code would look like this:

struct rdt_resource *rmbm = &rdt_resources_all[RDT_RESOURCE_L3].r_rescrl;
struct rdt_resource *rmba = &rdt_resources_all[RDT_RESOURCE_MBA].r_rescrl;

...

return (is_mbm_local_enabled() &&
r->alloc_capable && is_mba_linear() &&
rmbm->mon_scope == rmba->ctrl_scope);

I'm also contemplating dropping snc_nodes_per_l3_cache from being a
global variable and making it a field in "struct rdt_resource" (only needed
for the RDT_RESOURCE_L3 resource). N.B. Babu had suggested it
shouldn't be global many patch versions ago.

Perhaps name it .domains_per_l3_cache or .subdomains_per_l3_cache?

Bad idea? Good idea (but you have a better name for the field)?

-Tony