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)" <[email protected]>
> Closes:
> https://lore.kernel.org/all/tyapr01mb6330b9b17686ef426d2c3f308b...@tyapr01mb6330.jpnprd01.prod.outlook.com/
> Signed-off-by: Maciej Wieczor-Retman <[email protected]>
> ---
> tools/testing/selftests/resctrl/cache.c | 17 +++++++++++------
> tools/testing/selftests/resctrl/resctrl.h | 4 +++-
> tools/testing/selftests/resctrl/resctrl_val.c | 9 ++++++---
> 3 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/tools/testing/selftests/resctrl/cache.c
> b/tools/testing/selftests/resctrl/cache.c
> index 1b339d6bbff1..dab81920033b 100644
> --- a/tools/testing/selftests/resctrl/cache.c
> +++ b/tools/testing/selftests/resctrl/cache.c
> @@ -161,16 +161,21 @@ int perf_event_measure(int pe_fd, struct
> perf_event_read *pe_read,
> *
> * Return: =0 on success. <0 on failure.
> */
> -int measure_llc_resctrl(const char *filename, int bm_pid)
> +int measure_llc_resctrl(const char *filename, int bm_pid, const char
> *ctrlgrp,
> + const char *mongrp, int res_id)
> {
> - unsigned long llc_occu_resc = 0;
> + unsigned long sum = 0, llc_occu_resc = 0;
> int ret;
>
> - ret = get_llc_occu_resctrl(&llc_occu_resc);
> - if (ret < 0)
> - return ret;
> + for (int i = 0 ; i < snc_ways() ; i++) {
Spaces as per usual coding style:
for (int i = 0; i < snc_ways(); i++) {
> + set_cmt_path(ctrlgrp, mongrp, res_id + i);
> + ret = get_llc_occu_resctrl(&llc_occu_resc);
> + if (ret < 0)
> + return ret;
> + sum += llc_occu_resc;
> + }
>
> - return print_results_cache(filename, bm_pid, llc_occu_resc);
> + return print_results_cache(filename, bm_pid, sum);
> }
>
> /*
> @@ -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.
--
i.
> + res_id *= snc_ways();
> while (1) {
> ret = param->setup(test, uparams, param);
> if (ret == END_OF_TESTS) {
> @@ -844,7 +846,8 @@ int resctrl_val(const struct resctrl_test *test,
> break;
> } else if (!strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR))) {
> sleep(1);
> - ret = measure_llc_resctrl(param->filename, bm_pid);
> + ret = measure_llc_resctrl(param->filename, bm_pid,
> param->ctrlgrp,
> + param->mongrp, res_id);
> if (ret)
> break;
> }
>
