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
