| 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
| ||
|
Message-ID: <eac72c7b-bd01-084e-cf23-ec5e8e0e3cf3@intel.com>
Date: Fri, 21 Apr 2023 17:32:20 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
<linux-kselftest@...r.kernel.org>,
Fenghua Yu <fenghua.yu@...el.com>,
"Shuah Khan" <shuah@...nel.org>, <linux-kernel@...r.kernel.org>
CC: Shaopeng Tan <tan.shaopeng@...fujitsu.com>
Subject: Re: [PATCH v2 24/24] selftests/resctrl: Rewrite Cache Allocation
Technology (CAT) test
Hi Ilpo,
On 4/18/2023 4:45 AM, Ilpo Järvinen wrote:
> CAT test spawns two processes into two different control groups with
> exclusive schemata. Both the processes alloc a buffer from memory
> matching their allocated LLC block size and flush the entire buffer out
> of caches. Since the processes are reading through the buffer only once
> during the measurement and initially all the buffer was flushed, the
> test isn't testing CAT.
>
> Rewrite the CAT test to allocated a buffer sized to half of LLC. Then
"allocated a buffer" -> "allocate a buffer" ?
> perform a sequence of tests with different LLC alloc sizes starting
> from half of the CBM bits down to 1-bit CBM. Flush the buffer before
> each test and read the buffer twice. Observe the LLC misses on the
> second read through the buffer. As the allocated LLC block gets smaller
> and smaller, the LLC misses will become larger and larger giving a
> strong signal on CAT working properly.
Since the changelog starts by describing the CAT test needing two
processes I think it would help to highlight that this test uses a
single process. I think it would also help to describing how the cache
is used by the rest while this test is running.
>
> Suggested-by: Reinette Chatre <reinette.chatre@...el.com>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
> ---
> tools/testing/selftests/resctrl/cache.c | 20 +-
> tools/testing/selftests/resctrl/cat_test.c | 204 +++++++++------------
> 2 files changed, 97 insertions(+), 127 deletions(-)
>
> diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c
> index 7970239413da..64f08ba5edc2 100644
> --- a/tools/testing/selftests/resctrl/cache.c
> +++ b/tools/testing/selftests/resctrl/cache.c
> @@ -224,10 +224,10 @@ int measure_llc_resctrl(struct resctrl_val_param *param, int bm_pid)
> */
> int cat_val(struct resctrl_val_param *param)
> {
> - int memflush = 1, operation = 0, ret = 0;
> char *resctrl_val = param->resctrl_val;
> unsigned long llc_perf_miss = 0;
> pid_t bm_pid;
> + int ret;
>
> if (strcmp(param->filename, "") == 0)
> sprintf(param->filename, "stdio");
> @@ -245,6 +245,10 @@ int cat_val(struct resctrl_val_param *param)
> if (ret)
> return ret;
>
> + ret = alloc_buffer(param->span, 1);
> + if (ret)
> + return ret;
> +
> initialize_llc_perf();
>
> /* Test runs until the callback setup() tells the test to stop. */
> @@ -256,17 +260,15 @@ int cat_val(struct resctrl_val_param *param)
> }
> if (ret < 0)
> break;
> +
> + flush_buffer(param->span);
> + use_buffer(param->span, 0, true);
> +
> ret = reset_enable_llc_perf(bm_pid, param->cpu_no);
> if (ret)
> break;
>
> - if (run_fill_buf(param->span, memflush, operation, true)) {
> - fprintf(stderr, "Error-running fill buffer\n");
> - ret = -1;
> - break;
> - }
> -
> - sleep(1);
> + use_buffer(param->span, 0, true);
>
> /* Measure cache miss from perf */
> ret = get_llc_perf(&llc_perf_miss);
> @@ -279,6 +281,8 @@ int cat_val(struct resctrl_val_param *param)
> break;
> }
>
> + free_buffer();
> +
> return ret;
> }
>
> diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
> index 4b505fdb35d7..85053829b9c5 100644
> --- a/tools/testing/selftests/resctrl/cat_test.c
> +++ b/tools/testing/selftests/resctrl/cat_test.c
> @@ -11,11 +11,12 @@
> #include "resctrl.h"
> #include <unistd.h>
>
> -#define RESULT_FILE_NAME1 "result_cat1"
> -#define RESULT_FILE_NAME2 "result_cat2"
> -#define NUM_OF_RUNS 5
> -#define MAX_DIFF_PERCENT 4
> -#define MAX_DIFF 1000000
> +#define RESULT_FILE_NAME "result_cat"
> +#define NUM_OF_RUNS 5
> +#define MIN_DIFF_PERCENT_PER_BIT 2
Could you please start a new trend that adds documentation
that explains what this constant means and how it was chosen?
> +
> +static unsigned long current_mask;
> +static long prev_avg_llc_val;
>
> /*
> * Change schemata. Write schemata to specified
> @@ -28,13 +29,24 @@ static int cat_setup(struct resctrl_val_param *p)
> int ret = 0;
>
> /* Run NUM_OF_RUNS times */
> - if (p->num_of_runs >= NUM_OF_RUNS)
> - return END_OF_TESTS;
> + if (p->num_of_runs >= NUM_OF_RUNS) {
> + /* Remove one bit from the consecutive block */
> + current_mask &= current_mask >> 1;
> + if (!current_mask)
> + return END_OF_TESTS;
> +
> + p->num_of_runs = 0;
This seems like a workaround to get the schemata to be written. It is
problematic since now p->num_of_runs no longer accurately reflects the
number of test runs. I was expecting this mask manipulation to be
in cat_val() so that it is clear how test works instead of part
of the logic handled here.
> + }
>
> if (p->num_of_runs == 0) {
> - sprintf(schemata, "%lx", p->mask);
> - ret = write_schemata(p->ctrlgrp, schemata, p->cpu_no,
> - p->resctrl_val);
> + snprintf(schemata, sizeof(schemata), "%lx", p->mask & ~current_mask);
> + ret = write_schemata("", schemata, p->cpu_no, p->resctrl_val);
> + if (ret)
> + return ret;
> + snprintf(schemata, sizeof(schemata), "%lx", current_mask);
> + ret = write_schemata(p->ctrlgrp, schemata, p->cpu_no, p->resctrl_val);
> + if (ret)
> + return ret;
> }
> p->num_of_runs++;
>
...
> @@ -126,7 +162,7 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
> ret = get_mask_no_shareable(cache_type, &long_mask);
> if (ret)
> return ret;
> - count_of_bits = count_consecutive_bits(long_mask, NULL);
> + count_of_bits = count_consecutive_bits(long_mask, &start);
>
> /* Get L3/L2 cache size */
> ret = get_cache_size(cpu_no, cache_type, &cache_size);
> @@ -143,99 +179,29 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
> count_of_bits - 1);
> return -1;
> }
> -
> - /* Get core id from same socket for running another thread */
> - sibling_cpu_no = get_core_sibling(cpu_no);
Do any users of get_core_sibling() remain after this?
Reinette
Powered by blists - more mailing lists