| Message ID | 20231211121826.14392-24-ilpo.jarvinen@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests/resctrl: CAT test improvements & generalized test framework | expand |
Hi Ilpo, On 12/11/2023 4:18 AM, Ilpo Järvinen wrote: > CAT test does not reset the CPU affinity after the benchmark. > This is relatively harmless as is because CAT test is the last > benchmark to run, however, more tests may be added later. > > Store the CPU affinity the first time taskset_benchmark() is run and > add taskset_restore() which the test can call to reset the CPU mask to > its original value. > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > --- > > v3: > - New patch > --- > tools/testing/selftests/resctrl/cat_test.c | 13 +++++--- > tools/testing/selftests/resctrl/resctrl.h | 3 +- > tools/testing/selftests/resctrl/resctrl_val.c | 2 +- > tools/testing/selftests/resctrl/resctrlfs.c | 33 +++++++++++++++++-- > 4 files changed, 42 insertions(+), 9 deletions(-) > > diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c > index b79916069788..fa95433297c9 100644 > --- a/tools/testing/selftests/resctrl/cat_test.c > +++ b/tools/testing/selftests/resctrl/cat_test.c > @@ -156,6 +156,7 @@ static int cat_test(struct resctrl_val_param *param, size_t span, unsigned long > char *resctrl_val = param->resctrl_val; > struct perf_event_read pe_read; > struct perf_event_attr pea; > + cpu_set_t old_affinity; > unsigned char *buf; > char schemata[64]; > int ret, i, pe_fd; > @@ -167,7 +168,7 @@ static int cat_test(struct resctrl_val_param *param, size_t span, unsigned long > bm_pid = getpid(); > > /* Taskset benchmark to specified cpu */ > - ret = taskset_benchmark(bm_pid, param->cpu_no); > + ret = taskset_benchmark(bm_pid, param->cpu_no, &old_affinity); > if (ret) > return ret; > > @@ -175,13 +176,15 @@ static int cat_test(struct resctrl_val_param *param, size_t span, unsigned long > ret = write_bm_pid_to_resctrl(bm_pid, param->ctrlgrp, param->mongrp, > resctrl_val); > if (ret) > - return ret; > + goto reset_affinity; > > perf_event_attr_initialize(&pea, PERF_COUNT_HW_CACHE_MISSES); > perf_event_initialize_read_format(&pe_read); > pe_fd = perf_open(&pea, bm_pid, param->cpu_no); > - if (pe_fd < 0) > - return pe_fd; > + if (pe_fd < 0) { > + ret = -1; > + goto reset_affinity; > + } > > buf = alloc_buffer(span, 1); > if (!buf) { > @@ -220,6 +223,8 @@ static int cat_test(struct resctrl_val_param *param, size_t span, unsigned long > free(buf); > pe_close: > close(pe_fd); > +reset_affinity: > + taskset_restore(bm_pid, &old_affinity); > > return ret; > } > diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h > index da1f1b508aee..da62f4cd5add 100644 > --- a/tools/testing/selftests/resctrl/resctrl.h > +++ b/tools/testing/selftests/resctrl/resctrl.h > @@ -98,7 +98,8 @@ int umount_resctrlfs(void); > int validate_bw_report_request(char *bw_report); > bool validate_resctrl_feature_request(const char *resource, const char *feature); > char *fgrep(FILE *inf, const char *str); > -int taskset_benchmark(pid_t bm_pid, int cpu_no); > +int taskset_benchmark(pid_t bm_pid, int cpu_no, cpu_set_t *old_affinity); > +int taskset_restore(pid_t bm_pid, cpu_set_t *old_affinity); > int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, > char *resctrl_val); > int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp, > diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c > index d515850cc174..4aed974efa0f 100644 > --- a/tools/testing/selftests/resctrl/resctrl_val.c > +++ b/tools/testing/selftests/resctrl/resctrl_val.c > @@ -777,7 +777,7 @@ int resctrl_val(const char * const *benchmark_cmd, struct resctrl_val_param *par > value.sival_ptr = (void *)benchmark_cmd; > > /* Taskset benchmark to specified cpu */ > - ret = taskset_benchmark(bm_pid, param->cpu_no); > + ret = taskset_benchmark(bm_pid, param->cpu_no, NULL); > if (ret) > goto out; > > diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c > index dffe42e11c6c..97760fadcddf 100644 > --- a/tools/testing/selftests/resctrl/resctrlfs.c > +++ b/tools/testing/selftests/resctrl/resctrlfs.c > @@ -345,15 +345,25 @@ int get_mask_no_shareable(const char *cache_type, unsigned long *mask) > > /* > * taskset_benchmark - Taskset PID (i.e. benchmark) to a specified cpu > - * @bm_pid: PID that should be binded > - * @cpu_no: CPU number at which the PID would be binded > + * @bm_pid: PID that should be binded > + * @cpu_no: CPU number at which the PID would be binded > + * @old_affinity: When not NULL, set to old CPU affinity > * > * Return: 0 on success, < 0 on error. > */ > -int taskset_benchmark(pid_t bm_pid, int cpu_no) > +int taskset_benchmark(pid_t bm_pid, int cpu_no, cpu_set_t *old_affinity) > { > cpu_set_t my_set; > > + if (old_affinity) { > + CPU_ZERO(old_affinity); > + if (sched_getaffinity(bm_pid, sizeof(*old_affinity), > + old_affinity)) { > + ksft_perror("Unable to read previous CPU affinity"); "previous" can be confusing here (it is not trying to determine something from the past but instead the current state). It can just be "Unable to read CPU affinity" > + return -1; > + } > + } > + > CPU_ZERO(&my_set); > CPU_SET(cpu_no, &my_set); > > @@ -366,6 +376,23 @@ int taskset_benchmark(pid_t bm_pid, int cpu_no) > return 0; > } > > +/* > + * taskset_restore - Taskset PID to the earlier CPU affinity > + * @bm_pid: PID that should be reset > + * @old_affinity: The old CPU affinity to restore > + * > + * Return: 0 on success, < 0 on error. > + */ > +int taskset_restore(pid_t bm_pid, cpu_set_t *old_affinity) > +{ > + if (sched_setaffinity(bm_pid, sizeof(*old_affinity), old_affinity)) { > + ksft_perror("Unable to restore taskset"); This message is not clear to me. How about "Unable to restore CPU affinity"? > + return -1; > + } > + > + return 0; > +} > + > /* > * create_grp - Create a group only if one doesn't exist > * @grp_name: Name of the group Reviewed-by: Reinette Chatre <reinette.chatre@intel.com> Reinette
On Wed, 13 Dec 2023, Reinette Chatre wrote: > On 12/11/2023 4:18 AM, Ilpo Järvinen wrote: > > CAT test does not reset the CPU affinity after the benchmark. > > This is relatively harmless as is because CAT test is the last > > benchmark to run, however, more tests may be added later. > > > > Store the CPU affinity the first time taskset_benchmark() is run and > > add taskset_restore() which the test can call to reset the CPU mask to > > its original value. > > > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > > --- > > +/* > > + * taskset_restore - Taskset PID to the earlier CPU affinity > > + * @bm_pid: PID that should be reset > > + * @old_affinity: The old CPU affinity to restore > > + * > > + * Return: 0 on success, < 0 on error. > > + */ > > +int taskset_restore(pid_t bm_pid, cpu_set_t *old_affinity) > > +{ > > + if (sched_setaffinity(bm_pid, sizeof(*old_affinity), old_affinity)) { > > + ksft_perror("Unable to restore taskset"); > > This message is not clear to me. How about "Unable to restore CPU affinity"? Okay, I can change that. I actually was on the edge what to with these because I was just trying to be consistent with the existing error message in taskset_benchmark(). I reasoned that because "taskset" is the userspace tool which the user might be familiar the original idea of using "taskset" might be helpful. And now rereading what I wrote in that message, "restore taskset" does not sound very sensical grammarwise. :-)
diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c index b79916069788..fa95433297c9 100644 --- a/tools/testing/selftests/resctrl/cat_test.c +++ b/tools/testing/selftests/resctrl/cat_test.c @@ -156,6 +156,7 @@ static int cat_test(struct resctrl_val_param *param, size_t span, unsigned long char *resctrl_val = param->resctrl_val; struct perf_event_read pe_read; struct perf_event_attr pea; + cpu_set_t old_affinity; unsigned char *buf; char schemata[64]; int ret, i, pe_fd; @@ -167,7 +168,7 @@ static int cat_test(struct resctrl_val_param *param, size_t span, unsigned long bm_pid = getpid(); /* Taskset benchmark to specified cpu */ - ret = taskset_benchmark(bm_pid, param->cpu_no); + ret = taskset_benchmark(bm_pid, param->cpu_no, &old_affinity); if (ret) return ret; @@ -175,13 +176,15 @@ static int cat_test(struct resctrl_val_param *param, size_t span, unsigned long ret = write_bm_pid_to_resctrl(bm_pid, param->ctrlgrp, param->mongrp, resctrl_val); if (ret) - return ret; + goto reset_affinity; perf_event_attr_initialize(&pea, PERF_COUNT_HW_CACHE_MISSES); perf_event_initialize_read_format(&pe_read); pe_fd = perf_open(&pea, bm_pid, param->cpu_no); - if (pe_fd < 0) - return pe_fd; + if (pe_fd < 0) { + ret = -1; + goto reset_affinity; + } buf = alloc_buffer(span, 1); if (!buf) { @@ -220,6 +223,8 @@ static int cat_test(struct resctrl_val_param *param, size_t span, unsigned long free(buf); pe_close: close(pe_fd); +reset_affinity: + taskset_restore(bm_pid, &old_affinity); return ret; } diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h index da1f1b508aee..da62f4cd5add 100644 --- a/tools/testing/selftests/resctrl/resctrl.h +++ b/tools/testing/selftests/resctrl/resctrl.h @@ -98,7 +98,8 @@ int umount_resctrlfs(void); int validate_bw_report_request(char *bw_report); bool validate_resctrl_feature_request(const char *resource, const char *feature); char *fgrep(FILE *inf, const char *str); -int taskset_benchmark(pid_t bm_pid, int cpu_no); +int taskset_benchmark(pid_t bm_pid, int cpu_no, cpu_set_t *old_affinity); +int taskset_restore(pid_t bm_pid, cpu_set_t *old_affinity); int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val); int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp, diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c index d515850cc174..4aed974efa0f 100644 --- a/tools/testing/selftests/resctrl/resctrl_val.c +++ b/tools/testing/selftests/resctrl/resctrl_val.c @@ -777,7 +777,7 @@ int resctrl_val(const char * const *benchmark_cmd, struct resctrl_val_param *par value.sival_ptr = (void *)benchmark_cmd; /* Taskset benchmark to specified cpu */ - ret = taskset_benchmark(bm_pid, param->cpu_no); + ret = taskset_benchmark(bm_pid, param->cpu_no, NULL); if (ret) goto out; diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c index dffe42e11c6c..97760fadcddf 100644 --- a/tools/testing/selftests/resctrl/resctrlfs.c +++ b/tools/testing/selftests/resctrl/resctrlfs.c @@ -345,15 +345,25 @@ int get_mask_no_shareable(const char *cache_type, unsigned long *mask) /* * taskset_benchmark - Taskset PID (i.e. benchmark) to a specified cpu - * @bm_pid: PID that should be binded - * @cpu_no: CPU number at which the PID would be binded + * @bm_pid: PID that should be binded + * @cpu_no: CPU number at which the PID would be binded + * @old_affinity: When not NULL, set to old CPU affinity * * Return: 0 on success, < 0 on error. */ -int taskset_benchmark(pid_t bm_pid, int cpu_no) +int taskset_benchmark(pid_t bm_pid, int cpu_no, cpu_set_t *old_affinity) { cpu_set_t my_set; + if (old_affinity) { + CPU_ZERO(old_affinity); + if (sched_getaffinity(bm_pid, sizeof(*old_affinity), + old_affinity)) { + ksft_perror("Unable to read previous CPU affinity"); + return -1; + } + } + CPU_ZERO(&my_set); CPU_SET(cpu_no, &my_set); @@ -366,6 +376,23 @@ int taskset_benchmark(pid_t bm_pid, int cpu_no) return 0; } +/* + * taskset_restore - Taskset PID to the earlier CPU affinity + * @bm_pid: PID that should be reset + * @old_affinity: The old CPU affinity to restore + * + * Return: 0 on success, < 0 on error. + */ +int taskset_restore(pid_t bm_pid, cpu_set_t *old_affinity) +{ + if (sched_setaffinity(bm_pid, sizeof(*old_affinity), old_affinity)) { + ksft_perror("Unable to restore taskset"); + return -1; + } + + return 0; +} + /* * create_grp - Create a group only if one doesn't exist * @grp_name: Name of the group
CAT test does not reset the CPU affinity after the benchmark. This is relatively harmless as is because CAT test is the last benchmark to run, however, more tests may be added later. Store the CPU affinity the first time taskset_benchmark() is run and add taskset_restore() which the test can call to reset the CPU mask to its original value. Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> --- v3: - New patch --- tools/testing/selftests/resctrl/cat_test.c | 13 +++++--- tools/testing/selftests/resctrl/resctrl.h | 3 +- tools/testing/selftests/resctrl/resctrl_val.c | 2 +- tools/testing/selftests/resctrl/resctrlfs.c | 33 +++++++++++++++++-- 4 files changed, 42 insertions(+), 9 deletions(-)