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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5983d1aa-6e1e-fb49-b95a-429a9decfe7f@linux.intel.com>
Date: Fri, 8 Mar 2024 15:59:02 +0200 (EET)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Maciej Wieczor-Retman <maciej.wieczor-retman@...el.com>
cc: Fenghua Yu <fenghua.yu@...el.com>, 
    Reinette Chatre <reinette.chatre@...el.com>, Shuah Khan <shuah@...nel.org>, 
    tony.luck@...el.com, "Shaopeng Tan (Fujitsu)" <tan.shaopeng@...itsu.com>, 
    LKML <linux-kernel@...r.kernel.org>, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH 2/4] selftests/resctrl: SNC support for CMT

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@...itsu.com>
> > Closes: https://lore.kernel.org/all/TYAPR01MB6330B9B17686EF426D2C3F308B25A@TYAPR01MB6330.jpnprd01.prod.outlook.com/
> > Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@...el.com>
> > ---

> > @@ -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.