Hi Ilpo,
On 7/13/2023 6:19 AM, Ilpo Järvinen wrote:
> Resctrl FS mount/remount/umount code is hard to track. Better approach
> is to use mount/umount pair for each test but that assumes resctrl FS
> is not mounted beforehand.
>
> Change umount_resctrlfs() so that it can unmount resctrl FS from any
> path, and enable further simplifications into mount/remount/umount
> logic by unmounting resctrl FS at the start if a pre-existing
> mountpoint is found.
>
> Suggested-by: Reinette Chatre <reinette.chatre@xxxxxxxxx>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
> ---
> tools/testing/selftests/resctrl/resctrl_tests.c | 2 ++
> tools/testing/selftests/resctrl/resctrlfs.c | 6 ++++--
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c
> index 9b9751206e1c..b1b2d28b52f7 100644
> --- a/tools/testing/selftests/resctrl/resctrl_tests.c
> +++ b/tools/testing/selftests/resctrl/resctrl_tests.c
> @@ -250,6 +250,8 @@ int main(int argc, char **argv)
> if (!check_resctrlfs_support())
> return ksft_exit_skip("resctrl FS does not exist. Enable X86_CPU_RESCTRL config option.\n");
>
> + umount_resctrlfs();
> +
umount_resctrlfs() can fail. I think there should be error checking
here and no tests should be run if resctrl cannot be unmounted since
the hardware and resctrl state is not known (without more effort).
> filter_dmesg();
>
> ksft_set_plan(tests ? : 4);
> diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
> index fb00245dee92..23f75aeaa198 100644
> --- a/tools/testing/selftests/resctrl/resctrlfs.c
> +++ b/tools/testing/selftests/resctrl/resctrlfs.c
> @@ -82,10 +82,12 @@ int remount_resctrlfs(bool mum_resctrlfs)
>
> int umount_resctrlfs(void)
> {
> - if (find_resctrl_mount(NULL))
> + char mountpoint[256];
> +
> + if (find_resctrl_mount(mountpoint))
> return 0;
It looks like the intent is to return 0 if find_resctrl_mount()
returns -ENOENT. It is not clear to me that it should also
return 0 if find_resctrl_mount() returns -ENXIO.
>
> - if (umount(RESCTRL_PATH)) {
> + if (umount(mountpoint)) {
> perror("# Unable to umount resctrl");
>
> return errno;
Reinette
