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@intel.com Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com
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