Re: [PATCH 2/4] selftests/resctrl: SNC support for CMT

[Ilpo Järvinen]

On Fri, 8 Mar 2024, Ilpo Järvinen wrote:

> On Wed, 6 Mar 2024, Maciej Wieczor-Retman wrote:
> 
> > Cache Monitoring Technology (CMT) works by measuring how much data in L3
> > cache is occupied by a given process identified by its Resource
> > Monitoring ID (RMID).
> > 
> > On systems with Sub-Numa Clusters (SNC) enabled, a process can occupy
> > not only the cache that belongs to its own NUMA node but also pieces of
> > other NUMA nodes' caches that lie on the same socket.
> > 
> > A simple correction to make the CMT selftest NUMA-aware is to sum values
> > reported by all nodes on the same socket for a given RMID.
> > 
> > Reported-by: "Shaopeng Tan (Fujitsu)" <tan.shaopeng@xxxxxxxxxxx>
> > Closes: https://lore.kernel.org/all/TYAPR01MB6330B9B17686EF426D2C3F308B25A@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
> > Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@xxxxxxxxx>
> > ---

> > @@ -828,6 +828,8 @@ int resctrl_val(const struct resctrl_test *test,
> >  	sleep(1);
> >  
> >  	/* Test runs until the callback setup() tells the test to stop. */
> > +	get_domain_id("L3", uparams->cpu, &res_id);
> 
> Hardcoding L3 here limits the genericness of this function. You don't even 
> need to do it, get_domain_id() does "MB" -> "L3" transformation implicitly 
> for you so you can just pass test->resource instead.
> 
> Also, I don't understand why you now again make the naming inconsistent 
> with "res_id".
> 
> If you based this on top of the patches I just posted, resctl_val() 
> already the domain_id variable.

Ah, I retract what I said. I see you actually want it only from L3.

> > +     res_id *= snc_ways();

I don't understand what this is trying to achieve and how.

-- 
 i.