| Message ID | 20230713131932.133258-6-ilpo.jarvinen@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests/resctrl: Fixes and cleanups | expand |
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
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(); + 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; - if (umount(RESCTRL_PATH)) { + if (umount(mountpoint)) { perror("# Unable to umount resctrl"); return errno;
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(-)