Hi Sai,
On 3/10/2020 7:46 PM, Sai Praneeth Prakhya wrote:
On Tue, 2020-03-10 at 15:18 -0700, Reinette Chatre wrote:
On 3/6/2020 7:40 PM, Sai Praneeth Prakhya wrote:
.mum_resctrlfs = 0, .filename = RESULT_FILE_NAME,
.mask = ~(long_mask << n) & long_mask,.span = cache_size * n / count_of_bits,.setup = cqm_setup,
.setup = cqm_setup- int ret;
- char schemata[64];
- unsigned long long_mask;
- if (strcmp(benchmark_cmd[0], "fill_buf") == 0)
sprintf(benchmark_cmd[1], "%lu", param.span);
- ret = remount_resctrlfs(1);
- if (ret)
return ret;Here resctrl is remounted and followed by some changes to the root group's schemata. That is followed by a call to resctrl_val that attempts to remount resctrl again that will undo all the configurations inbetween.
No, it wouldn't because mum_resctrlfs is 0. When resctrl FS is already mounted and mum_resctrlfs is 0, then remount_resctrlfs() is a noop.
I missed that. Thank you.
fyi ... when I tried these tests I encountered the following error related to unmounting:
[SNIP] ok Write schema "L3:1=7fff" to resctrl FS ok Write schema "L3:1=ffff" to resctrl FS ok Write schema "L3:1=1ffff" to resctrl FS ok Write schema "L3:1=3ffff" to resctrl FS # Unable to umount resctrl: Device or resource busy # Results are displayed in (Bytes) ok CQM: diff within 5% for mask 1 # alloc_llc_cache_size: 2883584 # avg_llc_occu_resc: 2973696 ok CQM: diff within 5% for mask 3 [SNIP]
This seems to originate from resctrl_val() that forces an unmount but if that fails the error is not propagated.
diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c index 271cb5c976f5..c59fad6cb9b0 100644 --- a/tools/testing/selftests/resctrl/resctrl_val.c +++ b/tools/testing/selftests/resctrl/resctrl_val.c @@ -705,29 +705,21 @@ int resctrl_val(char **benchmark_cmd, struct resctrl_val_param *param) goto out; }
- /* Give benchmark enough time to fully run */
- sleep(1);
- /* Test runs until the callback setup() tells the test to stop. */ while (1) {
ret = param->setup(param);if (ret) {ret = 0;break;}/* Measure vals sleeps for a second */
ret = param->setup(param);if (ret) {ret = 0;break;}
(I refer to the above snippet in my comment below)
ret = measure_vals(param, &bw_resc_start); if (ret) break; } else if (strcmp(resctrl_val, "cqm") == 0) {
ret = param->setup(param);if (ret) {ret = 0;break;}sleep(1); ret = measure_cache_vals(param, bm_pid); if (ret) break;This change affects not just the cache monitoring test. Could this change be extracted into its own patch to be clear what is done here and how it impacts the other tests?
This change shouldn't impact other tests (i.e. CAT) because CAT will not call resctrl_val().
I was referring to the snippet above that seems to impact the "mbm" and "mba" tests by moving the call to "param->setup" for the them.
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c index 52452bb0178a..bd81a13ff9df 100644 --- a/tools/testing/selftests/resctrl/resctrlfs.c +++ b/tools/testing/selftests/resctrl/resctrlfs.c @@ -365,11 +365,7 @@ void run_benchmark(int signum, siginfo_t *info, void *ucontext) memflush = atoi(benchmark_cmd[3]); operation = atoi(benchmark_cmd[4]); sprintf(resctrl_val, "%s", benchmark_cmd[5]);
if (strcmp(resctrl_val, "cqm") != 0)buffer_span = span * MB;elsebuffer_span = span;
buffer_span = span * MB;This change seems to change the buffer_span used by the other tests. It is not obvious why this change is made to other tests while this commit intends to focus on the cache monitoring test. Perhaps this can be split into a separate patch to make this clear?
Got it. Thank you.
Reinette