Hi Ilpo,
On 4/18/2023 4:44 AM, Ilpo Järvinen wrote:
A few places currently lack umounting resctrl FS on error paths.
You mention "A few places" (plural). In the patch I do see that cmt_resctrl_val() is missing an unmount. Where are the other places?
Each and every test does require resctrl FS to be present already for feature check. Thus, it makes sense to just mount it on higher level in resctrl_tests.c.
Move resctrl FS mount/unmount into each test function in resctrl_tests.c. Make feature validation to simply check that resctrl FS is mounted.
...
diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c index af71b2141271..426d11189a99 100644 --- a/tools/testing/selftests/resctrl/cmt_test.c +++ b/tools/testing/selftests/resctrl/cmt_test.c @@ -86,10 +86,6 @@ int cmt_resctrl_val(int cpu_no, int n, char **benchmark_cmd) cache_size = 0;
- ret = remount_resctrlfs(true);
- if (ret)
return ret;
- if (!validate_resctrl_feature_request(CMT_STR)) return -1;
diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c index 9b9751206e1c..5c9ed52b69f2 100644 --- a/tools/testing/selftests/resctrl/resctrl_tests.c +++ b/tools/testing/selftests/resctrl/resctrl_tests.c @@ -77,9 +77,15 @@ static void run_mbm_test(bool has_ben, char **benchmark_cmd, int span, ksft_print_msg("Starting MBM BW change ...\n");
- res = remount_resctrlfs(false);
I think that should be remount_resctrlfs(true). Please note that any of the tests could be run separately from the command line and thus each test need to ensure a clean environment, it cannot assume that (a) user space provided it with a clean and/or unmounted resctrl or (b) that any test was run before it.
- if (res) {
ksft_exit_fail_msg("Failed to mount resctrl FS\n");
return;
- }
- if (!validate_resctrl_feature_request(MBM_STR) || (get_vendor() != ARCH_INTEL)) { ksft_test_result_skip("Hardware does not support MBM or MBM is disabled\n");
return;
}goto umount;
Reinette