The resctrl selftest code contains a number of perror() calls. Some of them come with hash character and some don't. The kselftest framework provides ksft_perror() that is compatible with test output formatting so it should be used instead of adding custom hash signs.

Some perror() calls are too far away from anything that sets error. For those call sites, ksft_print_msg() must be used instead.

Convert perror() to ksft_perror() or ksft_print_msg().

Other related changes: - Remove hash signs - Remove trailing stops & newlines from ksft_perror() - Add terminating newlines for converted ksft_print_msg() - Use consistent capitalization

Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com ---

v3: - New patch --- tools/testing/selftests/resctrl/cache.c | 10 +-- tools/testing/selftests/resctrl/cat_test.c | 8 +- tools/testing/selftests/resctrl/cmt_test.c | 2 +- tools/testing/selftests/resctrl/fill_buf.c | 2 +- tools/testing/selftests/resctrl/mba_test.c | 2 +- tools/testing/selftests/resctrl/mbm_test.c | 2 +- tools/testing/selftests/resctrl/resctrl.h | 3 +- tools/testing/selftests/resctrl/resctrl_val.c | 76 ++++++++++--------- tools/testing/selftests/resctrl/resctrlfs.c | 42 +++++----- 9 files changed, 77 insertions(+), 70 deletions(-)

diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c index bcbca356d56a..992bac8c352b 100644 --- a/tools/testing/selftests/resctrl/cache.c +++ b/tools/testing/selftests/resctrl/cache.c @@ -40,7 +40,7 @@ static int perf_event_open_llc_miss(pid_t pid, int cpu_no) fd_lm = perf_event_open(&pea_llc_miss, pid, cpu_no, -1, PERF_FLAG_FD_CLOEXEC); if (fd_lm == -1) { - perror("Error opening leader"); + ksft_perror("Error opening leader"); ctrlc_handler(0, NULL, NULL); return -1; } @@ -95,7 +95,7 @@ static int get_llc_perf(unsigned long *llc_perf_miss)

ret = read(fd_lm, &rf_cqm, sizeof(struct read_format)); if (ret == -1) { - perror("Could not get llc misses through perf"); + ksft_perror("Could not get llc misses through perf"); return -1; }

@@ -124,12 +124,12 @@ static int get_llc_occu_resctrl(unsigned long *llc_occupancy)

fp = fopen(llc_occup_path, "r"); if (!fp) { - perror("Failed to open results file"); + ksft_perror("Failed to open results file");

return errno; } if (fscanf(fp, "%lu", llc_occupancy) <= 0) { - perror("Could not get llc occupancy"); + ksft_perror("Could not get llc occupancy"); fclose(fp);

return -1; @@ -159,7 +159,7 @@ static int print_results_cache(char *filename, int bm_pid, } else { fp = fopen(filename, "a"); if (!fp) { - perror("Cannot open results file"); + ksft_perror("Cannot open results file");

return errno; } diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c index 224ba8544d8a..9bb8ba93f433 100644 --- a/tools/testing/selftests/resctrl/cat_test.c +++ b/tools/testing/selftests/resctrl/cat_test.c @@ -51,7 +51,7 @@ static int check_results(struct resctrl_val_param *param, size_t span) ksft_print_msg("Checking for pass/fail\n"); fp = fopen(param->filename, "r"); if (!fp) { - perror("# Cannot open file"); + ksft_perror("Cannot open file");

return errno; } @@ -149,7 +149,7 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type) param.num_of_runs = 0;

if (pipe(pipefd)) { - perror("# Unable to create pipe"); + ksft_perror("Unable to create pipe"); return errno; }

@@ -185,7 +185,7 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type) * Just print the error message. * Let while(1) run and wait for itself to be killed. */ - perror("# failed signaling parent process"); + ksft_perror("Failed signaling parent process");

close(pipefd[1]); while (1) @@ -197,7 +197,7 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type) while (pipe_message != 1) { if (read(pipefd[0], &pipe_message, sizeof(pipe_message)) < sizeof(pipe_message)) { - perror("# failed reading from child process"); + ksft_perror("Failed reading from child process"); break; } } diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c index 50bdbce9fba9..16fc0488e0a5 100644 --- a/tools/testing/selftests/resctrl/cmt_test.c +++ b/tools/testing/selftests/resctrl/cmt_test.c @@ -37,7 +37,7 @@ static int check_results(struct resctrl_val_param *param, size_t span, int no_of ksft_print_msg("Checking for pass/fail\n"); fp = fopen(param->filename, "r"); if (!fp) { - perror("# Error in opening file\n"); + ksft_perror("Error in opening file");

return errno; } diff --git a/tools/testing/selftests/resctrl/fill_buf.c b/tools/testing/selftests/resctrl/fill_buf.c index 0d425f26583a..0f6cca61ec94 100644 --- a/tools/testing/selftests/resctrl/fill_buf.c +++ b/tools/testing/selftests/resctrl/fill_buf.c @@ -115,7 +115,7 @@ static int fill_cache_read(unsigned char *buf, size_t buf_size, bool once) /* Consume read result so that reading memory is not optimized out. */ fp = fopen("/dev/null", "w"); if (!fp) { - perror("Unable to write to /dev/null"); + ksft_perror("Unable to write to /dev/null"); return -1; } fprintf(fp, "Sum: %d ", ret); diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c index d3bf4368341e..4988b93add6a 100644 --- a/tools/testing/selftests/resctrl/mba_test.c +++ b/tools/testing/selftests/resctrl/mba_test.c @@ -109,7 +109,7 @@ static int check_results(void)

fp = fopen(output, "r"); if (!fp) { - perror(output); + ksft_perror(output);

return errno; } diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c index 741533f2b075..045cd7818c79 100644 --- a/tools/testing/selftests/resctrl/mbm_test.c +++ b/tools/testing/selftests/resctrl/mbm_test.c @@ -59,7 +59,7 @@ static int check_results(size_t span)

fp = fopen(output, "r"); if (!fp) { - perror(output); + ksft_perror(output);

return errno; } diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h index a33f414f6019..dd3546655657 100644 --- a/tools/testing/selftests/resctrl/resctrl.h +++ b/tools/testing/selftests/resctrl/resctrl.h @@ -37,9 +37,8 @@

#define DEFAULT_SPAN (250 * MB)

-#define PARENT_EXIT(err_msg) \ +#define PARENT_EXIT() \ do { \ - perror(err_msg); \ kill(ppid, SIGKILL); \ umount_resctrlfs(); \ exit(EXIT_FAILURE); \ diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c index 88789678917b..03cedc47eaac 100644 --- a/tools/testing/selftests/resctrl/resctrl_val.c +++ b/tools/testing/selftests/resctrl/resctrl_val.c @@ -156,12 +156,12 @@ static int read_from_imc_dir(char *imc_dir, int count) sprintf(imc_counter_type, "%s%s", imc_dir, "type"); fp = fopen(imc_counter_type, "r"); if (!fp) { - perror("Failed to open imc counter type file"); + ksft_perror("Failed to open iMC counter type file");

return -1; } if (fscanf(fp, "%u", &imc_counters_config[count][READ].type) <= 0) { - perror("Could not get imc type"); + ksft_perror("Could not get iMC type"); fclose(fp);

return -1; @@ -175,12 +175,12 @@ static int read_from_imc_dir(char *imc_dir, int count) sprintf(imc_counter_cfg, "%s%s", imc_dir, READ_FILE_NAME); fp = fopen(imc_counter_cfg, "r"); if (!fp) { - perror("Failed to open imc config file"); + ksft_perror("Failed to open iMC config file");

return -1; } if (fscanf(fp, "%s", cas_count_cfg) <= 0) { - perror("Could not get imc cas count read"); + ksft_perror("Could not get iMC cas count read"); fclose(fp);

return -1; @@ -193,12 +193,12 @@ static int read_from_imc_dir(char *imc_dir, int count) sprintf(imc_counter_cfg, "%s%s", imc_dir, WRITE_FILE_NAME); fp = fopen(imc_counter_cfg, "r"); if (!fp) { - perror("Failed to open imc config file"); + ksft_perror("Failed to open iMC config file");

return -1; } if (fscanf(fp, "%s", cas_count_cfg) <= 0) { - perror("Could not get imc cas count write"); + ksft_perror("Could not get iMC cas count write"); fclose(fp);

return -1; @@ -262,12 +262,12 @@ static int num_of_imcs(void) } closedir(dp); if (count == 0) { - perror("Unable find iMC counters!\n"); + ksft_print_msg("Unable to find iMC counters\n");

return -1; } } else { - perror("Unable to open PMU directory!\n"); + ksft_perror("Unable to open PMU directory");

return -1; } @@ -339,14 +339,14 @@ static int get_mem_bw_imc(int cpu_no, char *bw_report, float *bw_imc)

if (read(r->fd, &r->return_value, sizeof(struct membw_read_format)) == -1) { - perror("Couldn't get read b/w through iMC"); + ksft_perror("Couldn't get read b/w through iMC");

return -1; }

if (read(w->fd, &w->return_value, sizeof(struct membw_read_format)) == -1) { - perror("Couldn't get write bw through iMC"); + ksft_perror("Couldn't get write bw through iMC");

return -1; } @@ -416,7 +416,7 @@ static void initialize_mem_bw_resctrl(const char *ctrlgrp, const char *mongrp, int resource_id;

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

@@ -449,12 +449,12 @@ static int get_mem_bw_resctrl(unsigned long *mbm_total)

fp = fopen(mbm_total_path, "r"); if (!fp) { - perror("Failed to open total bw file"); + ksft_perror("Failed to open total bw file");

return -1; } if (fscanf(fp, "%lu", mbm_total) <= 0) { - perror("Could not get mbm local bytes"); + ksft_perror("Could not get mbm local bytes"); fclose(fp);

return -1; @@ -495,7 +495,7 @@ int signal_handler_register(void) if (sigaction(SIGINT, &sigact, NULL) || sigaction(SIGTERM, &sigact, NULL) || sigaction(SIGHUP, &sigact, NULL)) { - perror("# sigaction"); + ksft_perror("sigaction"); ret = -1; } return ret; @@ -515,7 +515,7 @@ void signal_handler_unregister(void) if (sigaction(SIGINT, &sigact, NULL) || sigaction(SIGTERM, &sigact, NULL) || sigaction(SIGHUP, &sigact, NULL)) { - perror("# sigaction"); + ksft_perror("sigaction"); } }

@@ -540,14 +540,14 @@ static int print_results_bw(char *filename, int bm_pid, float bw_imc, } else { fp = fopen(filename, "a"); if (!fp) { - perror("Cannot open results file"); + ksft_perror("Cannot open results file");

return errno; } if (fprintf(fp, "Pid: %d \t Mem_BW_iMC: %f \t Mem_BW_resc: %lu \t Difference: %lu\n", bm_pid, bw_imc, bw_resc, diff) <= 0) { + ksft_perror("Could not log results"); fclose(fp); - perror("Could not log results.");

return errno; } @@ -585,7 +585,7 @@ static void initialize_llc_occu_resctrl(const char *ctrlgrp, const char *mongrp, int resource_id;

if (get_resource_id(cpu_no, &resource_id) < 0) { - perror("# Unable to resource_id"); + ksft_print_msg("Could not get resource_id\n"); return; }

@@ -647,20 +647,24 @@ static void run_benchmark(int signum, siginfo_t *info, void *ucontext) * stdio (console) */ fp = freopen("/dev/null", "w", stdout); - if (!fp) - PARENT_EXIT("Unable to direct benchmark status to /dev/null"); + if (!fp) { + ksft_perror("Unable to direct benchmark status to /dev/null"); + PARENT_EXIT(); + }

if (strcmp(benchmark_cmd[0], "fill_buf") == 0) { /* Execute default fill_buf benchmark */ span = strtoul(benchmark_cmd[1], NULL, 10); memflush = atoi(benchmark_cmd[2]); operation = atoi(benchmark_cmd[3]); - if (!strcmp(benchmark_cmd[4], "true")) + if (!strcmp(benchmark_cmd[4], "true")) { once = true; - else if (!strcmp(benchmark_cmd[4], "false")) + } else if (!strcmp(benchmark_cmd[4], "false")) { once = false; - else - PARENT_EXIT("Invalid once parameter"); + } else { + ksft_print_msg("Invalid once parameter\n"); + PARENT_EXIT(); + }

if (run_fill_buf(span, memflush, operation, once)) fprintf(stderr, "Error in running fill buffer\n"); @@ -668,11 +672,12 @@ static void run_benchmark(int signum, siginfo_t *info, void *ucontext) /* Execute specified benchmark */ ret = execvp(benchmark_cmd[0], benchmark_cmd); if (ret) - perror("wrong\n"); + ksft_perror("execvp"); }

fclose(stdout); - PARENT_EXIT("Unable to run specified benchmark"); + ksft_print_msg("Unable to run specified benchmark\n"); + PARENT_EXIT(); }

/* @@ -709,7 +714,7 @@ int resctrl_val(const char * const *benchmark_cmd, struct resctrl_val_param *par ppid = getpid();

if (pipe(pipefd)) { - perror("# Unable to create pipe"); + ksft_perror("Unable to create pipe");

return -1; } @@ -721,7 +726,7 @@ int resctrl_val(const char * const *benchmark_cmd, struct resctrl_val_param *par fflush(stdout); bm_pid = fork(); if (bm_pid == -1) { - perror("# Unable to fork"); + ksft_perror("Unable to fork");

return -1; } @@ -738,15 +743,17 @@ int resctrl_val(const char * const *benchmark_cmd, struct resctrl_val_param *par sigact.sa_flags = SA_SIGINFO;

/* Register for "SIGUSR1" signal from parent */ - if (sigaction(SIGUSR1, &sigact, NULL)) - PARENT_EXIT("Can't register child for signal"); + if (sigaction(SIGUSR1, &sigact, NULL)) { + ksft_perror("Can't register child for signal"); + PARENT_EXIT(); + }

/* Tell parent that child is ready */ close(pipefd[0]); pipe_message = 1; if (write(pipefd[1], &pipe_message, sizeof(pipe_message)) < sizeof(pipe_message)) { - perror("# failed signaling parent process"); + ksft_perror("Failed signaling parent process"); close(pipefd[1]); return -1; } @@ -755,7 +762,8 @@ int resctrl_val(const char * const *benchmark_cmd, struct resctrl_val_param *par /* Suspend child until delivery of "SIGUSR1" from parent */ sigsuspend(&sigact.sa_mask);

- PARENT_EXIT("Child is done"); + ksft_perror("Child is done"); + PARENT_EXIT(); }

ksft_print_msg("Benchmark PID: %d\n", bm_pid); @@ -796,7 +804,7 @@ int resctrl_val(const char * const *benchmark_cmd, struct resctrl_val_param *par while (pipe_message != 1) { if (read(pipefd[0], &pipe_message, sizeof(pipe_message)) < sizeof(pipe_message)) { - perror("# failed reading message from child process"); + ksft_perror("Failed reading message from child process"); close(pipefd[0]); goto out; } @@ -805,7 +813,7 @@ int resctrl_val(const char * const *benchmark_cmd, struct resctrl_val_param *par

/* Signal child to start benchmark */ if (sigqueue(bm_pid, SIGUSR1, value) == -1) { - perror("# sigqueue SIGUSR1 to child"); + ksft_perror("sigqueue SIGUSR1 to child"); ret = errno; goto out; } diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c index 5ebd43683876..a057f3bc2d27 100644 --- a/tools/testing/selftests/resctrl/resctrlfs.c +++ b/tools/testing/selftests/resctrl/resctrlfs.c @@ -20,7 +20,7 @@ static int find_resctrl_mount(char *buffer)

mounts = fopen("/proc/mounts", "r"); if (!mounts) { - perror("/proc/mounts"); + ksft_perror("/proc/mounts"); return -ENXIO; } while (!feof(mounts)) { @@ -69,7 +69,7 @@ int mount_resctrlfs(void) ksft_print_msg("Mounting resctrl to "%s"\n", RESCTRL_PATH); ret = mount("resctrl", RESCTRL_PATH, "resctrl", 0, NULL); if (ret) - perror("# mount"); + ksft_perror("mount");

return ret; } @@ -86,7 +86,7 @@ int umount_resctrlfs(void) return ret;

if (umount(mountpoint)) { - perror("# Unable to umount resctrl"); + ksft_perror("Unable to umount resctrl");

return errno; } @@ -115,12 +115,12 @@ int get_resource_id(int cpu_no, int *resource_id)

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

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

return -1; @@ -149,7 +149,7 @@ int get_cache_size(int cpu_no, char *cache_type, unsigned long *cache_size) } else if (!strcmp(cache_type, "L2")) { cache_num = 2; } else { - perror("Invalid cache level"); + ksft_print_msg("Invalid cache level\n"); return -1; }

@@ -157,12 +157,12 @@ int get_cache_size(int cpu_no, char *cache_type, unsigned long *cache_size) cpu_no, cache_num); fp = fopen(cache_path, "r"); if (!fp) { - perror("Failed to open cache size"); + ksft_perror("Failed to open cache size");

return -1; } if (fscanf(fp, "%s", cache_str) <= 0) { - perror("Could not get cache_size"); + ksft_perror("Could not get cache_size"); fclose(fp);

return -1; @@ -214,12 +214,12 @@ int get_cbm_mask(char *cache_type, char *cbm_mask)

fp = fopen(cbm_mask_path, "r"); if (!fp) { - perror("Failed to open cache level"); + ksft_perror("Failed to open cache level");

return -1; } if (fscanf(fp, "%s", cbm_mask) <= 0) { - perror("Could not get max cbm_mask"); + ksft_perror("Could not get max cbm_mask"); fclose(fp);

return -1; @@ -246,12 +246,12 @@ int get_core_sibling(int cpu_no)

fp = fopen(core_siblings_path, "r"); if (!fp) { - perror("Failed to open core siblings path"); + ksft_perror("Failed to open core siblings path");

return -1; } if (fscanf(fp, "%s", cpu_list_str) <= 0) { - perror("Could not get core_siblings list"); + ksft_perror("Could not get core_siblings list"); fclose(fp);

return -1; @@ -286,7 +286,7 @@ int taskset_benchmark(pid_t bm_pid, int cpu_no) CPU_SET(cpu_no, &my_set);

if (sched_setaffinity(bm_pid, sizeof(cpu_set_t), &my_set)) { - perror("Unable to taskset benchmark"); + ksft_perror("Unable to taskset benchmark");

return -1; } @@ -325,7 +325,7 @@ static int create_grp(const char *grp_name, char *grp, const char *parent_grp) } closedir(dp); } else { - perror("Unable to open resctrl for group"); + ksft_perror("Unable to open resctrl for group");

return -1; } @@ -333,7 +333,7 @@ static int create_grp(const char *grp_name, char *grp, const char *parent_grp) /* Requested grp doesn't exist, hence create it */ if (found_grp == 0) { if (mkdir(grp, 0) == -1) { - perror("Unable to create group"); + ksft_perror("Unable to create group");

return -1; } @@ -348,12 +348,12 @@ static int write_pid_to_tasks(char *tasks, pid_t pid)

fp = fopen(tasks, "w"); if (!fp) { - perror("Failed to open tasks file"); + ksft_perror("Failed to open tasks file");

return -1; } if (fprintf(fp, "%d\n", pid) < 0) { - perror("Failed to wr pid to tasks file"); + ksft_perror("Failed to wr pid to tasks file"); fclose(fp);

return -1; @@ -420,7 +420,7 @@ int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp, out: ksft_print_msg("Writing benchmark parameters to resctrl FS\n"); if (ret) - perror("# writing to resctrlfs"); + ksft_print_msg("Failed writing to resctrlfs\n");

return ret; } @@ -617,7 +617,7 @@ int filter_dmesg(void)

ret = pipe(pipefds); if (ret) { - perror("pipe"); + ksft_perror("pipe"); return ret; } fflush(stdout); @@ -626,13 +626,13 @@ int filter_dmesg(void) close(pipefds[0]); dup2(pipefds[1], STDOUT_FILENO); execlp("dmesg", "dmesg", NULL); - perror("executing dmesg"); + ksft_perror("Executing dmesg"); exit(1); } close(pipefds[1]); fp = fdopen(pipefds[0], "r"); if (!fp) { - perror("fdopen(pipe)"); + ksft_perror("fdopen(pipe)"); kill(pid, SIGTERM);

return -1;

A number of functions in the resctrl selftests return errno. It is problematic because errno is positive which is often counterintuitive. Also, every sites returning errno prints the error message already with ksft_perror() so there is not much added value in returning the precise error code.

Simply convert all places returning errno to return -1 that is typical userspace error code in case of failures.

While at it, improve resctrl_val() comment to state that 0 means the test was run (either pass or fail).

Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com ---

v3: - New patch --- tools/testing/selftests/resctrl/cache.c | 6 +++--- tools/testing/selftests/resctrl/cat_test.c | 4 ++-- tools/testing/selftests/resctrl/cmt_test.c | 2 +- tools/testing/selftests/resctrl/mba_test.c | 2 +- tools/testing/selftests/resctrl/mbm_test.c | 2 +- tools/testing/selftests/resctrl/resctrl_val.c | 10 +++++----- tools/testing/selftests/resctrl/resctrlfs.c | 2 +- 7 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c index 992bac8c352b..1fa4b86e1459 100644 --- a/tools/testing/selftests/resctrl/cache.c +++ b/tools/testing/selftests/resctrl/cache.c @@ -126,7 +126,7 @@ static int get_llc_occu_resctrl(unsigned long *llc_occupancy) if (!fp) { ksft_perror("Failed to open results file");

- return errno; + return -1; } if (fscanf(fp, "%lu", llc_occupancy) <= 0) { ksft_perror("Could not get llc occupancy"); @@ -146,7 +146,7 @@ static int get_llc_occu_resctrl(unsigned long *llc_occupancy) * @llc_value: perf miss value / * llc occupancy value reported by resctrl FS * - * Return: 0 on success. non-zero on failure. + * Return: 0 on success, < 0 on error. */ static int print_results_cache(char *filename, int bm_pid, unsigned long llc_value) @@ -161,7 +161,7 @@ static int print_results_cache(char *filename, int bm_pid, if (!fp) { ksft_perror("Cannot open results file");

- return errno; + return -1; } fprintf(fp, "Pid: %d \t llc_value: %lu\n", bm_pid, llc_value); fclose(fp); diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c index 9bb8ba93f433..fabb56ff68d1 100644 --- a/tools/testing/selftests/resctrl/cat_test.c +++ b/tools/testing/selftests/resctrl/cat_test.c @@ -53,7 +53,7 @@ static int check_results(struct resctrl_val_param *param, size_t span) if (!fp) { ksft_perror("Cannot open file");

- return errno; + return -1; }

while (fgets(temp, sizeof(temp), fp)) { @@ -150,7 +150,7 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)

if (pipe(pipefd)) { ksft_perror("Unable to create pipe"); - return errno; + return -1; }

fflush(stdout); diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c index 16fc0488e0a5..ffd302bd5c73 100644 --- a/tools/testing/selftests/resctrl/cmt_test.c +++ b/tools/testing/selftests/resctrl/cmt_test.c @@ -39,7 +39,7 @@ static int check_results(struct resctrl_val_param *param, size_t span, int no_of if (!fp) { ksft_perror("Error in opening file");

- return errno; + return -1; }

while (fgets(temp, sizeof(temp), fp)) { diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c index 4988b93add6a..e907adf7cd25 100644 --- a/tools/testing/selftests/resctrl/mba_test.c +++ b/tools/testing/selftests/resctrl/mba_test.c @@ -111,7 +111,7 @@ static int check_results(void) if (!fp) { ksft_perror(output);

- return errno; + return -1; }

runs = 0; diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c index 045cd7818c79..721b3ecbc158 100644 --- a/tools/testing/selftests/resctrl/mbm_test.c +++ b/tools/testing/selftests/resctrl/mbm_test.c @@ -61,7 +61,7 @@ static int check_results(size_t span) if (!fp) { ksft_perror(output);

- return errno; + return -1; }

runs = 0; diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c index 03cedc47eaac..2edc5202a543 100644 --- a/tools/testing/selftests/resctrl/resctrl_val.c +++ b/tools/testing/selftests/resctrl/resctrl_val.c @@ -526,7 +526,7 @@ void signal_handler_unregister(void) * @bw_imc: perf imc counter value * @bw_resc: memory bandwidth value * - * Return: 0 on success. non-zero on failure. + * Return: 0 on success, < 0 on error. */ static int print_results_bw(char *filename, int bm_pid, float bw_imc, unsigned long bw_resc) @@ -542,14 +542,14 @@ static int print_results_bw(char *filename, int bm_pid, float bw_imc, if (!fp) { ksft_perror("Cannot open results file");

- return errno; + return -1; } if (fprintf(fp, "Pid: %d \t Mem_BW_iMC: %f \t Mem_BW_resc: %lu \t Difference: %lu\n", bm_pid, bw_imc, bw_resc, diff) <= 0) { ksft_perror("Could not log results"); fclose(fp);

- return errno; + return -1; } fclose(fp); } @@ -686,7 +686,7 @@ static void run_benchmark(int signum, siginfo_t *info, void *ucontext) * @benchmark_cmd: benchmark command and its arguments * @param: parameters passed to resctrl_val() * - * Return: 0 on success. non-zero on failure. + * Return: 0 when the test was run, < 0 on error. */ int resctrl_val(const char * const *benchmark_cmd, struct resctrl_val_param *param) { @@ -814,7 +814,7 @@ int resctrl_val(const char * const *benchmark_cmd, struct resctrl_val_param *par /* Signal child to start benchmark */ if (sigqueue(bm_pid, SIGUSR1, value) == -1) { ksft_perror("sigqueue SIGUSR1 to child"); - ret = errno; + ret = -1; goto out; }

diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c index a057f3bc2d27..f5664392473c 100644 --- a/tools/testing/selftests/resctrl/resctrlfs.c +++ b/tools/testing/selftests/resctrl/resctrlfs.c @@ -88,7 +88,7 @@ int umount_resctrlfs(void) if (umount(mountpoint)) { ksft_perror("Unable to umount resctrl");

- return errno; + return -1; }

return 0;

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 reinette.chatre@intel.com Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com Reviewed-by: Reinette Chatre reinette.chatre@intel.com ---

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; }

There are unnecessary nested calls in fill_buf.c: - run_fill_buf() calls fill_cache() - alloc_buffer() calls malloc_and_init_memory()

Simplify the code flow and remove those unnecessary call levels by moving the called code inside the calling function and remove the duplicated error print.

Resolve the difference in run_fill_buf() and fill_cache() parameter name into 'buf_size' which is more descriptive than 'span'. Also, while moving the allocation related code, rename 'p' into 'buf' to be consistent in naming the variables.

Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com Reviewed-by: Reinette Chatre reinette.chatre@intel.com ---

v3: - Moved error printout remove to this patch --- tools/testing/selftests/resctrl/fill_buf.c | 59 +++++++--------------- tools/testing/selftests/resctrl/resctrl.h | 2 +- 2 files changed, 18 insertions(+), 43 deletions(-)

diff --git a/tools/testing/selftests/resctrl/fill_buf.c b/tools/testing/selftests/resctrl/fill_buf.c index 6d1d5eed595c..635f938b11f0 100644 --- a/tools/testing/selftests/resctrl/fill_buf.c +++ b/tools/testing/selftests/resctrl/fill_buf.c @@ -51,29 +51,6 @@ static void mem_flush(unsigned char *buf, size_t buf_size) sb(); }

-static void *malloc_and_init_memory(size_t buf_size) -{ - void *p = NULL; - uint64_t *p64; - size_t s64; - int ret; - - ret = posix_memalign(&p, PAGE_SIZE, buf_size); - if (ret < 0) - return NULL; - - p64 = (uint64_t *)p; - s64 = buf_size / sizeof(uint64_t); - - while (s64 > 0) { - *p64 = (uint64_t)rand(); - p64 += (CL_SIZE / sizeof(uint64_t)); - s64 -= (CL_SIZE / sizeof(uint64_t)); - } - - return p; -} - static int fill_one_span_read(unsigned char *buf, size_t buf_size) { unsigned char *end_ptr = buf + buf_size; @@ -137,12 +114,25 @@ static int fill_cache_write(unsigned char *buf, size_t buf_size, bool once)

static unsigned char *alloc_buffer(size_t buf_size, int memflush) { - unsigned char *buf; + void *buf = NULL; + uint64_t *p64; + size_t s64; + int ret;

- buf = malloc_and_init_memory(buf_size); - if (!buf) + ret = posix_memalign(&buf, PAGE_SIZE, buf_size); + if (ret < 0) return NULL;

+ /* Initialize the buffer */ + p64 = buf; + s64 = buf_size / sizeof(uint64_t); + + while (s64 > 0) { + *p64 = (uint64_t)rand(); + p64 += (CL_SIZE / sizeof(uint64_t)); + s64 -= (CL_SIZE / sizeof(uint64_t)); + } + /* Flush the memory before using to avoid "cache hot pages" effect */ if (memflush) mem_flush(buf, buf_size); @@ -150,7 +140,7 @@ static unsigned char *alloc_buffer(size_t buf_size, int memflush) return buf; }

-static int fill_cache(size_t buf_size, int memflush, int op, bool once) +int run_fill_buf(size_t buf_size, int memflush, int op, bool once) { unsigned char *buf; int ret; @@ -164,21 +154,6 @@ static int fill_cache(size_t buf_size, int memflush, int op, bool once) else ret = fill_cache_write(buf, buf_size, once); free(buf); - - if (ret) { - printf("\n Error in fill cache read/write...\n"); - return -1; - } - - return ret; -} - -int run_fill_buf(size_t span, int memflush, int op, bool once) -{ - size_t cache_size = span; - int ret; - - ret = fill_cache(cache_size, memflush, op, once); if (ret) { printf("\n Error in fill cache\n"); return -1; diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h index dd3546655657..a848e9c75578 100644 --- a/tools/testing/selftests/resctrl/resctrl.h +++ b/tools/testing/selftests/resctrl/resctrl.h @@ -91,7 +91,7 @@ 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, int group_fd, unsigned long flags); -int run_fill_buf(size_t span, int memflush, int op, bool once); +int run_fill_buf(size_t buf_size, int memflush, int op, bool once); int resctrl_val(const char * const *benchmark_cmd, struct resctrl_val_param *param); int mbm_bw_change(int cpu_no, const char * const *benchmark_cmd); void tests_cleanup(void);

get_cache_size() does not modify cache_type so it could be const.

Mark cache_type const so that const char * can be passed to it. This prevents warnings once many of the test parameters are marked const.

Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com Reviewed-by: Reinette Chatre reinette.chatre@intel.com --- tools/testing/selftests/resctrl/resctrl.h | 2 +- tools/testing/selftests/resctrl/resctrlfs.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h index 89cd89507891..9e5777e1eee3 100644 --- a/tools/testing/selftests/resctrl/resctrl.h +++ b/tools/testing/selftests/resctrl/resctrl.h @@ -99,7 +99,7 @@ void mbm_test_cleanup(void); int mba_schemata_change(int cpu_no, const char * const *benchmark_cmd); void mba_test_cleanup(void); 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); +int get_cache_size(int cpu_no, const char *cache_type, unsigned long *cache_size); void ctrlc_handler(int signum, siginfo_t *info, void *ptr); int signal_handler_register(void); void signal_handler_unregister(void); diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c index 5f322a55dbb6..65e149038248 100644 --- a/tools/testing/selftests/resctrl/resctrlfs.c +++ b/tools/testing/selftests/resctrl/resctrlfs.c @@ -138,7 +138,7 @@ int get_resource_id(int cpu_no, int *resource_id) * * Return: = 0 on success, < 0 on failure. */ -int get_cache_size(int cpu_no, char *cache_type, unsigned long *cache_size) +int get_cache_size(int cpu_no, const char *cache_type, unsigned long *cache_size) { char cache_path[1024], cache_str[64]; int length, i, cache_num;

CAT test doesn't take shareable bits into account, i.e., the test might be sharing cache with some devices (e.g., graphics).

Introduce get_mask_no_shareable() and use it to provision an environment for CAT test where the allocated LLC is isolated better. Excluding shareable_bits may create hole(s) into the cbm_mask, thus add a new helper count_contiguous_bits() to find the longest contiguous set of CBM bits.

create_bit_mask() is needed by an upcoming CAT test rewrite so make it available in resctrl.h right away.

Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com ---

v3: - Tweak comments - Make get_shareable_mask() static --- tools/testing/selftests/resctrl/cat_test.c | 12 ++- tools/testing/selftests/resctrl/resctrl.h | 2 + tools/testing/selftests/resctrl/resctrlfs.c | 89 +++++++++++++++++++++ 3 files changed, 99 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c index f18c95dda5d3..1e3c5432ecff 100644 --- a/tools/testing/selftests/resctrl/cat_test.c +++ b/tools/testing/selftests/resctrl/cat_test.c @@ -89,15 +89,19 @@ void cat_test_cleanup(void)

int cat_perf_miss_val(int cpu_no, int n, char *cache_type) { + unsigned long full_cache_mask, long_mask; unsigned long l_mask, l_mask_1; int ret, pipefd[2], sibling_cpu_no; unsigned long cache_total_size = 0; - unsigned long long_mask; int count_of_bits; char pipe_message; size_t span;

- ret = get_full_cbm(cache_type, &long_mask); + ret = get_full_cbm(cache_type, &full_cache_mask); + if (ret) + return ret; + /* Get the largest contiguous exclusive portion of the cache */ + ret = get_mask_no_shareable(cache_type, &long_mask); if (ret) return ret;

@@ -136,7 +140,7 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type) /* Set param values for parent thread which will be allocated bitmask * with (max_bits - n) bits */ - span = cache_portion_size(cache_total_size, l_mask, long_mask); + span = cache_portion_size(cache_total_size, l_mask, full_cache_mask); strcpy(param.ctrlgrp, "c2"); strcpy(param.mongrp, "m2"); strcpy(param.filename, RESULT_FILE_NAME2); @@ -158,7 +162,7 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type) param.mask = l_mask_1; strcpy(param.ctrlgrp, "c1"); strcpy(param.mongrp, "m1"); - span = cache_portion_size(cache_total_size, l_mask_1, long_mask); + span = cache_portion_size(cache_total_size, l_mask_1, full_cache_mask); strcpy(param.filename, RESULT_FILE_NAME1); param.num_of_runs = 0; param.cpu_no = sibling_cpu_no; diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h index 0e3a4da5f6ec..40baebb066aa 100644 --- a/tools/testing/selftests/resctrl/resctrl.h +++ b/tools/testing/selftests/resctrl/resctrl.h @@ -98,7 +98,9 @@ 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); +unsigned long create_bit_mask(unsigned int start, unsigned int len); int get_full_cbm(const char *cache_type, unsigned long *mask); +int get_mask_no_shareable(const char *cache_type, unsigned long *mask); int get_cache_size(int cpu_no, const 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 65e149038248..35425d00ede6 100644 --- a/tools/testing/selftests/resctrl/resctrlfs.c +++ b/tools/testing/selftests/resctrl/resctrlfs.c @@ -228,6 +228,44 @@ static int get_bit_mask(const char *filename, unsigned long *mask) return 0; }

+/* + * create_bit_mask- Create bit mask from start, len pair + * @start: LSB of the mask + * @len Number of bits in the mask + */ +unsigned long create_bit_mask(unsigned int start, unsigned int len) +{ + return ((1UL << len) - 1UL) << start; +} + +/* + * count_contiguous_bits - Returns the longest train of bits in a bit mask + * @val A bit mask + * @start The location of the least-significant bit of the longest train + * + * Return: The length of the contiguous bits in the longest train of bits + */ +static unsigned int count_contiguous_bits(unsigned long val, unsigned int *start) +{ + unsigned long last_val; + unsigned int count = 0; + + while (val) { + last_val = val; + val &= (val >> 1); + count++; + } + + if (start) { + if (count) + *start = ffsl(last_val) - 1; + else + *start = 0; + } + + return count; +} + /* * get_full_cbm - Get full Cache Bit Mask (CBM) * @cache_type: Cache type as "L2" or "L3" @@ -254,6 +292,57 @@ int get_full_cbm(const char *cache_type, unsigned long *mask) return 0; }

+/* + * get_shareable_mask - Get shareable mask from shareable_bits + * @cache_type: Cache type as "L2" or "L3" + * @shareable_mask: Shareable mask returned as unsigned long + * + * Return: = 0 on success, < 0 on failure. + */ +static int get_shareable_mask(const char *cache_type, unsigned long *shareable_mask) +{ + char mask_path[PATH_MAX]; + + if (!cache_type) + return -1; + + snprintf(mask_path, sizeof(mask_path), "%s/%s/shareable_bits", + INFO_PATH, cache_type); + + return get_bit_mask(mask_path, shareable_mask); +} + +/* + * get_mask_no_shareable - Get Cache Bit Mask (CBM) without shareable bits + * @cache_type: Cache type as "L2" or "L3" + * @mask: The largest exclusive portion of the cache out of the + * full CBM, returned as unsigned long + * + * Parts of a cache may be shared with other devices such as GPU. This function + * calculates the largest exclusive portion of the cache where no other devices + * besides CPU have access to the cache portion. + * + * Return: = 0 on success, < 0 on failure. + */ +int get_mask_no_shareable(const char *cache_type, unsigned long *mask) +{ + unsigned long full_mask, shareable_mask; + unsigned int start, len; + + if (get_full_cbm(cache_type, &full_mask) < 0) + return -1; + if (get_shareable_mask(cache_type, &shareable_mask) < 0) + return -1; + + len = count_contiguous_bits(full_mask & ~shareable_mask, &start); + if (!len) + return -1; + + *mask = create_bit_mask(start, len); + + return 0; +} + /* * get_core_sibling - Get sibling core id from the same socket for given CPU * @cpu_no: CPU number

measure_cache_vals() does a different thing depending on the test case that called it: - For CAT, it measures LLC misses through perf. - For CMT, it measures LLC occupancy through resctrl.

Split these two functionalities into own functions the CAT and CMT tests can call directly. Replace passing the struct resctrl_val_param parameter with the filename because it's more generic and all those functions need out of resctrl_val.

Co-developed-by: Fenghua Yu fenghua.yu@intel.com Signed-off-by: Fenghua Yu fenghua.yu@intel.com Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com ---

v3: - Return directly without storing result into ret variable first - llc -> LLC --- tools/testing/selftests/resctrl/cache.c | 64 +++++++++++-------- tools/testing/selftests/resctrl/resctrl.h | 2 +- tools/testing/selftests/resctrl/resctrl_val.c | 2 +- 3 files changed, 41 insertions(+), 27 deletions(-)

diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c index a9a0394b3d08..0c18603435c8 100644 --- a/tools/testing/selftests/resctrl/cache.c +++ b/tools/testing/selftests/resctrl/cache.c @@ -147,7 +147,7 @@ static int get_llc_occu_resctrl(unsigned long *llc_occupancy) * * Return: 0 on success, < 0 on error. */ -static int print_results_cache(char *filename, int bm_pid, +static int print_results_cache(const char *filename, int bm_pid, unsigned long llc_value) { FILE *fp; @@ -169,35 +169,49 @@ static int print_results_cache(char *filename, int bm_pid, return 0; }

-int measure_cache_vals(struct resctrl_val_param *param, int bm_pid) +/* + * perf_event_measure - Measure perf events + * @filename: Filename for writing the results + * @bm_pid: PID that runs the benchmark + * + * Measures perf events (e.g., cache misses) and writes the results into + * @filename. @bm_pid is written to the results file along with the measured + * value. + * + * Return: =0 on success. <0 on failure. + */ +static int perf_event_measure(const char *filename, int bm_pid) { - unsigned long llc_perf_miss = 0, llc_occu_resc = 0, llc_value = 0; + unsigned long llc_perf_miss = 0; int ret;

- /* - * Measure cache miss from perf. - */ - if (!strncmp(param->resctrl_val, CAT_STR, sizeof(CAT_STR))) { - ret = get_llc_perf(&llc_perf_miss); - if (ret < 0) - return ret; - llc_value = llc_perf_miss; - } + ret = get_llc_perf(&llc_perf_miss); + if (ret < 0) + return ret;

- /* - * Measure llc occupancy from resctrl. - */ - if (!strncmp(param->resctrl_val, CMT_STR, sizeof(CMT_STR))) { - ret = get_llc_occu_resctrl(&llc_occu_resc); - if (ret < 0) - return ret; - llc_value = llc_occu_resc; - } - ret = print_results_cache(param->filename, bm_pid, llc_value); - if (ret) + return print_results_cache(filename, bm_pid, llc_perf_miss); +} + +/* + * measure_llc_resctrl - Measure resctrl LLC value from resctrl + * @filename: Filename for writing the results + * @bm_pid: PID that runs the benchmark + * + * Measures LLC occupancy from resctrl and writes the results into @filename. + * @bm_pid is written to the results file along with the measured value. + * + * Return: =0 on success. <0 on failure. + */ +int measure_llc_resctrl(const char *filename, int bm_pid) +{ + unsigned long llc_occu_resc = 0; + int ret; + + ret = get_llc_occu_resctrl(&llc_occu_resc); + if (ret < 0) return ret;

- return 0; + return print_results_cache(filename, bm_pid, llc_occu_resc); }

/* @@ -252,7 +266,7 @@ int cat_val(struct resctrl_val_param *param, size_t span) }

sleep(1); - ret = measure_cache_vals(param, bm_pid); + ret = perf_event_measure(param->filename, bm_pid); if (ret) goto pe_close; } diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h index 40baebb066aa..2a08dedcc9e4 100644 --- a/tools/testing/selftests/resctrl/resctrl.h +++ b/tools/testing/selftests/resctrl/resctrl.h @@ -112,7 +112,7 @@ int cmt_resctrl_val(int cpu_no, int n, const char * const *benchmark_cmd); unsigned int count_bits(unsigned long n); void cmt_test_cleanup(void); int get_core_sibling(int cpu_no); -int measure_cache_vals(struct resctrl_val_param *param, int bm_pid); +int measure_llc_resctrl(const char *filename, int bm_pid); int show_cache_info(unsigned long sum_llc_val, int no_of_bits, size_t cache_span, unsigned long max_diff, unsigned long max_diff_percent, unsigned long num_of_runs, diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c index 2edc5202a543..d515850cc174 100644 --- a/tools/testing/selftests/resctrl/resctrl_val.c +++ b/tools/testing/selftests/resctrl/resctrl_val.c @@ -838,7 +838,7 @@ int resctrl_val(const char * const *benchmark_cmd, struct resctrl_val_param *par break; } else if (!strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR))) { sleep(1); - ret = measure_cache_vals(param, bm_pid); + ret = measure_llc_resctrl(param->filename, bm_pid); if (ret) break; }

show_cache_info() calculates results and provides generic cache information. This makes it hard to alter pass/fail conditions.

Separate the test specific checks into CAT and CMT test files and leave only the generic information part into show_cache_info().

Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com Reviewed-by: Reinette Chatre reinette.chatre@intel.com --- tools/testing/selftests/resctrl/cache.c | 42 +++++----------------- tools/testing/selftests/resctrl/cat_test.c | 30 ++++++++++++++-- tools/testing/selftests/resctrl/cmt_test.c | 32 +++++++++++++++-- tools/testing/selftests/resctrl/resctrl.h | 6 ++-- 4 files changed, 66 insertions(+), 44 deletions(-)

diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c index 0c18603435c8..32e262fa2a6c 100644 --- a/tools/testing/selftests/resctrl/cache.c +++ b/tools/testing/selftests/resctrl/cache.c @@ -279,43 +279,17 @@ int cat_val(struct resctrl_val_param *param, size_t span) }

/* - * show_cache_info: show cache test result information - * @sum_llc_val: sum of LLC cache result data - * @no_of_bits: number of bits - * @cache_span: cache span in bytes for CMT or in lines for CAT - * @max_diff: max difference - * @max_diff_percent: max difference percentage - * @num_of_runs: number of runs - * @platform: show test information on this platform - * @cmt: CMT test or CAT test - * - * Return: 0 on success. non-zero on failure. + * show_cache_info - Show generic cache test information + * @no_of_bits: Number of bits + * @avg_llc_val: Average of LLC cache result data + * @cache_span: Cache span + * @lines: @cache_span in lines or bytes */ -int show_cache_info(unsigned long sum_llc_val, int no_of_bits, - size_t cache_span, unsigned long max_diff, - unsigned long max_diff_percent, unsigned long num_of_runs, - bool platform, bool cmt) +void show_cache_info(int no_of_bits, unsigned long avg_llc_val, + size_t cache_span, bool lines) { - unsigned long avg_llc_val = 0; - float diff_percent; - long avg_diff = 0; - int ret; - - avg_llc_val = sum_llc_val / num_of_runs; - avg_diff = (long)abs(cache_span - avg_llc_val); - diff_percent = ((float)cache_span - avg_llc_val) / cache_span * 100; - - ret = platform && abs((int)diff_percent) > max_diff_percent && - (cmt ? (abs(avg_diff) > max_diff) : true); - - ksft_print_msg("%s Check cache miss rate within %lu%%\n", - ret ? "Fail:" : "Pass:", max_diff_percent); - - ksft_print_msg("Percent diff=%d\n", abs((int)diff_percent)); ksft_print_msg("Number of bits: %d\n", no_of_bits); ksft_print_msg("Average LLC val: %lu\n", avg_llc_val); - ksft_print_msg("Cache span (%s): %zu\n", cmt ? "bytes" : "lines", + ksft_print_msg("Cache span (%s): %zu\n", lines ? "lines" : "bytes", cache_span); - - return ret; } diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c index 1e3c5432ecff..0955f819bd83 100644 --- a/tools/testing/selftests/resctrl/cat_test.c +++ b/tools/testing/selftests/resctrl/cat_test.c @@ -41,6 +41,30 @@ static int cat_setup(struct resctrl_val_param *p) return ret; }

+static int show_results_info(unsigned long sum_llc_val, int no_of_bits, + unsigned long cache_span, unsigned long max_diff, + unsigned long max_diff_percent, unsigned long num_of_runs, + bool platform) +{ + unsigned long avg_llc_val = 0; + float diff_percent; + int ret; + + avg_llc_val = sum_llc_val / num_of_runs; + diff_percent = ((float)cache_span - avg_llc_val) / cache_span * 100; + + ret = platform && abs((int)diff_percent) > max_diff_percent; + + ksft_print_msg("%s Check cache miss rate within %lu%%\n", + ret ? "Fail:" : "Pass:", max_diff_percent); + + ksft_print_msg("Percent diff=%d\n", abs((int)diff_percent)); + + show_cache_info(no_of_bits, avg_llc_val, cache_span, true); + + return ret; +} + static int check_results(struct resctrl_val_param *param, size_t span) { char *token_array[8], temp[512]; @@ -76,9 +100,9 @@ static int check_results(struct resctrl_val_param *param, size_t span) fclose(fp); no_of_bits = count_bits(param->mask);

- return show_cache_info(sum_llc_perf_miss, no_of_bits, span / 64, - MAX_DIFF, MAX_DIFF_PERCENT, runs - 1, - get_vendor() == ARCH_INTEL, false); + return show_results_info(sum_llc_perf_miss, no_of_bits, span / 64, + MAX_DIFF, MAX_DIFF_PERCENT, runs - 1, + get_vendor() == ARCH_INTEL); }

void cat_test_cleanup(void) diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c index a14d46353865..604690cd8b3e 100644 --- a/tools/testing/selftests/resctrl/cmt_test.c +++ b/tools/testing/selftests/resctrl/cmt_test.c @@ -27,6 +27,33 @@ static int cmt_setup(struct resctrl_val_param *p) return 0; }

+static int show_results_info(unsigned long sum_llc_val, int no_of_bits, + unsigned long cache_span, unsigned long max_diff, + unsigned long max_diff_percent, unsigned long num_of_runs, + bool platform) +{ + unsigned long avg_llc_val = 0; + float diff_percent; + long avg_diff = 0; + int ret; + + avg_llc_val = sum_llc_val / num_of_runs; + avg_diff = (long)abs(cache_span - avg_llc_val); + diff_percent = ((float)cache_span - avg_llc_val) / cache_span * 100; + + ret = platform && abs((int)diff_percent) > max_diff_percent && + abs(avg_diff) > max_diff; + + ksft_print_msg("%s Check cache miss rate within %lu%%\n", + ret ? "Fail:" : "Pass:", max_diff_percent); + + ksft_print_msg("Percent diff=%d\n", abs((int)diff_percent)); + + show_cache_info(no_of_bits, avg_llc_val, cache_span, false); + + return ret; +} + static int check_results(struct resctrl_val_param *param, size_t span, int no_of_bits) { char *token_array[8], temp[512]; @@ -58,9 +85,8 @@ static int check_results(struct resctrl_val_param *param, size_t span, int no_of } fclose(fp);

- return show_cache_info(sum_llc_occu_resc, no_of_bits, span, - MAX_DIFF, MAX_DIFF_PERCENT, runs - 1, - true, true); + return show_results_info(sum_llc_occu_resc, no_of_bits, span, + MAX_DIFF, MAX_DIFF_PERCENT, runs - 1, true); }

void cmt_test_cleanup(void) diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h index 2a08dedcc9e4..4186d61abd33 100644 --- a/tools/testing/selftests/resctrl/resctrl.h +++ b/tools/testing/selftests/resctrl/resctrl.h @@ -113,10 +113,8 @@ unsigned int count_bits(unsigned long n); void cmt_test_cleanup(void); int get_core_sibling(int cpu_no); int measure_llc_resctrl(const char *filename, int bm_pid); -int show_cache_info(unsigned long sum_llc_val, int no_of_bits, - size_t cache_span, unsigned long max_diff, - unsigned long max_diff_percent, unsigned long num_of_runs, - bool platform, bool cmt); +void show_cache_info(int no_of_bits, unsigned long avg_llc_val, + size_t cache_span, bool lines);

/* * cache_portion_size - Calculate the size of a cache portion

Perf event handling has functions that are the sole caller of another perf event handling related function: - reset_enable_llc_perf() calls perf_event_open_llc_miss() - perf_event_measure() calls get_llc_perf()

Remove the extra layer of calls to make the code easier to follow by moving the code into the calling function.

Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com Reviewed-by: Reinette Chatre reinette.chatre@intel.com --- tools/testing/selftests/resctrl/cache.c | 64 ++++++------------------- 1 file changed, 14 insertions(+), 50 deletions(-)

diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c index 11d1a6ce14c2..4f846a2e5e26 100644 --- a/tools/testing/selftests/resctrl/cache.c +++ b/tools/testing/selftests/resctrl/cache.c @@ -29,24 +29,13 @@ static void initialize_perf_event_attr(void) pea_llc_miss.disabled = 1; }

+/* Start counters to log values */ static void ioctl_perf_event_ioc_reset_enable(void) { ioctl(fd_lm, PERF_EVENT_IOC_RESET, 0); ioctl(fd_lm, PERF_EVENT_IOC_ENABLE, 0); }

-static int perf_event_open_llc_miss(pid_t pid, int cpu_no) -{ - fd_lm = perf_event_open(&pea_llc_miss, pid, cpu_no, -1, - PERF_FLAG_FD_CLOEXEC); - if (fd_lm == -1) { - ksft_perror("Error opening leader"); - return -1; - } - - return 0; -} - static void initialize_llc_perf(void) { memset(&pea_llc_miss, 0, sizeof(struct perf_event_attr)); @@ -62,42 +51,13 @@ static void initialize_llc_perf(void)

static int reset_enable_llc_perf(pid_t pid, int cpu_no) { - int ret = 0; - - ret = perf_event_open_llc_miss(pid, cpu_no); - if (ret < 0) - return ret; - - /* Start counters to log values */ - ioctl_perf_event_ioc_reset_enable(); - - return 0; -} - -/* - * get_llc_perf: llc cache miss through perf events - * @llc_perf_miss: LLC miss counter that is filled on success - * - * Perf events like HW_CACHE_MISSES could be used to validate number of - * cache lines allocated. - * - * Return: =0 on success. <0 on failure. - */ -static int get_llc_perf(__u64 *llc_perf_miss) -{ - int ret; - - /* Stop counters after one span to get miss rate */ - - ioctl(fd_lm, PERF_EVENT_IOC_DISABLE, 0); - - ret = read(fd_lm, &rf_cqm, sizeof(struct read_format)); - if (ret == -1) { - ksft_perror("Could not get llc misses through perf"); + fd_lm = perf_event_open(&pea_llc_miss, pid, cpu_no, -1, PERF_FLAG_FD_CLOEXEC); + if (fd_lm == -1) { + ksft_perror("Error opening leader"); return -1; }

- *llc_perf_miss = rf_cqm.values[0].value; + ioctl_perf_event_ioc_reset_enable();

return 0; } @@ -178,14 +138,18 @@ static int print_results_cache(const char *filename, int bm_pid, __u64 llc_value */ static int perf_event_measure(const char *filename, int bm_pid) { - __u64 llc_perf_miss = 0; int ret;

- ret = get_llc_perf(&llc_perf_miss); - if (ret < 0) - return ret; + /* Stop counters after one span to get miss rate */ + ioctl(fd_lm, PERF_EVENT_IOC_DISABLE, 0); + + ret = read(fd_lm, &rf_cqm, sizeof(struct read_format)); + if (ret == -1) { + ksft_perror("Could not get perf value"); + return -1; + }

- return print_results_cache(filename, bm_pid, llc_perf_miss); + return print_results_cache(filename, bm_pid, rf_cqm.values[0].value); }

/*

struct perf_event_attr initialization is spread into perf_event_initialize() and perf_event_attr_initialize() and setting ->config is hardcoded by the deepest level.

perf_event_attr init belongs to perf_event_attr_initialize() so move it entirely there. Rename the other function perf_event_initialized_read_format().

Call each init function directly from the test as they will take different parameters (especially true after the perf related global variables are moved to local variables).

Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com Reviewed-by: Reinette Chatre reinette.chatre@intel.com --- tools/testing/selftests/resctrl/cache.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c index 7f45c4957f81..58ee6f6b0edf 100644 --- a/tools/testing/selftests/resctrl/cache.c +++ b/tools/testing/selftests/resctrl/cache.c @@ -15,8 +15,9 @@ static struct perf_event_read pe_read; static int pe_fd; char llc_occup_path[1024];

-static void perf_event_attr_initialize(void) +static void perf_event_attr_initialize(__u64 config) { + memset(&pea_llc_miss, 0, sizeof(struct perf_event_attr)); pea_llc_miss.type = PERF_TYPE_HARDWARE; pea_llc_miss.size = sizeof(struct perf_event_attr); pea_llc_miss.read_format = PERF_FORMAT_GROUP; @@ -27,6 +28,7 @@ static void perf_event_attr_initialize(void) pea_llc_miss.inherit = 1; pea_llc_miss.exclude_guest = 1; pea_llc_miss.disabled = 1; + pea_llc_miss.config = config; }

/* Start counters to log values */ @@ -36,16 +38,9 @@ static void perf_event_reset_enable(void) ioctl(pe_fd, PERF_EVENT_IOC_ENABLE, 0); }

-static void perf_event_initialize(void) +static void perf_event_initialize_read_format(void) { - memset(&pea_llc_miss, 0, sizeof(struct perf_event_attr)); memset(&pe_read, 0, sizeof(struct perf_event_read)); - - /* Initialize perf_event_attr structures for HW_CACHE_MISSES */ - perf_event_attr_initialize(); - - pea_llc_miss.config = PERF_COUNT_HW_CACHE_MISSES; - pe_read.nr = 1; }

@@ -204,7 +199,8 @@ int cat_val(struct resctrl_val_param *param, size_t span) if (ret) return ret;

- perf_event_initialize(); + perf_event_attr_initialize(PERF_COUNT_HW_CACHE_MISSES); + perf_event_initialize_read_format();

/* Test runs until the callback setup() tells the test to stop. */ while (1) {

The main CAT test function is called cat_val() and resides in cache.c which is illogical.

Rename the function to cat_test() and move it into cat_test.c.

Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com Reviewed-by: Reinette Chatre reinette.chatre@intel.com --- tools/testing/selftests/resctrl/cache.c | 90 ++-------------------- tools/testing/selftests/resctrl/cat_test.c | 73 +++++++++++++++++- tools/testing/selftests/resctrl/resctrl.h | 14 +++- 3 files changed, 90 insertions(+), 87 deletions(-)

diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c index 335415eaa406..319d0cdd7225 100644 --- a/tools/testing/selftests/resctrl/cache.c +++ b/tools/testing/selftests/resctrl/cache.c @@ -3,16 +3,9 @@ #include <stdint.h> #include "resctrl.h"

-struct perf_event_read { - __u64 nr; /* The number of events */ - struct { - __u64 value; /* The value of the event */ - } values[2]; -}; - char llc_occup_path[1024];

-static void perf_event_attr_initialize(struct perf_event_attr *pea, __u64 config) +void perf_event_attr_initialize(struct perf_event_attr *pea, __u64 config) { memset(pea, 0, sizeof(*pea)); pea->type = PERF_TYPE_HARDWARE; @@ -35,13 +28,13 @@ static void perf_event_reset_enable(int pe_fd) ioctl(pe_fd, PERF_EVENT_IOC_ENABLE, 0); }

-static void perf_event_initialize_read_format(struct perf_event_read *pe_read) +void perf_event_initialize_read_format(struct perf_event_read *pe_read) { memset(pe_read, 0, sizeof(*pe_read)); pe_read->nr = 1; }

-static int perf_open(struct perf_event_attr *pea, pid_t pid, int cpu_no) +int perf_open(struct perf_event_attr *pea, pid_t pid, int cpu_no) { int pe_fd;

@@ -130,8 +123,8 @@ static int print_results_cache(const char *filename, int bm_pid, __u64 llc_value * * Return: =0 on success. <0 on failure. */ -static int perf_event_measure(int pe_fd, struct perf_event_read *pe_read, - const char *filename, int bm_pid) +int perf_event_measure(int pe_fd, struct perf_event_read *pe_read, + const char *filename, int bm_pid) { int ret;

@@ -169,79 +162,6 @@ int measure_llc_resctrl(const char *filename, int bm_pid) return print_results_cache(filename, bm_pid, llc_occu_resc); }

-/* - * cache_val: execute benchmark and measure LLC occupancy resctrl - * and perf cache miss for the benchmark - * @param: parameters passed to cache_val() - * @span: buffer size for the benchmark - * - * Return: 0 when the test was run, < 0 on error. - */ -int cat_val(struct resctrl_val_param *param, size_t span) -{ - int memflush = 1, operation = 0, ret = 0; - char *resctrl_val = param->resctrl_val; - struct perf_event_read pe_read; - struct perf_event_attr pea; - pid_t bm_pid; - int pe_fd; - - if (strcmp(param->filename, "") == 0) - sprintf(param->filename, "stdio"); - - bm_pid = getpid(); - - /* Taskset benchmark to specified cpu */ - ret = taskset_benchmark(bm_pid, param->cpu_no); - if (ret) - return ret; - - /* Write benchmark to specified con_mon grp, mon_grp in resctrl FS*/ - ret = write_bm_pid_to_resctrl(bm_pid, param->ctrlgrp, param->mongrp, - resctrl_val); - if (ret) - return ret; - - perf_event_attr_initialize(&pea, PERF_COUNT_HW_CACHE_MISSES); - perf_event_initialize_read_format(&pe_read); - - /* Test runs until the callback setup() tells the test to stop. */ - while (1) { - ret = param->setup(param); - if (ret == END_OF_TESTS) { - ret = 0; - break; - } - if (ret < 0) - break; - - pe_fd = perf_open(&pea, bm_pid, param->cpu_no); - if (pe_fd < 0) { - ret = -1; - break; - } - - if (run_fill_buf(span, memflush, operation, true)) { - fprintf(stderr, "Error-running fill buffer\n"); - ret = -1; - goto pe_close; - } - - sleep(1); - ret = perf_event_measure(pe_fd, &pe_read, param->filename, bm_pid); - if (ret) - goto pe_close; - - close(pe_fd); - } - - return ret; - -pe_close: - close(pe_fd); - return ret; -} - /* * show_cache_info - Show generic cache test information * @no_of_bits: Number of bits diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c index 516eaa08e967..bfb607b13491 100644 --- a/tools/testing/selftests/resctrl/cat_test.c +++ b/tools/testing/selftests/resctrl/cat_test.c @@ -111,6 +111,77 @@ void cat_test_cleanup(void) remove(RESULT_FILE_NAME2); }

+/* + * cat_test - Execute CAT benchmark and measure cache misses + * @param: Parameters passed to cat_test() + * @span: Buffer size for the benchmark + * + * Return: 0 when the test was run, < 0 on error. + */ +static int cat_test(struct resctrl_val_param *param, size_t span) +{ + int memflush = 1, operation = 0, ret = 0; + char *resctrl_val = param->resctrl_val; + struct perf_event_read pe_read; + struct perf_event_attr pea; + pid_t bm_pid; + int pe_fd; + + if (strcmp(param->filename, "") == 0) + sprintf(param->filename, "stdio"); + + bm_pid = getpid(); + + /* Taskset benchmark to specified cpu */ + ret = taskset_benchmark(bm_pid, param->cpu_no); + if (ret) + return ret; + + /* Write benchmark to specified con_mon grp, mon_grp in resctrl FS*/ + ret = write_bm_pid_to_resctrl(bm_pid, param->ctrlgrp, param->mongrp, + resctrl_val); + if (ret) + return ret; + + perf_event_attr_initialize(&pea, PERF_COUNT_HW_CACHE_MISSES); + perf_event_initialize_read_format(&pe_read); + + /* Test runs until the callback setup() tells the test to stop. */ + while (1) { + ret = param->setup(param); + if (ret == END_OF_TESTS) { + ret = 0; + break; + } + if (ret < 0) + break; + pe_fd = perf_open(&pea, bm_pid, param->cpu_no); + if (pe_fd < 0) { + ret = -1; + break; + } + + if (run_fill_buf(span, memflush, operation, true)) { + fprintf(stderr, "Error-running fill buffer\n"); + ret = -1; + goto pe_close; + } + + sleep(1); + ret = perf_event_measure(pe_fd, &pe_read, param->filename, bm_pid); + if (ret) + goto pe_close; + + close(pe_fd); + } + + return ret; + +pe_close: + close(pe_fd); + return ret; +} + int cat_perf_miss_val(int cpu_no, int n, char *cache_type) { unsigned long full_cache_mask, long_mask; @@ -194,7 +265,7 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)

remove(param.filename);

- ret = cat_val(&param, span); + ret = cat_test(&param, span); if (ret == 0) ret = check_results(&param, span);

diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h index 9aac7b66ada8..6635d5c1908a 100644 --- a/tools/testing/selftests/resctrl/resctrl.h +++ b/tools/testing/selftests/resctrl/resctrl.h @@ -66,6 +66,13 @@ struct resctrl_val_param { int (*setup)(struct resctrl_val_param *param); };

+struct perf_event_read { + __u64 nr; /* The number of events */ + struct { + __u64 value; /* The value of the event */ + } values[2]; +}; + #define MBM_STR "mbm" #define MBA_STR "mba" #define CMT_STR "cmt" @@ -105,13 +112,18 @@ int get_cache_size(int cpu_no, const char *cache_type, unsigned long *cache_size void ctrlc_handler(int signum, siginfo_t *info, void *ptr); int signal_handler_register(void); void signal_handler_unregister(void); -int cat_val(struct resctrl_val_param *param, size_t span); void cat_test_cleanup(void); int cat_perf_miss_val(int cpu_no, int no_of_bits, char *cache_type); int cmt_resctrl_val(int cpu_no, int n, const char * const *benchmark_cmd); unsigned int count_bits(unsigned long n); void cmt_test_cleanup(void); int get_core_sibling(int cpu_no); + +void perf_event_attr_initialize(struct perf_event_attr *pea, __u64 config); +void perf_event_initialize_read_format(struct perf_event_read *pe_read); +int perf_open(struct perf_event_attr *pea, pid_t pid, int cpu_no); +int perf_event_measure(int pe_fd, struct perf_event_read *pe_read, + const char *filename, int bm_pid); int measure_llc_resctrl(const char *filename, int bm_pid); void show_cache_info(int no_of_bits, __u64 avg_llc_val, size_t cache_span, bool lines);

The fill_buf code prevents compiler optimizating the entire read loop away by writing the final value of the variable into a file. While it achieves the goal, writing into a file requires significant amount of work within the innermost test loop and also error handling.

A simpler approach is to take advantage of volatile. Writing through a pointer to a volatile variable is enough to prevent compiler from optimizing the write away, and therefore compiler cannot remove the read loop either.

Add a volatile 'value_sink' into resctrl_tests.c and make fill_buf to write into it. As a result, the error handling in fill_buf.c can be simplified.

Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com ---

v3: - Altered changelog's wording to not say "a volatile pointer" --- tools/testing/selftests/resctrl/fill_buf.c | 26 ++++--------------- tools/testing/selftests/resctrl/resctrl.h | 7 +++++ .../testing/selftests/resctrl/resctrl_tests.c | 4 +++ 3 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/tools/testing/selftests/resctrl/fill_buf.c b/tools/testing/selftests/resctrl/fill_buf.c index 635f938b11f0..8fe9574db9d8 100644 --- a/tools/testing/selftests/resctrl/fill_buf.c +++ b/tools/testing/selftests/resctrl/fill_buf.c @@ -78,10 +78,9 @@ static void fill_one_span_write(unsigned char *buf, size_t buf_size) } }

-static int fill_cache_read(unsigned char *buf, size_t buf_size, bool once) +static void fill_cache_read(unsigned char *buf, size_t buf_size, bool once) { int ret = 0; - FILE *fp;

while (1) { ret = fill_one_span_read(buf, buf_size); @@ -90,26 +89,16 @@ static int fill_cache_read(unsigned char *buf, size_t buf_size, bool once) }

/* Consume read result so that reading memory is not optimized out. */ - fp = fopen("/dev/null", "w"); - if (!fp) { - ksft_perror("Unable to write to /dev/null"); - return -1; - } - fprintf(fp, "Sum: %d ", ret); - fclose(fp); - - return 0; + *value_sink = ret; }

-static int fill_cache_write(unsigned char *buf, size_t buf_size, bool once) +static void fill_cache_write(unsigned char *buf, size_t buf_size, bool once) { while (1) { fill_one_span_write(buf, buf_size); if (once) break; } - - return 0; }

static unsigned char *alloc_buffer(size_t buf_size, int memflush) @@ -143,21 +132,16 @@ static unsigned char *alloc_buffer(size_t buf_size, int memflush) int run_fill_buf(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); + fill_cache_read(buf, buf_size, once); else - ret = fill_cache_write(buf, buf_size, once); + fill_cache_write(buf, buf_size, once); free(buf); - if (ret) { - printf("\n Error in fill cache\n"); - return -1; - }

return 0; } diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h index 419669502959..4e1f7a4bbaf5 100644 --- a/tools/testing/selftests/resctrl/resctrl.h +++ b/tools/testing/selftests/resctrl/resctrl.h @@ -78,6 +78,13 @@ struct perf_event_read { #define CMT_STR "cmt" #define CAT_STR "cat"

+/* + * Memory location that consumes values compiler must not optimize away. + * Volatile ensures writes to this location cannot be optimized away by + * compiler. + */ +extern volatile int *value_sink; + extern pid_t bm_pid, ppid;

extern char llc_occup_path[1024]; diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c index 2bbe3045a018..f0d573aa363b 100644 --- a/tools/testing/selftests/resctrl/resctrl_tests.c +++ b/tools/testing/selftests/resctrl/resctrl_tests.c @@ -10,6 +10,10 @@ */ #include "resctrl.h"

+/* Volatile memory sink to prevent compiler optimizations */ +static volatile int sink_target; +volatile int *value_sink = &sink_target; + static int detect_vendor(void) { FILE *inf = fopen("/proc/cpuinfo", "r");

When reading memory in order, HW prefetching optimizations will interfere with measuring how caches and memory are being accessed. This adds noise into the results.

Change the fill_buf reading loop to not use an obvious in-order access using multiply by a prime and modulo.

Using a prime multiplier with modulo ensures the entire buffer is eventually read. 23 is small enough that the reads are spread out but wrapping does not occur very frequently (wrapping too often can trigger L2 hits more frequently which causes noise to the test because getting the data from LLC is not required).

It was discovered that not all primes work equally well and some can cause wildly unstable results (e.g., in an earlier version of this patch, the reads were done in reversed order and 59 was used as the prime resulting in unacceptably high and unstable results in MBA and MBM test on some architectures).

Link: https://lore.kernel.org/linux-kselftest/TYAPR01MB6330025B5E6537F94DA49ACB8B4... Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com Reviewed-by: Reinette Chatre reinette.chatre@intel.com --- tools/testing/selftests/resctrl/fill_buf.c | 38 +++++++++++++++++----- 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/resctrl/fill_buf.c b/tools/testing/selftests/resctrl/fill_buf.c index 8fe9574db9d8..93a3d408339c 100644 --- a/tools/testing/selftests/resctrl/fill_buf.c +++ b/tools/testing/selftests/resctrl/fill_buf.c @@ -51,16 +51,38 @@ static void mem_flush(unsigned char *buf, size_t buf_size) sb(); }

+/* + * Buffer index step advance to workaround HW prefetching interfering with + * the measurements. + * + * Must be a prime to step through all indexes of the buffer. + * + * Some primes work better than others on some architectures (from MBA/MBM + * result stability point of view). + */ +#define FILL_IDX_MULT 23 + static int fill_one_span_read(unsigned char *buf, size_t buf_size) { - unsigned char *end_ptr = buf + buf_size; - unsigned char sum, *p; - - sum = 0; - p = buf; - while (p < end_ptr) { - sum += *p; - p += (CL_SIZE / 2); + unsigned int size = buf_size / (CL_SIZE / 2); + unsigned int i, idx = 0; + unsigned char sum = 0; + + /* + * Read the buffer in an order that is unexpected by HW prefetching + * optimizations to prevent them interfering with the caching pattern. + * + * The read order is (in terms of halves of cachelines): + * i * FILL_IDX_MULT % size + * The formula is open-coded below to avoiding modulo inside the loop + * as it improves MBA/MBM result stability on some architectures. + */ + for (i = 0; i < size; i++) { + sum += buf[idx * (CL_SIZE / 2)]; + + idx += FILL_IDX_MULT; + while (idx >= size) + idx -= size; }

return sum;

resctrl_tests reads a set of parameters and passes them individually for each tests which causes variations in the call signature between the tests.

Add struct input_params to hold all input parameters. It can be easily passed to every test without varying the call signature.

Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com ---

v3: - Move uparams clear into init function --- tools/testing/selftests/resctrl/cat_test.c | 20 +++--- tools/testing/selftests/resctrl/cmt_test.c | 16 ++--- tools/testing/selftests/resctrl/mba_test.c | 9 ++- tools/testing/selftests/resctrl/mbm_test.c | 9 ++- tools/testing/selftests/resctrl/resctrl.h | 28 ++++++--- .../testing/selftests/resctrl/resctrl_tests.c | 63 +++++++++++-------- tools/testing/selftests/resctrl/resctrl_val.c | 21 ++++--- 7 files changed, 95 insertions(+), 71 deletions(-)

diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c index fa95433297c9..272132ff7337 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 + * @uparams: User supplied parameters * @param: Parameters passed to cat_test() * @span: Buffer size for the benchmark * @current_mask Start mask for the first iteration @@ -151,7 +152,8 @@ void cat_test_cleanup(void) * * Return: 0 when the test was run, < 0 on error. */ -static int cat_test(struct resctrl_val_param *param, size_t span, unsigned long current_mask) +static int cat_test(const struct user_params *uparams, struct resctrl_val_param *param, + size_t span, unsigned long current_mask) { char *resctrl_val = param->resctrl_val; struct perf_event_read pe_read; @@ -168,7 +170,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, &old_affinity); + ret = taskset_benchmark(bm_pid, uparams->cpu, &old_affinity); if (ret) return ret;

@@ -180,7 +182,7 @@ static int cat_test(struct resctrl_val_param *param, size_t span, unsigned long

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); + pe_fd = perf_open(&pea, bm_pid, uparams->cpu); if (pe_fd < 0) { ret = -1; goto reset_affinity; @@ -194,11 +196,11 @@ static int cat_test(struct resctrl_val_param *param, size_t span, unsigned long

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

@@ -229,10 +231,11 @@ static int cat_test(struct resctrl_val_param *param, size_t span, unsigned long return ret; }

-int cat_perf_miss_val(int cpu_no, int n, char *cache_type) +int cat_perf_miss_val(const struct user_params *uparams, char *cache_type) { unsigned long long_mask, start_mask, full_cache_mask; unsigned long cache_total_size = 0; + int n = uparams->bits; unsigned int start; int count_of_bits; size_t span; @@ -247,7 +250,7 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type) return ret;

/* Get L3/L2 cache size */ - ret = get_cache_size(cpu_no, cache_type, &cache_total_size); + ret = get_cache_size(uparams->cpu, cache_type, &cache_total_size); if (ret) return ret; ksft_print_msg("Cache size :%lu\n", cache_total_size); @@ -267,7 +270,6 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)

struct resctrl_val_param param = { .resctrl_val = CAT_STR, - .cpu_no = cpu_no, .ctrlgrp = "c1", .filename = RESULT_FILE_NAME, .num_of_runs = 0, @@ -277,7 +279,7 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)

remove(param.filename);

- ret = cat_test(&param, span, start_mask); + ret = cat_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 604690cd8b3e..a5d0c0878c42 100644 --- a/tools/testing/selftests/resctrl/cmt_test.c +++ b/tools/testing/selftests/resctrl/cmt_test.c @@ -16,7 +16,7 @@ #define MAX_DIFF 2000000 #define MAX_DIFF_PERCENT 15

-static int cmt_setup(struct resctrl_val_param *p) +static int cmt_setup(const struct user_params *uparams, struct resctrl_val_param *p) { /* Run NUM_OF_RUNS times */ if (p->num_of_runs >= NUM_OF_RUNS) @@ -94,11 +94,12 @@ void cmt_test_cleanup(void) remove(RESULT_FILE_NAME); }

-int cmt_resctrl_val(int cpu_no, int n, const char * const *benchmark_cmd) +int cmt_resctrl_val(const struct user_params *uparams) { - const char * const *cmd = benchmark_cmd; + const char * const *cmd = uparams->benchmark_cmd; const char *new_cmd[BENCHMARK_ARGS]; unsigned long cache_total_size = 0; + int n = uparams->bits ? : 5; unsigned long long_mask; char *span_str = NULL; int count_of_bits; @@ -109,7 +110,7 @@ int cmt_resctrl_val(int cpu_no, int n, const char * const *benchmark_cmd) if (ret) return ret;

- ret = get_cache_size(cpu_no, "L3", &cache_total_size); + ret = get_cache_size(uparams->cpu, "L3", &cache_total_size); if (ret) return ret; ksft_print_msg("Cache size :%lu\n", cache_total_size); @@ -126,7 +127,6 @@ int cmt_resctrl_val(int cpu_no, int n, const char * const *benchmark_cmd) .resctrl_val = CMT_STR, .ctrlgrp = "c1", .mongrp = "m1", - .cpu_no = cpu_no, .filename = RESULT_FILE_NAME, .mask = ~(long_mask << n) & long_mask, .num_of_runs = 0, @@ -137,8 +137,8 @@ int cmt_resctrl_val(int cpu_no, int n, const char * const *benchmark_cmd)

if (strcmp(cmd[0], "fill_buf") == 0) { /* Duplicate the command to be able to replace span in it */ - for (i = 0; benchmark_cmd[i]; i++) - new_cmd[i] = benchmark_cmd[i]; + for (i = 0; uparams->benchmark_cmd[i]; i++) + new_cmd[i] = uparams->benchmark_cmd[i]; new_cmd[i] = NULL;

ret = asprintf(&span_str, "%zu", span); @@ -150,7 +150,7 @@ int cmt_resctrl_val(int cpu_no, int n, const char * const *benchmark_cmd)

remove(RESULT_FILE_NAME);

- ret = resctrl_val(cmd, &param); + ret = resctrl_val(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 e907adf7cd25..8e54bc508a75 100644 --- a/tools/testing/selftests/resctrl/mba_test.c +++ b/tools/testing/selftests/resctrl/mba_test.c @@ -22,7 +22,7 @@ * con_mon grp, mon_grp in resctrl FS. * For each allocation, run 5 times in order to get average values. */ -static int mba_setup(struct resctrl_val_param *p) +static int mba_setup(const struct user_params *uparams, struct resctrl_val_param *p) { static int runs_per_allocation, allocation = 100; char allocation_str[64]; @@ -40,7 +40,7 @@ static int mba_setup(struct resctrl_val_param *p)

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

- ret = write_schemata(p->ctrlgrp, allocation_str, p->cpu_no, + ret = write_schemata(p->ctrlgrp, allocation_str, uparams->cpu, p->resctrl_val); if (ret < 0) return ret; @@ -141,13 +141,12 @@ void mba_test_cleanup(void) remove(RESULT_FILE_NAME); }

-int mba_schemata_change(int cpu_no, const char * const *benchmark_cmd) +int mba_schemata_change(const struct user_params *uparams) { struct resctrl_val_param param = { .resctrl_val = MBA_STR, .ctrlgrp = "c1", .mongrp = "m1", - .cpu_no = cpu_no, .filename = RESULT_FILE_NAME, .bw_report = "reads", .setup = mba_setup @@ -156,7 +155,7 @@ int mba_schemata_change(int cpu_no, const char * const *benchmark_cmd)

remove(RESULT_FILE_NAME);

- ret = resctrl_val(benchmark_cmd, &param); + ret = resctrl_val(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 721b3ecbc158..ea951cfae5fe 100644 --- a/tools/testing/selftests/resctrl/mbm_test.c +++ b/tools/testing/selftests/resctrl/mbm_test.c @@ -86,7 +86,7 @@ static int check_results(size_t span) return ret; }

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

@@ -96,7 +96,7 @@ static int mbm_setup(struct resctrl_val_param *p)

/* 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", p->cpu_no, + ret = write_schemata(p->ctrlgrp, "100", uparams->cpu, p->resctrl_val);

p->num_of_runs++; @@ -109,13 +109,12 @@ void mbm_test_cleanup(void) remove(RESULT_FILE_NAME); }

-int mbm_bw_change(int cpu_no, const char * const *benchmark_cmd) +int mbm_bw_change(const struct user_params *uparams) { struct resctrl_val_param param = { .resctrl_val = MBM_STR, .ctrlgrp = "c1", .mongrp = "m1", - .cpu_no = cpu_no, .filename = RESULT_FILE_NAME, .bw_report = "reads", .setup = mbm_setup @@ -124,7 +123,7 @@ int mbm_bw_change(int cpu_no, const char * const *benchmark_cmd)

remove(RESULT_FILE_NAME);

- ret = resctrl_val(benchmark_cmd, &param); + ret = resctrl_val(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 da62f4cd5add..3155631cd3c6 100644 --- a/tools/testing/selftests/resctrl/resctrl.h +++ b/tools/testing/selftests/resctrl/resctrl.h @@ -44,12 +44,23 @@ exit(EXIT_FAILURE); \ } while (0)

+/* + * user_params: User supplied parameters + * @cpu: CPU number to which the benchmark will be bound to + * @bits: Number of bits used for cache allocation size + * @benchmark_cmd: Benchmark command to run during (some of the) tests + */ +struct user_params { + int cpu; + int bits; + const char *benchmark_cmd[BENCHMARK_ARGS]; +}; + /* * resctrl_val_param: resctrl test parameters * @resctrl_val: Resctrl feature (Eg: mbm, mba.. etc) * @ctrlgrp: Name of the control monitor group (con_mon grp) * @mongrp: Name of the monitor group (mon grp) - * @cpu_no: CPU number to which the benchmark would be binded * @filename: Name of file to which the o/p should be written * @bw_report: Bandwidth report type (reads vs writes) * @setup: Call back function to setup test environment @@ -58,12 +69,12 @@ struct resctrl_val_param { char *resctrl_val; char ctrlgrp[64]; char mongrp[64]; - int cpu_no; char filename[64]; char *bw_report; unsigned long mask; int num_of_runs; - int (*setup)(struct resctrl_val_param *param); + int (*setup)(const struct user_params *uparams, + struct resctrl_val_param *param); };

struct perf_event_read { @@ -110,11 +121,12 @@ 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 char * const *benchmark_cmd, struct resctrl_val_param *param); -int mbm_bw_change(int cpu_no, const char * const *benchmark_cmd); +int resctrl_val(const struct user_params *uparams, const char * const *benchmark_cmd, + struct resctrl_val_param *param); +int mbm_bw_change(const struct user_params *uparams); void tests_cleanup(void); void mbm_test_cleanup(void); -int mba_schemata_change(int cpu_no, const char * const *benchmark_cmd); +int mba_schemata_change(const struct user_params *uparams); void mba_test_cleanup(void); unsigned long create_bit_mask(unsigned int start, unsigned int len); unsigned int count_contiguous_bits(unsigned long val, unsigned int *start); @@ -125,8 +137,8 @@ void ctrlc_handler(int signum, siginfo_t *info, void *ptr); int signal_handler_register(void); void signal_handler_unregister(void); void cat_test_cleanup(void); -int cat_perf_miss_val(int cpu_no, int no_of_bits, char *cache_type); -int cmt_resctrl_val(int cpu_no, int n, const char * const *benchmark_cmd); +int cat_perf_miss_val(const struct user_params *uparams, char *cache_type); +int cmt_resctrl_val(const struct user_params *uparams); unsigned int count_bits(unsigned long n); void cmt_test_cleanup(void);

diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c index f0d573aa363b..3059a124ac59 100644 --- a/tools/testing/selftests/resctrl/resctrl_tests.c +++ b/tools/testing/selftests/resctrl/resctrl_tests.c @@ -96,7 +96,7 @@ static void test_cleanup(void) signal_handler_unregister(); }

-static void run_mbm_test(const char * const *benchmark_cmd, int cpu_no) +static void run_mbm_test(const struct user_params *uparams) { int res;

@@ -114,7 +114,7 @@ static void run_mbm_test(const char * const *benchmark_cmd, int cpu_no) goto cleanup; }

- res = mbm_bw_change(cpu_no, benchmark_cmd); + res = mbm_bw_change(uparams); ksft_test_result(!res, "MBM: bw change\n"); if ((get_vendor() == ARCH_INTEL) && res) ksft_print_msg("Intel MBM may be inaccurate when Sub-NUMA Clustering is enabled. Check BIOS configuration.\n"); @@ -123,7 +123,7 @@ static void run_mbm_test(const char * const *benchmark_cmd, int cpu_no) test_cleanup(); }

-static void run_mba_test(const char * const *benchmark_cmd, int cpu_no) +static void run_mba_test(const struct user_params *uparams) { int res;

@@ -141,14 +141,14 @@ static void run_mba_test(const char * const *benchmark_cmd, int cpu_no) goto cleanup; }

- res = mba_schemata_change(cpu_no, benchmark_cmd); + res = mba_schemata_change(uparams); ksft_test_result(!res, "MBA: schemata change\n");

cleanup: test_cleanup(); }

-static void run_cmt_test(const char * const *benchmark_cmd, int cpu_no) +static void run_cmt_test(const struct user_params *uparams) { int res;

@@ -165,7 +165,7 @@ static void run_cmt_test(const char * const *benchmark_cmd, int cpu_no) goto cleanup; }

- res = cmt_resctrl_val(cpu_no, 5, benchmark_cmd); + res = cmt_resctrl_val(uparams); ksft_test_result(!res, "CMT: test\n"); if ((get_vendor() == ARCH_INTEL) && res) ksft_print_msg("Intel CMT may be inaccurate when Sub-NUMA Clustering is enabled. Check BIOS configuration.\n"); @@ -174,7 +174,7 @@ static void run_cmt_test(const char * const *benchmark_cmd, int cpu_no) test_cleanup(); }

-static void run_cat_test(int cpu_no, int no_of_bits) +static void run_cat_test(const struct user_params *uparams) { int res;

@@ -190,22 +190,31 @@ static void run_cat_test(int cpu_no, int no_of_bits) goto cleanup; }

- res = cat_perf_miss_val(cpu_no, no_of_bits, "L3"); + res = cat_perf_miss_val(uparams, "L3"); ksft_test_result(!res, "CAT: test\n");

cleanup: test_cleanup(); }

+static void init_user_params(struct user_params *uparams) +{ + memset(uparams, 0, sizeof(*uparams)); + + uparams->cpu = 1; + uparams->bits = 0; +} + int main(int argc, char **argv) { bool mbm_test = true, mba_test = true, cmt_test = true; - const char *benchmark_cmd[BENCHMARK_ARGS] = {}; - int c, cpu_no = 1, i, no_of_bits = 0; + struct user_params uparams; char *span_str = NULL; bool cat_test = true; int tests = 0; - int ret; + int ret, c, i; + + init_user_params(&uparams);

while ((c = getopt(argc, argv, "ht:b:n:p:")) != -1) { char *token; @@ -223,8 +232,8 @@ int main(int argc, char **argv)

/* Extract benchmark command from command line. */ for (i = 0; i < argc - optind; i++) - benchmark_cmd[i] = argv[i + optind]; - benchmark_cmd[i] = NULL; + uparams.benchmark_cmd[i] = argv[i + optind]; + uparams.benchmark_cmd[i] = NULL;

goto last_arg; case 't': @@ -256,11 +265,11 @@ int main(int argc, char **argv) } break; case 'p': - cpu_no = atoi(optarg); + uparams.cpu = atoi(optarg); break; case 'n': - no_of_bits = atoi(optarg); - if (no_of_bits <= 0) { + uparams.bits = atoi(optarg); + if (uparams.bits <= 0) { printf("Bail out! invalid argument for no_of_bits\n"); return -1; } @@ -295,32 +304,32 @@ int main(int argc, char **argv)

filter_dmesg();

- if (!benchmark_cmd[0]) { + if (!uparams.benchmark_cmd[0]) { /* If no benchmark is given by "-b" argument, use fill_buf. */ - benchmark_cmd[0] = "fill_buf"; + uparams.benchmark_cmd[0] = "fill_buf"; ret = asprintf(&span_str, "%u", DEFAULT_SPAN); if (ret < 0) ksft_exit_fail_msg("Out of memory!\n"); - benchmark_cmd[1] = span_str; - benchmark_cmd[2] = "1"; - benchmark_cmd[3] = "0"; - benchmark_cmd[4] = "false"; - benchmark_cmd[5] = NULL; + uparams.benchmark_cmd[1] = span_str; + uparams.benchmark_cmd[2] = "1"; + uparams.benchmark_cmd[3] = "0"; + uparams.benchmark_cmd[4] = "false"; + uparams.benchmark_cmd[5] = NULL; }

ksft_set_plan(tests ? : 4);

if (mbm_test) - run_mbm_test(benchmark_cmd, cpu_no); + run_mbm_test(&uparams);

if (mba_test) - run_mba_test(benchmark_cmd, cpu_no); + run_mba_test(&uparams);

if (cmt_test) - run_cmt_test(benchmark_cmd, cpu_no); + run_cmt_test(&uparams);

if (cat_test) - run_cat_test(cpu_no, no_of_bits); + run_cat_test(&uparams);

free(span_str); ksft_finished(); diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c index 4aed974efa0f..ba4a33f3c13f 100644 --- a/tools/testing/selftests/resctrl/resctrl_val.c +++ b/tools/testing/selftests/resctrl/resctrl_val.c @@ -593,8 +593,9 @@ static void initialize_llc_occu_resctrl(const char *ctrlgrp, const char *mongrp, set_cmt_path(ctrlgrp, mongrp, resource_id); }

-static int -measure_vals(struct resctrl_val_param *param, unsigned long *bw_resc_start) +static int measure_vals(const struct user_params *uparams, + struct resctrl_val_param *param, + unsigned long *bw_resc_start) { unsigned long bw_resc, bw_resc_end; float bw_imc; @@ -607,7 +608,7 @@ measure_vals(struct resctrl_val_param *param, unsigned long *bw_resc_start) * Compare the two values to validate resctrl value. * It takes 1sec to measure the data. */ - ret = get_mem_bw_imc(param->cpu_no, param->bw_report, &bw_imc); + ret = get_mem_bw_imc(uparams->cpu, param->bw_report, &bw_imc); if (ret < 0) return ret;

@@ -683,12 +684,14 @@ static void run_benchmark(int signum, siginfo_t *info, void *ucontext) /* * resctrl_val: execute benchmark and measure memory bandwidth on * the benchmark + * @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 char * const *benchmark_cmd, struct resctrl_val_param *param) +int resctrl_val(const struct user_params *uparams, const char * const *benchmark_cmd, + struct resctrl_val_param *param) { char *resctrl_val = param->resctrl_val; unsigned long bw_resc_start = 0; @@ -777,7 +780,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, NULL); + ret = taskset_benchmark(bm_pid, uparams->cpu, NULL); if (ret) goto out;

@@ -794,10 +797,10 @@ int resctrl_val(const char * const *benchmark_cmd, struct resctrl_val_param *par goto out;

initialize_mem_bw_resctrl(param->ctrlgrp, param->mongrp, - param->cpu_no, resctrl_val); + uparams->cpu, resctrl_val); } else if (!strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR))) initialize_llc_occu_resctrl(param->ctrlgrp, param->mongrp, - param->cpu_no, resctrl_val); + uparams->cpu, resctrl_val);

/* Parent waits for child to be ready. */ close(pipefd[1]); @@ -823,7 +826,7 @@ int resctrl_val(const char * const *benchmark_cmd, struct resctrl_val_param *par

/* Test runs until the callback setup() tells the test to stop. */ while (1) { - ret = param->setup(param); + ret = param->setup(uparams, param); if (ret == END_OF_TESTS) { ret = 0; break; @@ -833,7 +836,7 @@ int resctrl_val(const char * const *benchmark_cmd, struct resctrl_val_param *par

if (!strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)) || !strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR))) { - ret = measure_vals(param, &bw_resc_start); + ret = measure_vals(uparams, param, &bw_resc_start); if (ret) break; } else if (!strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR))) {

Each test currently has a "run test" function in per test file and another resctrl_tests.c. The functions in resctrl_tests.c are almost identical.

Generalize the one in resctrl_tests.c such that it can be shared between all of the tests. It makes adding new tests easier and removes the per test if () forests.

Also add comment to CPU vendor IDs that they must be defined as bits for a bitmask.

Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com ---

v3: - Add CPU vendor ID bitmask comment - Use test_resource_feature_check(test) in CMT --- tools/testing/selftests/resctrl/cat_test.c | 18 +- tools/testing/selftests/resctrl/cmt_test.c | 17 +- tools/testing/selftests/resctrl/mba_test.c | 16 +- tools/testing/selftests/resctrl/mbm_test.c | 18 +- tools/testing/selftests/resctrl/resctrl.h | 35 +++- .../testing/selftests/resctrl/resctrl_tests.c | 160 ++++++------------ tools/testing/selftests/resctrl/resctrlfs.c | 5 + 7 files changed, 148 insertions(+), 121 deletions(-)

diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c index 272132ff7337..a9b4583620d0 100644 --- a/tools/testing/selftests/resctrl/cat_test.c +++ b/tools/testing/selftests/resctrl/cat_test.c @@ -231,7 +231,7 @@ static int cat_test(const struct user_params *uparams, struct resctrl_val_param return ret; }

-int cat_perf_miss_val(const struct user_params *uparams, char *cache_type) +static int cat_run_test(const struct resctrl_test *test, const struct user_params *uparams) { unsigned long long_mask, start_mask, full_cache_mask; unsigned long cache_total_size = 0; @@ -241,16 +241,16 @@ int cat_perf_miss_val(const struct user_params *uparams, char *cache_type) size_t span; int ret;

- ret = get_full_cbm(cache_type, &full_cache_mask); + ret = get_full_cbm(test->resource, &full_cache_mask); if (ret) return ret; /* Get the largest contiguous exclusive portion of the cache */ - ret = get_mask_no_shareable(cache_type, &long_mask); + ret = get_mask_no_shareable(test->resource, &long_mask); if (ret) return ret;

/* Get L3/L2 cache size */ - ret = get_cache_size(uparams->cpu, cache_type, &cache_total_size); + ret = get_cache_size(uparams->cpu, test->resource, &cache_total_size); if (ret) return ret; ksft_print_msg("Cache size :%lu\n", cache_total_size); @@ -283,9 +283,17 @@ int cat_perf_miss_val(const struct user_params *uparams, char *cache_type) if (ret) goto out;

- ret = check_results(&param, cache_type, cache_total_size, full_cache_mask, start_mask); + ret = check_results(&param, test->resource, + cache_total_size, full_cache_mask, start_mask); out: cat_test_cleanup();

return ret; } + +struct resctrl_test l3_cat_test = { + .name = "CAT", + .resource = "L3", + .feature_check = test_resource_feature_check, + .run_test = cat_run_test, +}; diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c index a5d0c0878c42..c01980039118 100644 --- a/tools/testing/selftests/resctrl/cmt_test.c +++ b/tools/testing/selftests/resctrl/cmt_test.c @@ -94,7 +94,7 @@ void cmt_test_cleanup(void) remove(RESULT_FILE_NAME); }

-int cmt_resctrl_val(const struct user_params *uparams) +static int cmt_run_test(const struct resctrl_test *test, const struct user_params *uparams) { const char * const *cmd = uparams->benchmark_cmd; const char *new_cmd[BENCHMARK_ARGS]; @@ -155,6 +155,8 @@ int cmt_resctrl_val(const struct user_params *uparams) goto out;

ret = check_results(&param, span, n); + if (ret && (get_vendor() == ARCH_INTEL)) + ksft_print_msg("Intel CMT may be inaccurate when Sub-NUMA Clustering is enabled. Check BIOS configuration.\n");

out: cmt_test_cleanup(); @@ -162,3 +164,16 @@ int cmt_resctrl_val(const struct user_params *uparams)

return ret; } + +static bool cmt_feature_check(const struct resctrl_test *test) +{ + return test_resource_feature_check(test) && + validate_resctrl_feature_request("L3_MON", "llc_occupancy"); +} + +struct resctrl_test cmt_test = { + .name = "CMT", + .resource = "L3", + .feature_check = cmt_feature_check, + .run_test = cmt_run_test, +}; diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c index 8e54bc508a75..c218af24f91d 100644 --- a/tools/testing/selftests/resctrl/mba_test.c +++ b/tools/testing/selftests/resctrl/mba_test.c @@ -141,7 +141,7 @@ void mba_test_cleanup(void) remove(RESULT_FILE_NAME); }

-int mba_schemata_change(const struct user_params *uparams) +static int mba_run_test(const struct resctrl_test *test, const struct user_params *uparams) { struct resctrl_val_param param = { .resctrl_val = MBA_STR, @@ -166,3 +166,17 @@ int mba_schemata_change(const struct user_params *uparams)

return ret; } + +static bool mba_feature_check(const struct resctrl_test *test) +{ + return test_resource_feature_check(test) && + validate_resctrl_feature_request("L3_MON", "mbm_local_bytes"); +} + +struct resctrl_test mba_test = { + .name = "MBA", + .resource = "MB", + .vendor_specific = ARCH_INTEL, + .feature_check = mba_feature_check, + .run_test = mba_run_test, +}; diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c index ea951cfae5fe..919b10459c22 100644 --- a/tools/testing/selftests/resctrl/mbm_test.c +++ b/tools/testing/selftests/resctrl/mbm_test.c @@ -109,7 +109,7 @@ void mbm_test_cleanup(void) remove(RESULT_FILE_NAME); }

-int mbm_bw_change(const struct user_params *uparams) +static int mbm_run_test(const struct resctrl_test *test, const struct user_params *uparams) { struct resctrl_val_param param = { .resctrl_val = MBM_STR, @@ -128,9 +128,25 @@ int mbm_bw_change(const struct user_params *uparams) goto out;

ret = check_results(DEFAULT_SPAN); + if (ret && (get_vendor() == ARCH_INTEL)) + ksft_print_msg("Intel MBM may be inaccurate when Sub-NUMA Clustering is enabled. Check BIOS configuration.\n");

out: mbm_test_cleanup();

return ret; } + +static bool mbm_feature_check(const struct resctrl_test *test) +{ + return validate_resctrl_feature_request("L3_MON", "mbm_total_bytes") && + validate_resctrl_feature_request("L3_MON", "mbm_local_bytes"); +} + +struct resctrl_test mbm_test = { + .name = "MBM", + .resource = "MB", + .vendor_specific = ARCH_INTEL, + .feature_check = mbm_feature_check, + .run_test = mbm_run_test, +}; diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h index 3155631cd3c6..9a6b304044cc 100644 --- a/tools/testing/selftests/resctrl/resctrl.h +++ b/tools/testing/selftests/resctrl/resctrl.h @@ -28,6 +28,12 @@ #define PHYS_ID_PATH "/sys/devices/system/cpu/cpu" #define INFO_PATH "/sys/fs/resctrl/info"

+/* + * CPU vendor IDs + * + * Define as bits because they're used for vendor_specific bitmask in + * the struct resctrl_test. + */ #define ARCH_INTEL 1 #define ARCH_AMD 2

@@ -56,6 +62,25 @@ struct user_params { const char *benchmark_cmd[BENCHMARK_ARGS]; };

+/* + * resctrl_test: resctrl test definition + * @name: Test name + * @resource: Resource to test (e.g., MB, L3, L2, etc.) + * @vendor_specific: Bitmask for vendor-specific tests (can be 0 for universal tests) + * @disabled: Test is disabled + * @feature_check: Callback to check required resctrl features + * @run_test: Callback to run the test + */ +struct resctrl_test { + const char *name; + const char *resource; + unsigned int vendor_specific; + bool disabled; + bool (*feature_check)(const struct resctrl_test *test); + int (*run_test)(const struct resctrl_test *test, + const struct user_params *uparams); +}; + /* * resctrl_val_param: resctrl test parameters * @resctrl_val: Resctrl feature (Eg: mbm, mba.. etc) @@ -108,6 +133,7 @@ int mount_resctrlfs(void); int umount_resctrlfs(void); int validate_bw_report_request(char *bw_report); bool validate_resctrl_feature_request(const char *resource, const char *feature); +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); @@ -123,10 +149,8 @@ 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, struct resctrl_val_param *param); -int mbm_bw_change(const struct user_params *uparams); void tests_cleanup(void); void mbm_test_cleanup(void); -int mba_schemata_change(const struct user_params *uparams); void mba_test_cleanup(void); unsigned long create_bit_mask(unsigned int start, unsigned int len); unsigned int count_contiguous_bits(unsigned long val, unsigned int *start); @@ -137,8 +161,6 @@ void ctrlc_handler(int signum, siginfo_t *info, void *ptr); int signal_handler_register(void); void signal_handler_unregister(void); void cat_test_cleanup(void); -int cat_perf_miss_val(const struct user_params *uparams, char *cache_type); -int cmt_resctrl_val(const struct user_params *uparams); unsigned int count_bits(unsigned long n); void cmt_test_cleanup(void);

@@ -166,4 +188,9 @@ static inline int cache_portion_size(unsigned long cache_size, return cache_size * count_bits(portion_mask) / count_bits(full_cache_mask); }

+extern struct resctrl_test mbm_test; +extern struct resctrl_test mba_test; +extern struct resctrl_test cmt_test; +extern struct resctrl_test l3_cat_test; + #endif /* RESCTRL_H */ diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c index 3059a124ac59..75fc49ba3efb 100644 --- a/tools/testing/selftests/resctrl/resctrl_tests.c +++ b/tools/testing/selftests/resctrl/resctrl_tests.c @@ -14,6 +14,13 @@ static volatile int sink_target; volatile int *value_sink = &sink_target;

+static struct resctrl_test *resctrl_tests[] = { + &mbm_test, + &mba_test, + &cmt_test, + &l3_cat_test, +}; + static int detect_vendor(void) { FILE *inf = fopen("/proc/cpuinfo", "r"); @@ -53,11 +60,16 @@ int get_vendor(void)

static void cmd_help(void) { + int i; + printf("usage: resctrl_tests [-h] [-t test list] [-n no_of_bits] [-b benchmark_cmd [option]...]\n"); printf("\t-b benchmark_cmd [option]...: run specified benchmark for MBM, MBA and CMT\n"); printf("\t default benchmark is builtin fill_buf\n"); printf("\t-t test list: run tests specified in the test list, "); printf("e.g. -t mbm,mba,cmt,cat\n"); + printf("\t\tSupported tests:\n"); + for (i = 0; i < ARRAY_SIZE(resctrl_tests); i++) + printf("\t\t\t%s\n", resctrl_tests[i]->name); printf("\t-n no_of_bits: run cache tests using specified no of bits in cache bit mask\n"); printf("\t-p cpu_no: specify CPU number to run the test. 1 is default\n"); printf("\t-h: help\n"); @@ -96,102 +108,41 @@ static void test_cleanup(void) signal_handler_unregister(); }

-static void run_mbm_test(const struct user_params *uparams) +static bool test_vendor_specific_check(const struct resctrl_test *test) { - int res; - - ksft_print_msg("Starting MBM BW change ...\n"); - - if (test_prepare()) { - ksft_exit_fail_msg("Abnormal failure when preparing for the test\n"); - return; - } - - if (!validate_resctrl_feature_request("L3_MON", "mbm_total_bytes") || - !validate_resctrl_feature_request("L3_MON", "mbm_local_bytes") || - (get_vendor() != ARCH_INTEL)) { - ksft_test_result_skip("Hardware does not support MBM or MBM is disabled\n"); - goto cleanup; - } - - res = mbm_bw_change(uparams); - ksft_test_result(!res, "MBM: bw change\n"); - if ((get_vendor() == ARCH_INTEL) && res) - ksft_print_msg("Intel MBM may be inaccurate when Sub-NUMA Clustering is enabled. Check BIOS configuration.\n"); + if (!test->vendor_specific) + return true;

-cleanup: - test_cleanup(); + return get_vendor() & test->vendor_specific; }

-static void run_mba_test(const struct user_params *uparams) +static void run_single_test(const struct resctrl_test *test, const struct user_params *uparams) { - int res; - - ksft_print_msg("Starting MBA Schemata change ...\n"); + int ret;

- if (test_prepare()) { - ksft_exit_fail_msg("Abnormal failure when preparing for the test\n"); + if (test->disabled) return; - }

- if (!validate_resctrl_feature_request("MB", NULL) || - !validate_resctrl_feature_request("L3_MON", "mbm_local_bytes") || - (get_vendor() != ARCH_INTEL)) { - ksft_test_result_skip("Hardware does not support MBA or MBA is disabled\n"); - goto cleanup; - } - - res = mba_schemata_change(uparams); - ksft_test_result(!res, "MBA: schemata change\n"); - -cleanup: - test_cleanup(); -} - -static void run_cmt_test(const struct user_params *uparams) -{ - int res; - - ksft_print_msg("Starting CMT test ...\n"); - - if (test_prepare()) { - ksft_exit_fail_msg("Abnormal failure when preparing for the test\n"); + if (!test_vendor_specific_check(test)) { + ksft_test_result_skip("Hardware does not support %s\n", test->name); return; }

- if (!validate_resctrl_feature_request("L3_MON", "llc_occupancy") || - !validate_resctrl_feature_request("L3", NULL)) { - ksft_test_result_skip("Hardware does not support CMT or CMT is disabled\n"); - goto cleanup; - } - - res = cmt_resctrl_val(uparams); - ksft_test_result(!res, "CMT: test\n"); - if ((get_vendor() == ARCH_INTEL) && res) - ksft_print_msg("Intel CMT may be inaccurate when Sub-NUMA Clustering is enabled. Check BIOS configuration.\n"); - -cleanup: - test_cleanup(); -} - -static void run_cat_test(const struct user_params *uparams) -{ - int res; - - ksft_print_msg("Starting CAT test ...\n"); + ksft_print_msg("Starting %s test ...\n", test->name);

if (test_prepare()) { ksft_exit_fail_msg("Abnormal failure when preparing for the test\n"); return; }

- if (!validate_resctrl_feature_request("L3", NULL)) { - ksft_test_result_skip("Hardware does not support CAT or CAT is disabled\n"); + if (!test->feature_check(test)) { + ksft_test_result_skip("Hardware does not support %s or %s is disabled\n", + test->name, test->name); goto cleanup; }

- res = cat_perf_miss_val(uparams, "L3"); - ksft_test_result(!res, "CAT: test\n"); + ret = test->run_test(test, uparams); + ksft_test_result(!ret, "%s: test\n", test->name);

cleanup: test_cleanup(); @@ -207,11 +158,10 @@ static void init_user_params(struct user_params *uparams)

int main(int argc, char **argv) { - bool mbm_test = true, mba_test = true, cmt_test = true; + int tests = ARRAY_SIZE(resctrl_tests); + bool test_param_seen = false; struct user_params uparams; char *span_str = NULL; - bool cat_test = true; - int tests = 0; int ret, c, i;

init_user_params(&uparams); @@ -239,25 +189,26 @@ int main(int argc, char **argv) case 't': token = strtok(optarg, ",");

- mbm_test = false; - mba_test = false; - cmt_test = false; - cat_test = false; + if (!test_param_seen) { + for (i = 0; i < ARRAY_SIZE(resctrl_tests); i++) + resctrl_tests[i]->disabled = true; + tests = 0; + test_param_seen = true; + } while (token) { - if (!strncmp(token, MBM_STR, sizeof(MBM_STR))) { - mbm_test = true; - tests++; - } else if (!strncmp(token, MBA_STR, sizeof(MBA_STR))) { - mba_test = true; - tests++; - } else if (!strncmp(token, CMT_STR, sizeof(CMT_STR))) { - cmt_test = true; - tests++; - } else if (!strncmp(token, CAT_STR, sizeof(CAT_STR))) { - cat_test = true; - tests++; - } else { - printf("invalid argument\n"); + bool found = false; + + for (i = 0; i < ARRAY_SIZE(resctrl_tests); i++) { + if (!strcasecmp(token, resctrl_tests[i]->name)) { + if (resctrl_tests[i]->disabled) + tests++; + resctrl_tests[i]->disabled = false; + found = true; + } + } + + if (!found) { + printf("invalid test: %s\n", token);

return -1; } @@ -317,19 +268,10 @@ int main(int argc, char **argv) uparams.benchmark_cmd[5] = NULL; }

- ksft_set_plan(tests ? : 4); - - if (mbm_test) - run_mbm_test(&uparams); - - if (mba_test) - run_mba_test(&uparams); - - if (cmt_test) - run_cmt_test(&uparams); + ksft_set_plan(tests);

- if (cat_test) - run_cat_test(&uparams); + for (i = 0; i < ARRAY_SIZE(resctrl_tests); i++) + run_single_test(resctrl_tests[i], &uparams);

free(span_str); ksft_finished(); diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c index 97760fadcddf..37ca06f38673 100644 --- a/tools/testing/selftests/resctrl/resctrlfs.c +++ b/tools/testing/selftests/resctrl/resctrlfs.c @@ -706,6 +706,11 @@ bool validate_resctrl_feature_request(const char *resource, const char *feature) return !!res; }

+bool test_resource_feature_check(const struct resctrl_test *test) +{ + return validate_resctrl_feature_request(test->resource, NULL); +} + int filter_dmesg(void) { char line[1024];

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 ilpo.jarvinen@linux.intel.com Reviewed-by: Reinette Chatre reinette.chatre@intel.com ---

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 9a6b304044cc..8f5f56130d9f 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 ba4a33f3c13f..0f06e6bbaf5a 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 37ca06f38673..b989b5809902 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);