2023-12-15 15:14:13

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH v4 00/29] selftests/resctrl: CAT test improvements & generalized test framework

Hi all,

Here's v4 series to improve resctrl selftests with generalized test
framework and rewritten CAT test.

The series contains following improvements:

- Excludes shareable bits from CAT test allocation to avoid interference
- Replaces file "sink" with a volatile variable
- Alters read pattern to defeat HW prefetcher optimizations
- Rewrites CAT test to make the CAT test reliable and truly measure
if CAT is working or not
- Introduces generalized test framework making easier to add new tests
- Lots of other cleanups & refactoring

This series has been tested across a large number of systems from
different generations.

v4:
- Reworded a few error prints
- Changelog improvements
- fprintf()'s error handling changed ksft_perror() -> ksft_print_msg()
- Keep using ksft_*() instead of fprintf() in get_bit_mask()
- Check against div-by-zero
- Adjust one return type

v3:
- New patches to handle return errno, perror() and return value comments
- Tweak changelogs
- Moved error printout removal to other patch
- Zero bit CBM returns error
- Tweak comments
- Make get_shareable_mask() static
- Return directly without storing result into ret variable first
- llc -> LLC
- Altered changelog and removed "the whole time" wording because
llc occu results are still unsigned long
- Altered changelog's wording to not say "a volatile pointer"
- Make min_diff_percent and MIN_DIFF_PERCENT_PER_BIT unsigned long
- Add patch to restore CPU affinity after CAT test
- Move uparams clear into init function
- Add CPU vendor ID bitmask comment
- Use test_resource_feature_check(test) in CMT
- "feature" -> "resource" in function comment

v2:
- Postpone adding L2 CAT test as more investigations are necessary
- Add patch to remove ctrlc_handler() from wrong place
- Improvements to changelogs
- Function comments improvements & comment cleanups
- Move some parts of the changes into more logical patch
- If checks: buf == NULL -> !buf
- Variable naming:
- p -> buf
- cbm_mask_path -> cbm_path
- Function naming:
- get_cbm_mask() -> get_full_cbm()
- cache_size() -> cache_portion_size()
- Use PATH_MAX
- Improved cache_portion_size() parameter names
- int count -> unsigned int
- Pass filename to measurement taking functions instead of
resctrl_val_param
- !lines ? : reversal
- Removed bogus static from function local variable
- Open perf fd only once, reset & enable in the innermost test loop
- Add perf fd ioctl() error handling
- Add patch to change compiler optimization prevention "sink" from file
to volatile variable
- Remove cpu_no and resource (the latter was added in v1) members from
resctrl_val_param (pass uparams and test where those are needed)
- Removed ARRAY_SIZE() macro
- Add patch to rename "resource_id" to "domain_id"


Ilpo Järvinen (29):
selftests/resctrl: Convert perror() to ksft_perror() or
ksft_print_msg()
selftests/resctrl: Return -1 instead of errno on error
selftests/resctrl: Don't use ctrlc_handler() outside signal handling
selftests/resctrl: Change function comments to say < 0 on error
selftests/resctrl: Split fill_buf to allow tests finer-grained control
selftests/resctrl: Refactor fill_buf functions
selftests/resctrl: Refactor get_cbm_mask() and rename to
get_full_cbm()
selftests/resctrl: Mark get_cache_size() cache_type const
selftests/resctrl: Create cache_portion_size() helper
selftests/resctrl: Exclude shareable bits from schemata in CAT test
selftests/resctrl: Split measure_cache_vals()
selftests/resctrl: Split show_cache_info() to test specific and
generic parts
selftests/resctrl: Remove unnecessary __u64 -> unsigned long
conversion
selftests/resctrl: Remove nested calls in perf event handling
selftests/resctrl: Consolidate naming of perf event related things
selftests/resctrl: Improve perf init
selftests/resctrl: Convert perf related globals to locals
selftests/resctrl: Move cat_val() to cat_test.c and rename to
cat_test()
selftests/resctrl: Open perf fd before start & add error handling
selftests/resctrl: Replace file write with volatile variable
selftests/resctrl: Read in less obvious order to defeat prefetch
optimizations
selftests/resctrl: Rewrite Cache Allocation Technology (CAT) test
selftests/resctrl: Restore the CPU affinity after CAT test
selftests/resctrl: Create struct for input parameters
selftests/resctrl: Introduce generalized test framework
selftests/resctrl: Pass write_schemata() resource instead of test name
selftests/resctrl: Add helper to convert L2/3 to integer
selftests/resctrl: Rename resource ID to domain ID
selftests/resctrl: Get domain id from cache id

tools/testing/selftests/resctrl/cache.c | 287 +++++----------
tools/testing/selftests/resctrl/cat_test.c | 337 +++++++++++-------
tools/testing/selftests/resctrl/cmt_test.c | 80 +++--
tools/testing/selftests/resctrl/fill_buf.c | 132 ++++---
tools/testing/selftests/resctrl/mba_test.c | 30 +-
tools/testing/selftests/resctrl/mbm_test.c | 32 +-
tools/testing/selftests/resctrl/resctrl.h | 135 +++++--
.../testing/selftests/resctrl/resctrl_tests.c | 197 ++++------
tools/testing/selftests/resctrl/resctrl_val.c | 138 +++----
tools/testing/selftests/resctrl/resctrlfs.c | 321 +++++++++++------
10 files changed, 945 insertions(+), 744 deletions(-)

--
2.30.2



2023-12-15 15:14:15

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH v4 29/29] selftests/resctrl: Get domain id from cache id

Domain id is acquired differently depending on CPU. AMD tests use id
from L3 cache, whereas CPUs from other vendors base the id on topology
package id. In order to support L2 CAT test, this has to be
generalized.

The driver side code seems to get the domain ids from cache ids so the
approach used by the AMD branch seems to match the kernel-side code. It
will also work with L2 domain IDs as long as the cache level is
generalized.

Using the topology id was always fragile due to mismatch with the
kernel-side way to acquire the domain id. It got incorrect domain id,
e.g., when Cluster-on-Die (CoD) is enabled for CPU (but CoD is not well
suited for resctrl in the first place so it has not been a big issue if
tests don't work correctly with it).

Taking all the above into account, generalize acquiring the domain id
by taking it from the cache id and do not hard-code the cache level.

Signed-off-by: Ilpo Järvinen <[email protected]>
Reviewed-by: Reinette Chatre <[email protected]>
---
tools/testing/selftests/resctrl/resctrl.h | 2 +-
tools/testing/selftests/resctrl/resctrl_val.c | 4 +--
tools/testing/selftests/resctrl/resctrlfs.c | 27 ++++++++++++-------
3 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index d4eef20723fc..c52eaf46f24d 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -129,7 +129,7 @@ extern char llc_occup_path[1024];
int get_vendor(void);
bool check_resctrlfs_support(void);
int filter_dmesg(void);
-int get_domain_id(int cpu_no, int *domain_id);
+int get_domain_id(const char *resource, int cpu_no, int *domain_id);
int mount_resctrlfs(void);
int umount_resctrlfs(void);
int validate_bw_report_request(char *bw_report);
diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
index 631e5f055694..5a49f07a6c85 100644
--- a/tools/testing/selftests/resctrl/resctrl_val.c
+++ b/tools/testing/selftests/resctrl/resctrl_val.c
@@ -415,7 +415,7 @@ static void initialize_mem_bw_resctrl(const char *ctrlgrp, const char *mongrp,
{
int domain_id;

- if (get_domain_id(cpu_no, &domain_id) < 0) {
+ if (get_domain_id("MB", cpu_no, &domain_id) < 0) {
ksft_print_msg("Could not get domain ID\n");
return;
}
@@ -584,7 +584,7 @@ static void initialize_llc_occu_resctrl(const char *ctrlgrp, const char *mongrp,
{
int domain_id;

- if (get_domain_id(cpu_no, &domain_id) < 0) {
+ if (get_domain_id("L3", cpu_no, &domain_id) < 0) {
ksft_print_msg("Could not get domain ID\n");
return;
}
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
index f29dc65d8b30..5750662cce57 100644
--- a/tools/testing/selftests/resctrl/resctrlfs.c
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -111,28 +111,37 @@ static int get_cache_level(const char *cache_type)
return -1;
}

+static int get_resource_cache_level(const char *resource)
+{
+ /* "MB" use L3 (LLC) as resource */
+ if (!strcmp(resource, "MB"))
+ return 3;
+ return get_cache_level(resource);
+}
+
/*
* get_domain_id - Get resctrl domain ID for a specified CPU
+ * @resource: resource name
* @cpu_no: CPU number
* @domain_id: domain ID (cache ID; for MB, L3 cache ID)
*
* Return: >= 0 on success, < 0 on failure.
*/
-int get_domain_id(int cpu_no, int *domain_id)
+int get_domain_id(const char *resource, int cpu_no, int *domain_id)
{
char phys_pkg_path[1024];
+ int cache_num;
FILE *fp;

- if (get_vendor() == ARCH_AMD)
- sprintf(phys_pkg_path, "%s%d/cache/index3/id",
- PHYS_ID_PATH, cpu_no);
- else
- sprintf(phys_pkg_path, "%s%d/topology/physical_package_id",
- PHYS_ID_PATH, cpu_no);
+ cache_num = get_resource_cache_level(resource);
+ if (cache_num < 0)
+ return cache_num;
+
+ sprintf(phys_pkg_path, "%s%d/cache/index%d/id", PHYS_ID_PATH, cpu_no, cache_num);

fp = fopen(phys_pkg_path, "r");
if (!fp) {
- ksft_perror("Failed to open physical_package_id");
+ ksft_perror("Failed to open cache id file");

return -1;
}
@@ -559,7 +568,7 @@ int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, const char *resour
return -1;
}

- if (get_domain_id(cpu_no, &domain_id) < 0) {
+ if (get_domain_id(resource, cpu_no, &domain_id) < 0) {
sprintf(reason, "Failed to get domain ID");
ret = -1;

--
2.30.2


2023-12-15 15:14:22

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH v4 28/29] selftests/resctrl: Rename resource ID to domain ID

Kernel-side calls the instances of a resource domains.

Change the resource_id naming in the selftest code to domain_id to
match the kernel side better.

Suggested-by: Maciej Wieczór-Retman <[email protected]>
Signed-off-by: Ilpo Järvinen <[email protected]>
Reviewed-by: Reinette Chatre <[email protected]>
---
tools/testing/selftests/resctrl/resctrl.h | 2 +-
tools/testing/selftests/resctrl/resctrl_val.c | 30 +++++++++----------
tools/testing/selftests/resctrl/resctrlfs.c | 18 +++++------
3 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index 97d16daf8190..d4eef20723fc 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -129,7 +129,7 @@ extern char llc_occup_path[1024];
int get_vendor(void);
bool check_resctrlfs_support(void);
int filter_dmesg(void);
-int get_resource_id(int cpu_no, int *resource_id);
+int get_domain_id(int cpu_no, int *domain_id);
int mount_resctrlfs(void);
int umount_resctrlfs(void);
int validate_bw_report_request(char *bw_report);
diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
index 16ad91ccbcd3..631e5f055694 100644
--- a/tools/testing/selftests/resctrl/resctrl_val.c
+++ b/tools/testing/selftests/resctrl/resctrl_val.c
@@ -387,20 +387,20 @@ static int get_mem_bw_imc(int cpu_no, char *bw_report, float *bw_imc)
return 0;
}

-void set_mbm_path(const char *ctrlgrp, const char *mongrp, int resource_id)
+void set_mbm_path(const char *ctrlgrp, const char *mongrp, int domain_id)
{
if (ctrlgrp && mongrp)
sprintf(mbm_total_path, CON_MON_MBM_LOCAL_BYTES_PATH,
- RESCTRL_PATH, ctrlgrp, mongrp, resource_id);
+ RESCTRL_PATH, ctrlgrp, mongrp, domain_id);
else if (!ctrlgrp && mongrp)
sprintf(mbm_total_path, MON_MBM_LOCAL_BYTES_PATH, RESCTRL_PATH,
- mongrp, resource_id);
+ mongrp, domain_id);
else if (ctrlgrp && !mongrp)
sprintf(mbm_total_path, CON_MBM_LOCAL_BYTES_PATH, RESCTRL_PATH,
- ctrlgrp, resource_id);
+ ctrlgrp, domain_id);
else if (!ctrlgrp && !mongrp)
sprintf(mbm_total_path, MBM_LOCAL_BYTES_PATH, RESCTRL_PATH,
- resource_id);
+ domain_id);
}

/*
@@ -413,23 +413,23 @@ void set_mbm_path(const char *ctrlgrp, const char *mongrp, int resource_id)
static void initialize_mem_bw_resctrl(const char *ctrlgrp, const char *mongrp,
int cpu_no, char *resctrl_val)
{
- int resource_id;
+ int domain_id;

- if (get_resource_id(cpu_no, &resource_id) < 0) {
- ksft_print_msg("Could not get resource_id\n");
+ if (get_domain_id(cpu_no, &domain_id) < 0) {
+ ksft_print_msg("Could not get domain ID\n");
return;
}

if (!strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)))
- set_mbm_path(ctrlgrp, mongrp, resource_id);
+ set_mbm_path(ctrlgrp, mongrp, domain_id);

if (!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR))) {
if (ctrlgrp)
sprintf(mbm_total_path, CON_MBM_LOCAL_BYTES_PATH,
- RESCTRL_PATH, ctrlgrp, resource_id);
+ RESCTRL_PATH, ctrlgrp, domain_id);
else
sprintf(mbm_total_path, MBM_LOCAL_BYTES_PATH,
- RESCTRL_PATH, resource_id);
+ RESCTRL_PATH, domain_id);
}
}

@@ -582,15 +582,15 @@ static void set_cmt_path(const char *ctrlgrp, const char *mongrp, char sock_num)
static void initialize_llc_occu_resctrl(const char *ctrlgrp, const char *mongrp,
int cpu_no, char *resctrl_val)
{
- int resource_id;
+ int domain_id;

- if (get_resource_id(cpu_no, &resource_id) < 0) {
- ksft_print_msg("Could not get resource_id\n");
+ if (get_domain_id(cpu_no, &domain_id) < 0) {
+ ksft_print_msg("Could not get domain ID\n");
return;
}

if (!strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR)))
- set_cmt_path(ctrlgrp, mongrp, resource_id);
+ set_cmt_path(ctrlgrp, mongrp, domain_id);
}

static int measure_vals(const struct user_params *uparams,
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
index eab928c46f98..f29dc65d8b30 100644
--- a/tools/testing/selftests/resctrl/resctrlfs.c
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -112,13 +112,13 @@ static int get_cache_level(const char *cache_type)
}

/*
- * get_resource_id - Get socket number/l3 id for a specified CPU
+ * get_domain_id - Get resctrl domain ID for a specified CPU
* @cpu_no: CPU number
- * @resource_id: Socket number or l3_id
+ * @domain_id: domain ID (cache ID; for MB, L3 cache ID)
*
* Return: >= 0 on success, < 0 on failure.
*/
-int get_resource_id(int cpu_no, int *resource_id)
+int get_domain_id(int cpu_no, int *domain_id)
{
char phys_pkg_path[1024];
FILE *fp;
@@ -136,8 +136,8 @@ int get_resource_id(int cpu_no, int *resource_id)

return -1;
}
- if (fscanf(fp, "%d", resource_id) <= 0) {
- ksft_perror("Could not get socket number or l3 id");
+ if (fscanf(fp, "%d", domain_id) <= 0) {
+ ksft_perror("Could not get domain ID");
fclose(fp);

return -1;
@@ -551,7 +551,7 @@ int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp,
int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, const char *resource)
{
char controlgroup[1024], reason[128], schema[1024] = {};
- int resource_id, fd, schema_len, ret = 0;
+ int domain_id, fd, schema_len, ret = 0;

if (!schemata) {
ksft_print_msg("Skipping empty schemata update\n");
@@ -559,8 +559,8 @@ int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, const char *resour
return -1;
}

- if (get_resource_id(cpu_no, &resource_id) < 0) {
- sprintf(reason, "Failed to get resource id");
+ if (get_domain_id(cpu_no, &domain_id) < 0) {
+ sprintf(reason, "Failed to get domain ID");
ret = -1;

goto out;
@@ -572,7 +572,7 @@ int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, const char *resour
sprintf(controlgroup, "%s/schemata", RESCTRL_PATH);

schema_len = snprintf(schema, sizeof(schema), "%s:%d=%s\n",
- resource, resource_id, schemata);
+ resource, domain_id, schemata);
if (schema_len < 0 || schema_len >= sizeof(schema)) {
snprintf(reason, sizeof(reason),
"snprintf() failed with return value : %d", schema_len);
--
2.30.2


2023-12-15 15:15:21

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH v4 26/29] selftests/resctrl: Pass write_schemata() resource instead of test name

write_schemata() takes the test name as an argument and determines the
relevant resource based on the test name. Such mapping from name to
resource does not really belong to resctrlfs.c that should provide
only generic, test-independent functions.

Pass the resource stored in the test information structure to
write_schemata() instead of the test name. The new API is also more
flexible as it enables to use write_schemata() for more than one
resource within a test.

While touching the sprintf(), move the unnecessary %c that is always
'=' directly into the format string.

Signed-off-by: Ilpo Järvinen <[email protected]>
Reviewed-by: Reinette Chatre <[email protected]>
---

v3:
- "feature" -> "resource" in function comment
---
tools/testing/selftests/resctrl/cat_test.c | 11 +++++----
tools/testing/selftests/resctrl/cmt_test.c | 6 +++--
tools/testing/selftests/resctrl/mba_test.c | 9 +++----
tools/testing/selftests/resctrl/mbm_test.c | 9 +++----
tools/testing/selftests/resctrl/resctrl.h | 10 ++++----
tools/testing/selftests/resctrl/resctrl_val.c | 7 ++++--
tools/testing/selftests/resctrl/resctrlfs.c | 24 +++++--------------
7 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
index a9b4583620d0..24af8310288a 100644
--- a/tools/testing/selftests/resctrl/cat_test.c
+++ b/tools/testing/selftests/resctrl/cat_test.c
@@ -135,6 +135,7 @@ void cat_test_cleanup(void)

/*
* cat_test - Execute CAT benchmark and measure cache misses
+ * @test: Test information structure
* @uparams: User supplied parameters
* @param: Parameters passed to cat_test()
* @span: Buffer size for the benchmark
@@ -152,7 +153,9 @@ void cat_test_cleanup(void)
*
* Return: 0 when the test was run, < 0 on error.
*/
-static int cat_test(const struct user_params *uparams, struct resctrl_val_param *param,
+static int cat_test(const struct resctrl_test *test,
+ const struct user_params *uparams,
+ struct resctrl_val_param *param,
size_t span, unsigned long current_mask)
{
char *resctrl_val = param->resctrl_val;
@@ -196,11 +199,11 @@ static int cat_test(const struct user_params *uparams, struct resctrl_val_param

while (current_mask) {
snprintf(schemata, sizeof(schemata), "%lx", param->mask & ~current_mask);
- ret = write_schemata("", schemata, uparams->cpu, param->resctrl_val);
+ ret = write_schemata("", schemata, uparams->cpu, test->resource);
if (ret)
goto free_buf;
snprintf(schemata, sizeof(schemata), "%lx", current_mask);
- ret = write_schemata(param->ctrlgrp, schemata, uparams->cpu, param->resctrl_val);
+ ret = write_schemata(param->ctrlgrp, schemata, uparams->cpu, test->resource);
if (ret)
goto free_buf;

@@ -279,7 +282,7 @@ static int cat_run_test(const struct resctrl_test *test, const struct user_param

remove(param.filename);

- ret = cat_test(uparams, &param, span, start_mask);
+ ret = cat_test(test, uparams, &param, span, start_mask);
if (ret)
goto out;

diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c
index c01980039118..dd5ca343c469 100644
--- a/tools/testing/selftests/resctrl/cmt_test.c
+++ b/tools/testing/selftests/resctrl/cmt_test.c
@@ -16,7 +16,9 @@
#define MAX_DIFF 2000000
#define MAX_DIFF_PERCENT 15

-static int cmt_setup(const struct user_params *uparams, struct resctrl_val_param *p)
+static int cmt_setup(const struct resctrl_test *test,
+ const struct user_params *uparams,
+ struct resctrl_val_param *p)
{
/* Run NUM_OF_RUNS times */
if (p->num_of_runs >= NUM_OF_RUNS)
@@ -150,7 +152,7 @@ static int cmt_run_test(const struct resctrl_test *test, const struct user_param

remove(RESULT_FILE_NAME);

- ret = resctrl_val(uparams, cmd, &param);
+ ret = resctrl_val(test, uparams, cmd, &param);
if (ret)
goto out;

diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c
index c218af24f91d..da256d2dbe5c 100644
--- a/tools/testing/selftests/resctrl/mba_test.c
+++ b/tools/testing/selftests/resctrl/mba_test.c
@@ -22,7 +22,9 @@
* con_mon grp, mon_grp in resctrl FS.
* For each allocation, run 5 times in order to get average values.
*/
-static int mba_setup(const struct user_params *uparams, struct resctrl_val_param *p)
+static int mba_setup(const struct resctrl_test *test,
+ const struct user_params *uparams,
+ struct resctrl_val_param *p)
{
static int runs_per_allocation, allocation = 100;
char allocation_str[64];
@@ -40,8 +42,7 @@ static int mba_setup(const struct user_params *uparams, struct resctrl_val_param

sprintf(allocation_str, "%d", allocation);

- ret = write_schemata(p->ctrlgrp, allocation_str, uparams->cpu,
- p->resctrl_val);
+ ret = write_schemata(p->ctrlgrp, allocation_str, uparams->cpu, test->resource);
if (ret < 0)
return ret;

@@ -155,7 +156,7 @@ static int mba_run_test(const struct resctrl_test *test, const struct user_param

remove(RESULT_FILE_NAME);

- ret = resctrl_val(uparams, uparams->benchmark_cmd, &param);
+ ret = resctrl_val(test, uparams, uparams->benchmark_cmd, &param);
if (ret)
goto out;

diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c
index 919b10459c22..34879e7b71a0 100644
--- a/tools/testing/selftests/resctrl/mbm_test.c
+++ b/tools/testing/selftests/resctrl/mbm_test.c
@@ -86,7 +86,9 @@ static int check_results(size_t span)
return ret;
}

-static int mbm_setup(const struct user_params *uparams, struct resctrl_val_param *p)
+static int mbm_setup(const struct resctrl_test *test,
+ const struct user_params *uparams,
+ struct resctrl_val_param *p)
{
int ret = 0;

@@ -96,8 +98,7 @@ static int mbm_setup(const struct user_params *uparams, struct resctrl_val_param

/* Set up shemata with 100% allocation on the first run. */
if (p->num_of_runs == 0 && validate_resctrl_feature_request("MB", NULL))
- ret = write_schemata(p->ctrlgrp, "100", uparams->cpu,
- p->resctrl_val);
+ ret = write_schemata(p->ctrlgrp, "100", uparams->cpu, test->resource);

p->num_of_runs++;

@@ -123,7 +124,7 @@ static int mbm_run_test(const struct resctrl_test *test, const struct user_param

remove(RESULT_FILE_NAME);

- ret = resctrl_val(uparams, uparams->benchmark_cmd, &param);
+ ret = resctrl_val(test, uparams, uparams->benchmark_cmd, &param);
if (ret)
goto out;

diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index 1168364b4cc2..97d16daf8190 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -98,7 +98,8 @@ struct resctrl_val_param {
char *bw_report;
unsigned long mask;
int num_of_runs;
- int (*setup)(const struct user_params *uparams,
+ int (*setup)(const struct resctrl_test *test,
+ const struct user_params *uparams,
struct resctrl_val_param *param);
};

@@ -137,8 +138,7 @@ bool test_resource_feature_check(const struct resctrl_test *test);
char *fgrep(FILE *inf, const char *str);
int taskset_benchmark(pid_t bm_pid, int cpu_no, cpu_set_t *old_affinity);
int taskset_restore(pid_t bm_pid, cpu_set_t *old_affinity);
-int write_schemata(char *ctrlgrp, char *schemata, int cpu_no,
- char *resctrl_val);
+int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, const char *resource);
int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp,
char *resctrl_val);
int perf_event_open(struct perf_event_attr *hw_event, pid_t pid, int cpu,
@@ -147,7 +147,9 @@ unsigned char *alloc_buffer(size_t buf_size, int memflush);
void mem_flush(unsigned char *buf, size_t buf_size);
void fill_cache_read(unsigned char *buf, size_t buf_size, bool once);
int run_fill_buf(size_t buf_size, int memflush, int op, bool once);
-int resctrl_val(const struct user_params *uparams, const char * const *benchmark_cmd,
+int resctrl_val(const struct resctrl_test *test,
+ const struct user_params *uparams,
+ const char * const *benchmark_cmd,
struct resctrl_val_param *param);
void tests_cleanup(void);
void mbm_test_cleanup(void);
diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
index 6d0a35e8bd02..16ad91ccbcd3 100644
--- a/tools/testing/selftests/resctrl/resctrl_val.c
+++ b/tools/testing/selftests/resctrl/resctrl_val.c
@@ -684,13 +684,16 @@ static void run_benchmark(int signum, siginfo_t *info, void *ucontext)
/*
* resctrl_val: execute benchmark and measure memory bandwidth on
* the benchmark
+ * @test: test information structure
* @uparams: user supplied parameters
* @benchmark_cmd: benchmark command and its arguments
* @param: parameters passed to resctrl_val()
*
* Return: 0 when the test was run, < 0 on error.
*/
-int resctrl_val(const struct user_params *uparams, const char * const *benchmark_cmd,
+int resctrl_val(const struct resctrl_test *test,
+ const struct user_params *uparams,
+ const char * const *benchmark_cmd,
struct resctrl_val_param *param)
{
char *resctrl_val = param->resctrl_val;
@@ -826,7 +829,7 @@ int resctrl_val(const struct user_params *uparams, const char * const *benchmark

/* Test runs until the callback setup() tells the test to stop. */
while (1) {
- ret = param->setup(uparams, param);
+ ret = param->setup(test, uparams, param);
if (ret == END_OF_TESTS) {
ret = 0;
break;
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
index 140f65467ddb..fed6741edc5f 100644
--- a/tools/testing/selftests/resctrl/resctrlfs.c
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -529,23 +529,17 @@ int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp,
* @ctrlgrp: Name of the con_mon grp
* @schemata: Schemata that should be updated to
* @cpu_no: CPU number that the benchmark PID is binded to
- * @resctrl_val: Resctrl feature (Eg: mbm, mba.. etc)
+ * @resource: Resctrl resource (Eg: MB, L3, L2, etc.)
*
- * Update schemata of a con_mon grp *only* if requested resctrl feature is
+ * Update schemata of a con_mon grp *only* if requested resctrl resource is
* allocation type
*
* Return: 0 on success, < 0 on error.
*/
-int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val)
+int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, const char *resource)
{
char controlgroup[1024], reason[128], schema[1024] = {};
- int resource_id, fd, schema_len = -1, ret = 0;
-
- if (strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)) &&
- strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)) &&
- strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR)) &&
- strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR)))
- return -ENOENT;
+ int resource_id, fd, schema_len, ret = 0;

if (!schemata) {
ksft_print_msg("Skipping empty schemata update\n");
@@ -565,14 +559,8 @@ int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val)
else
sprintf(controlgroup, "%s/schemata", RESCTRL_PATH);

- if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR)) ||
- !strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR)))
- schema_len = snprintf(schema, sizeof(schema), "%s%d%c%s\n",
- "L3:", resource_id, '=', schemata);
- if (!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)) ||
- !strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)))
- schema_len = snprintf(schema, sizeof(schema), "%s%d%c%s\n",
- "MB:", resource_id, '=', schemata);
+ schema_len = snprintf(schema, sizeof(schema), "%s:%d=%s\n",
+ resource, resource_id, schemata);
if (schema_len < 0 || schema_len >= sizeof(schema)) {
snprintf(reason, sizeof(reason),
"snprintf() failed with return value : %d", schema_len);
--
2.30.2


2023-12-15 15:15:49

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH v4 05/29] selftests/resctrl: Split fill_buf to allow tests finer-grained control

MBM, MBA and CMT test cases call run_fill_buf() that in turn calls
fill_cache() to alloc and loop indefinitely around the buffer. This
binds buffer allocation and running the benchmark into a single bundle
so that a selftest cannot allocate a buffer once and reuse it. CAT test
doesn't want to loop around the buffer continuously and after rewrite
it needs the ability to allocate the buffer separately.

Split buffer allocation out of fill_cache() into alloc_buffer(). This
change is part of preparation for the new CAT test that allocates a
buffer and does multiple passes over the same buffer (but not in an
infinite loop).

Co-developed-by: Fenghua Yu <[email protected]>
Signed-off-by: Fenghua Yu <[email protected]>
Signed-off-by: Ilpo Järvinen <[email protected]>
Reviewed-by: Reinette Chatre <[email protected]>
---

v3:
- Moved error printout removal to other patch
---
tools/testing/selftests/resctrl/fill_buf.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/resctrl/fill_buf.c b/tools/testing/selftests/resctrl/fill_buf.c
index 0f6cca61ec94..6d1d5eed595c 100644
--- a/tools/testing/selftests/resctrl/fill_buf.c
+++ b/tools/testing/selftests/resctrl/fill_buf.c
@@ -135,24 +135,34 @@ static int fill_cache_write(unsigned char *buf, size_t buf_size, bool once)
return 0;
}

-static int fill_cache(size_t buf_size, int memflush, int op, bool once)
+static unsigned char *alloc_buffer(size_t buf_size, int memflush)
{
unsigned char *buf;
- int ret;

buf = malloc_and_init_memory(buf_size);
if (!buf)
- return -1;
+ return NULL;

/* Flush the memory before using to avoid "cache hot pages" effect */
if (memflush)
mem_flush(buf, buf_size);

+ return buf;
+}
+
+static int fill_cache(size_t buf_size, int memflush, int op, bool once)
+{
+ unsigned char *buf;
+ int ret;
+
+ buf = alloc_buffer(buf_size, memflush);
+ if (!buf)
+ return -1;
+
if (op == 0)
ret = fill_cache_read(buf, buf_size, once);
else
ret = fill_cache_write(buf, buf_size, once);
-
free(buf);

if (ret) {
@@ -160,8 +170,7 @@ static int fill_cache(size_t buf_size, int memflush, int op, bool once)
return -1;
}

-
- return 0;
+ return ret;
}

int run_fill_buf(size_t span, int memflush, int op, bool once)
--
2.30.2


2023-12-15 15:16:02

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH v4 07/29] selftests/resctrl: Refactor get_cbm_mask() and rename to get_full_cbm()

Callers of get_cbm_mask() are required to pass a string into which the
capacity bitmask (CBM) is read. Neither CAT nor CMT tests need the
bitmask as string but just convert it into an unsigned long value.

Another limitation is that the bit mask reader can only read
.../cbm_mask files.

Generalize the bit mask reading function into get_bit_mask() such that
it can be used to handle other files besides the .../cbm_mask and
handles the unsigned long conversion within get_bit_mask() using
fscanf(). Change get_cbm_mask() to use get_bit_mask() and rename it to
get_full_cbm() to better indicate what the function does.

Return error from get_full_cbm() if the bitmask is zero for some reason
because it makes the code more robust as the selftests naturally assume
the bitmask has some bits.

Also mark cache_type const while at it and remove useless comments that
are related to processing of CBM bits.

Co-developed-by: Fenghua Yu <[email protected]>
Signed-off-by: Fenghua Yu <[email protected]>
Signed-off-by: Ilpo Järvinen <[email protected]>
---

v4:
- Convert from fprintf() to ksft_print_msg()

v3:
- Grammar fix changelog
- Zero bit mask returns error
---
tools/testing/selftests/resctrl/cat_test.c | 7 +--
tools/testing/selftests/resctrl/cmt_test.c | 5 +-
tools/testing/selftests/resctrl/resctrl.h | 2 +-
tools/testing/selftests/resctrl/resctrlfs.c | 51 +++++++++++++++------
4 files changed, 41 insertions(+), 24 deletions(-)

diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
index fabb56ff68d1..242c4c6200aa 100644
--- a/tools/testing/selftests/resctrl/cat_test.c
+++ b/tools/testing/selftests/resctrl/cat_test.c
@@ -93,25 +93,20 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
int ret, pipefd[2], sibling_cpu_no;
unsigned long cache_size = 0;
unsigned long long_mask;
- char cbm_mask[256];
int count_of_bits;
char pipe_message;
size_t span;

- /* Get default cbm mask for L3/L2 cache */
- ret = get_cbm_mask(cache_type, cbm_mask);
+ ret = get_full_cbm(cache_type, &long_mask);
if (ret)
return ret;

- long_mask = strtoul(cbm_mask, NULL, 16);
-
/* Get L3/L2 cache size */
ret = get_cache_size(cpu_no, cache_type, &cache_size);
if (ret)
return ret;
ksft_print_msg("Cache size :%lu\n", cache_size);

- /* Get max number of bits from default-cabm mask */
count_of_bits = count_bits(long_mask);

if (!n)
diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c
index ffd302bd5c73..a18c6825802c 100644
--- a/tools/testing/selftests/resctrl/cmt_test.c
+++ b/tools/testing/selftests/resctrl/cmt_test.c
@@ -75,17 +75,14 @@ int cmt_resctrl_val(int cpu_no, int n, const char * const *benchmark_cmd)
unsigned long cache_size = 0;
unsigned long long_mask;
char *span_str = NULL;
- char cbm_mask[256];
int count_of_bits;
size_t span;
int ret, i;

- ret = get_cbm_mask("L3", cbm_mask);
+ ret = get_full_cbm("L3", &long_mask);
if (ret)
return ret;

- long_mask = strtoul(cbm_mask, NULL, 16);
-
ret = get_cache_size(cpu_no, "L3", &cache_size);
if (ret)
return ret;
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index a848e9c75578..89cd89507891 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -98,7 +98,7 @@ void tests_cleanup(void);
void mbm_test_cleanup(void);
int mba_schemata_change(int cpu_no, const char * const *benchmark_cmd);
void mba_test_cleanup(void);
-int get_cbm_mask(char *cache_type, char *cbm_mask);
+int get_full_cbm(const char *cache_type, unsigned long *mask);
int get_cache_size(int cpu_no, char *cache_type, unsigned long *cache_size);
void ctrlc_handler(int signum, siginfo_t *info, void *ptr);
int signal_handler_register(void);
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
index 81d1e043e17a..a74be17eaa4e 100644
--- a/tools/testing/selftests/resctrl/resctrlfs.c
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -196,30 +196,29 @@ int get_cache_size(int cpu_no, char *cache_type, unsigned long *cache_size)
#define CORE_SIBLINGS_PATH "/sys/bus/cpu/devices/cpu"

/*
- * get_cbm_mask - Get cbm mask for given cache
- * @cache_type: Cache level L2/L3
- * @cbm_mask: cbm_mask returned as a string
+ * get_bit_mask - Get bit mask from given file
+ * @filename: File containing the mask
+ * @mask: The bit mask returned as unsigned long
*
* Return: = 0 on success, < 0 on failure.
*/
-int get_cbm_mask(char *cache_type, char *cbm_mask)
+static int get_bit_mask(const char *filename, unsigned long *mask)
{
- char cbm_mask_path[1024];
FILE *fp;

- if (!cbm_mask)
+ if (!filename || !mask)
return -1;

- sprintf(cbm_mask_path, "%s/%s/cbm_mask", INFO_PATH, cache_type);
-
- fp = fopen(cbm_mask_path, "r");
+ fp = fopen(filename, "r");
if (!fp) {
- ksft_perror("Failed to open cache level");
-
+ ksft_print_msg("Failed to open bit mask file '%s': %s\n",
+ filename, strerror(errno));
return -1;
}
- if (fscanf(fp, "%s", cbm_mask) <= 0) {
- ksft_perror("Could not get max cbm_mask");
+
+ if (fscanf(fp, "%lx", mask) <= 0) {
+ ksft_print_msg("Could not read bit mask file '%s': %s\n",
+ filename, strerror(errno));
fclose(fp);

return -1;
@@ -229,6 +228,32 @@ int get_cbm_mask(char *cache_type, char *cbm_mask)
return 0;
}

+/*
+ * get_full_cbm - Get full Cache Bit Mask (CBM)
+ * @cache_type: Cache type as "L2" or "L3"
+ * @mask: Full cache bit mask representing the maximal portion of cache
+ * available for allocation, returned as unsigned long.
+ *
+ * Return: = 0 on success, < 0 on failure.
+ */
+int get_full_cbm(const char *cache_type, unsigned long *mask)
+{
+ char cbm_path[PATH_MAX];
+ int ret;
+
+ if (!cache_type)
+ return -1;
+
+ snprintf(cbm_path, sizeof(cbm_path), "%s/%s/cbm_mask",
+ INFO_PATH, cache_type);
+
+ ret = get_bit_mask(cbm_path, mask);
+ if (ret || !*mask)
+ return -1;
+
+ return 0;
+}
+
/*
* get_core_sibling - Get sibling core id from the same socket for given CPU
* @cpu_no: CPU number
--
2.30.2


2023-12-15 15:16:04

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH v4 03/29] selftests/resctrl: Don't use ctrlc_handler() outside signal handling

perf_event_open_llc_miss() calls ctrlc_handler() to cleanup if
perf_event_open() returns an error. Those cleanups, however, are not
the responsibility of perf_event_open_llc_miss() and it thus interferes
unnecessarily with the usual cleanup pattern. Worse yet,
ctrlc_handler() calls exit() in the end preventing the ordinary cleanup
done in the calling function from executing.

ctrlc_handler() should only be used as a signal handler, not during
normal error handling.

Remove call to ctrlc_handler() from perf_event_open_llc_miss(). As
unmounting resctrlfs and test cleanup are already handled properly
by error rollbacks in the calling functions, no other changes are
necessary.

Suggested-by: Reinette Chatre <[email protected]>
Signed-off-by: Ilpo Järvinen <[email protected]>
Reviewed-by: Reinette Chatre <[email protected]>
---

v3:
- Corrected typo in the changelog
---
tools/testing/selftests/resctrl/cache.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c
index 1fa4b86e1459..6d60a2f1b3aa 100644
--- a/tools/testing/selftests/resctrl/cache.c
+++ b/tools/testing/selftests/resctrl/cache.c
@@ -41,7 +41,6 @@ static int perf_event_open_llc_miss(pid_t pid, int cpu_no)
PERF_FLAG_FD_CLOEXEC);
if (fd_lm == -1) {
ksft_perror("Error opening leader");
- ctrlc_handler(0, NULL, NULL);
return -1;
}

--
2.30.2


2023-12-15 15:16:27

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH v4 27/29] selftests/resctrl: Add helper to convert L2/3 to integer

"L2"/"L3" conversion to integer is embedded into get_cache_size()
which prevents reuse.

Create a helper for the cache string to integer conversion to make
it reusable.

Signed-off-by: Ilpo Järvinen <[email protected]>
Reviewed-by: Reinette Chatre <[email protected]>
---
tools/testing/selftests/resctrl/resctrlfs.c | 28 +++++++++++++++------
1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
index fed6741edc5f..eab928c46f98 100644
--- a/tools/testing/selftests/resctrl/resctrlfs.c
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -94,6 +94,23 @@ int umount_resctrlfs(void)
return 0;
}

+/*
+ * get_cache_level - Convert cache level from string to integer
+ * @cache_type: Cache level as string
+ *
+ * Return: cache level as integer or -1 if @cache_type is invalid.
+ */
+static int get_cache_level(const char *cache_type)
+{
+ if (!strcmp(cache_type, "L3"))
+ return 3;
+ if (!strcmp(cache_type, "L2"))
+ return 2;
+
+ ksft_print_msg("Invalid cache level\n");
+ return -1;
+}
+
/*
* get_resource_id - Get socket number/l3 id for a specified CPU
* @cpu_no: CPU number
@@ -144,14 +161,9 @@ int get_cache_size(int cpu_no, const char *cache_type, unsigned long *cache_size
int length, i, cache_num;
FILE *fp;

- if (!strcmp(cache_type, "L3")) {
- cache_num = 3;
- } else if (!strcmp(cache_type, "L2")) {
- cache_num = 2;
- } else {
- ksft_print_msg("Invalid cache level\n");
- return -1;
- }
+ cache_num = get_cache_level(cache_type);
+ if (cache_num < 0)
+ return cache_num;

sprintf(cache_path, "/sys/bus/cpu/devices/cpu%d/cache/index%d/size",
cpu_no, cache_num);
--
2.30.2


2023-12-15 15:17:27

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH v4 23/29] selftests/resctrl: Restore the CPU affinity after CAT test

CAT test does not reset the CPU affinity after the benchmark.
This is relatively harmless as is because CAT test is the last
benchmark to run, however, more tests may be added later.

Store the CPU affinity the first time taskset_benchmark() is run and
add taskset_restore() which the test can call to reset the CPU mask to
its original value.

Signed-off-by: Ilpo Järvinen <[email protected]>
Reviewed-by: Reinette Chatre <[email protected]>
---

v4:
- Reworded error prints

v3:
- New patch
---
tools/testing/selftests/resctrl/cat_test.c | 13 +++++---
tools/testing/selftests/resctrl/resctrl.h | 3 +-
tools/testing/selftests/resctrl/resctrl_val.c | 2 +-
tools/testing/selftests/resctrl/resctrlfs.c | 33 +++++++++++++++++--
4 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
index b79916069788..fa95433297c9 100644
--- a/tools/testing/selftests/resctrl/cat_test.c
+++ b/tools/testing/selftests/resctrl/cat_test.c
@@ -156,6 +156,7 @@ static int cat_test(struct resctrl_val_param *param, size_t span, unsigned long
char *resctrl_val = param->resctrl_val;
struct perf_event_read pe_read;
struct perf_event_attr pea;
+ cpu_set_t old_affinity;
unsigned char *buf;
char schemata[64];
int ret, i, pe_fd;
@@ -167,7 +168,7 @@ static int cat_test(struct resctrl_val_param *param, size_t span, unsigned long
bm_pid = getpid();

/* Taskset benchmark to specified cpu */
- ret = taskset_benchmark(bm_pid, param->cpu_no);
+ ret = taskset_benchmark(bm_pid, param->cpu_no, &old_affinity);
if (ret)
return ret;

@@ -175,13 +176,15 @@ static int cat_test(struct resctrl_val_param *param, size_t span, unsigned long
ret = write_bm_pid_to_resctrl(bm_pid, param->ctrlgrp, param->mongrp,
resctrl_val);
if (ret)
- return ret;
+ goto reset_affinity;

perf_event_attr_initialize(&pea, PERF_COUNT_HW_CACHE_MISSES);
perf_event_initialize_read_format(&pe_read);
pe_fd = perf_open(&pea, bm_pid, param->cpu_no);
- if (pe_fd < 0)
- return pe_fd;
+ if (pe_fd < 0) {
+ ret = -1;
+ goto reset_affinity;
+ }

buf = alloc_buffer(span, 1);
if (!buf) {
@@ -220,6 +223,8 @@ static int cat_test(struct resctrl_val_param *param, size_t span, unsigned long
free(buf);
pe_close:
close(pe_fd);
+reset_affinity:
+ taskset_restore(bm_pid, &old_affinity);

return ret;
}
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index 4f040e999eea..94347acafe55 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -98,7 +98,8 @@ int umount_resctrlfs(void);
int validate_bw_report_request(char *bw_report);
bool validate_resctrl_feature_request(const char *resource, const char *feature);
char *fgrep(FILE *inf, const char *str);
-int taskset_benchmark(pid_t bm_pid, int cpu_no);
+int taskset_benchmark(pid_t bm_pid, int cpu_no, cpu_set_t *old_affinity);
+int taskset_restore(pid_t bm_pid, cpu_set_t *old_affinity);
int write_schemata(char *ctrlgrp, char *schemata, int cpu_no,
char *resctrl_val);
int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp,
diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
index f6859fe433d9..6574a13f3919 100644
--- a/tools/testing/selftests/resctrl/resctrl_val.c
+++ b/tools/testing/selftests/resctrl/resctrl_val.c
@@ -777,7 +777,7 @@ int resctrl_val(const char * const *benchmark_cmd, struct resctrl_val_param *par
value.sival_ptr = (void *)benchmark_cmd;

/* Taskset benchmark to specified cpu */
- ret = taskset_benchmark(bm_pid, param->cpu_no);
+ ret = taskset_benchmark(bm_pid, param->cpu_no, NULL);
if (ret)
goto out;

diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
index 846281e429ca..1bcb9aa96cc2 100644
--- a/tools/testing/selftests/resctrl/resctrlfs.c
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -345,15 +345,25 @@ int get_mask_no_shareable(const char *cache_type, unsigned long *mask)

/*
* taskset_benchmark - Taskset PID (i.e. benchmark) to a specified cpu
- * @bm_pid: PID that should be binded
- * @cpu_no: CPU number at which the PID would be binded
+ * @bm_pid: PID that should be binded
+ * @cpu_no: CPU number at which the PID would be binded
+ * @old_affinity: When not NULL, set to old CPU affinity
*
* Return: 0 on success, < 0 on error.
*/
-int taskset_benchmark(pid_t bm_pid, int cpu_no)
+int taskset_benchmark(pid_t bm_pid, int cpu_no, cpu_set_t *old_affinity)
{
cpu_set_t my_set;

+ if (old_affinity) {
+ CPU_ZERO(old_affinity);
+ if (sched_getaffinity(bm_pid, sizeof(*old_affinity),
+ old_affinity)) {
+ ksft_perror("Unable to read CPU affinity");
+ return -1;
+ }
+ }
+
CPU_ZERO(&my_set);
CPU_SET(cpu_no, &my_set);

@@ -366,6 +376,23 @@ int taskset_benchmark(pid_t bm_pid, int cpu_no)
return 0;
}

+/*
+ * taskset_restore - Taskset PID to the earlier CPU affinity
+ * @bm_pid: PID that should be reset
+ * @old_affinity: The old CPU affinity to restore
+ *
+ * Return: 0 on success, < 0 on error.
+ */
+int taskset_restore(pid_t bm_pid, cpu_set_t *old_affinity)
+{
+ if (sched_setaffinity(bm_pid, sizeof(*old_affinity), old_affinity)) {
+ ksft_perror("Unable to restore CPU affinity");
+ return -1;
+ }
+
+ return 0;
+}
+
/*
* create_grp - Create a group only if one doesn't exist
* @grp_name: Name of the group
--
2.30.2


2023-12-15 17:50:31

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH v4 00/29] selftests/resctrl: CAT test improvements & generalized test framework

Hi Ilpo and Shuah,

On 12/15/2023 7:04 AM, Ilpo Järvinen wrote:
> Here's v4 series to improve resctrl selftests with generalized test
> framework and rewritten CAT test.
>
> The series contains following improvements:
>
> - Excludes shareable bits from CAT test allocation to avoid interference
> - Replaces file "sink" with a volatile variable
> - Alters read pattern to defeat HW prefetcher optimizations
> - Rewrites CAT test to make the CAT test reliable and truly measure
> if CAT is working or not
> - Introduces generalized test framework making easier to add new tests
> - Lots of other cleanups & refactoring
>
> This series has been tested across a large number of systems from
> different generations.

Ilpo, thank you very much for this great cleanup and a creating a
reliable CAT test. This work is focused on kernel health and greatly
appreciated.

All patches in this series should have my reviewed-by tag. For
confirmation, for this whole series:
Reviewed-by: Reinette Chatre <[email protected]>

Shuah, could you please consider this series for inclusion at
your convenience?

Thank you very much.

Reinette

2023-12-15 17:51:01

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH v4 07/29] selftests/resctrl: Refactor get_cbm_mask() and rename to get_full_cbm()

Hi Ilpo,

On 12/15/2023 7:04 AM, Ilpo Järvinen wrote:
> Callers of get_cbm_mask() are required to pass a string into which the
> capacity bitmask (CBM) is read. Neither CAT nor CMT tests need the
> bitmask as string but just convert it into an unsigned long value.
>
> Another limitation is that the bit mask reader can only read
> .../cbm_mask files.
>
> Generalize the bit mask reading function into get_bit_mask() such that
> it can be used to handle other files besides the .../cbm_mask and
> handles the unsigned long conversion within get_bit_mask() using
> fscanf(). Change get_cbm_mask() to use get_bit_mask() and rename it to
> get_full_cbm() to better indicate what the function does.
>
> Return error from get_full_cbm() if the bitmask is zero for some reason
> because it makes the code more robust as the selftests naturally assume
> the bitmask has some bits.
>
> Also mark cache_type const while at it and remove useless comments that
> are related to processing of CBM bits.
>
> Co-developed-by: Fenghua Yu <[email protected]>
> Signed-off-by: Fenghua Yu <[email protected]>
> Signed-off-by: Ilpo Järvinen <[email protected]>
> ---

Reviewed-by: Reinette Chatre <[email protected]>

Reinette

2023-12-15 23:45:32

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH v4 00/29] selftests/resctrl: CAT test improvements & generalized test framework



On 12/15/2023 9:45 AM, Reinette Chatre wrote:
> Hi Ilpo and Shuah,
>
> On 12/15/2023 7:04 AM, Ilpo Järvinen wrote:
>> Here's v4 series to improve resctrl selftests with generalized test
>> framework and rewritten CAT test.
>>
>> The series contains following improvements:
>>
>> - Excludes shareable bits from CAT test allocation to avoid interference
>> - Replaces file "sink" with a volatile variable
>> - Alters read pattern to defeat HW prefetcher optimizations
>> - Rewrites CAT test to make the CAT test reliable and truly measure
>> if CAT is working or not
>> - Introduces generalized test framework making easier to add new tests
>> - Lots of other cleanups & refactoring
>>
>> This series has been tested across a large number of systems from
>> different generations.
>
> Ilpo, thank you very much for this great cleanup and a creating a
> reliable CAT test. This work is focused on kernel health and greatly
> appreciated.
>
> All patches in this series should have my reviewed-by tag. For
> confirmation, for this whole series:
> Reviewed-by: Reinette Chatre <[email protected]>
>
> Shuah, could you please consider this series for inclusion at
> your convenience?

Just in case somebody tries this series out against kernel v6.7-rc5 ...

A problematic perf patch made it into v6.7-rc5 (v6.7-rc4 and before are fine).
When testing this series against kernel v6.7-rc5 the splat reported at [1]
is triggered. A perf fix [2] is already queued up so all will be fine when testing
this series against a kernel with [2] merged.

Reinette


[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lore.kernel.org/lkml/[email protected]/