From: "Mike Rapoport (Microsoft)" <[email protected]> khugepaged registers atexit() and signal handlers that ensure that THP settings are restored regardless of how the test exited.
Make these handlers available for all users of thp_settings. The call to thp_save_settings() installs thp_restore_settings as the atexit() callback and makes sure that signals that kill a process would still call exit() and atexit() callback. Update child process in khugepaged tests using thp_settings to use _exit() instead of exit() to avoid altering THP settings in the middle of a test. Remove redundant THP cleanup from folio_split_race_test.c. Tested-by: Sarthak Sharma <[email protected]> Tested-by: Luiz Capitulino <[email protected]> Signed-off-by: Mike Rapoport (Microsoft) <[email protected]> --- tools/testing/selftests/mm/cow.c | 21 ++++------ .../selftests/mm/folio_split_race_test.c | 23 ----------- tools/testing/selftests/mm/khugepaged.c | 41 ++----------------- tools/testing/selftests/mm/thp_settings.c | 27 +++++++++++- tools/testing/selftests/mm/uffd-wp-mremap.c | 4 -- 5 files changed, 38 insertions(+), 78 deletions(-) diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c index d9c69c04b67d..6abdcb30aba8 100644 --- a/tools/testing/selftests/mm/cow.c +++ b/tools/testing/selftests/mm/cow.c @@ -202,7 +202,7 @@ static void do_test_cow_in_parent(char *mem, size_t size, bool do_mprotect, log_test_result(KSFT_FAIL); goto close_comm_pipes; } else if (!ret) { - exit(fn(mem, size, &comm_pipes)); + _exit(fn(mem, size, &comm_pipes)); } while (read(comm_pipes.child_ready[0], &buf, 1) != 1) @@ -333,7 +333,7 @@ static void do_test_vmsplice_in_parent(char *mem, size_t size, ; /* Modify page content in the child. */ memset(mem, 0xff, size); - exit(0); + _exit(0); } if (!before_fork) { @@ -480,7 +480,7 @@ static void do_test_iouring(char *mem, size_t size, bool use_fork) write(comm_pipes.child_ready[1], "0", 1); while (read(comm_pipes.parent_ready[0], &buf, 1) != 1) ; - exit(0); + _exit(0); } while (read(comm_pipes.child_ready[0], &buf, 1) != 1) @@ -645,7 +645,7 @@ static void do_test_ro_pin(char *mem, size_t size, enum ro_pin_test test, write(comm_pipes.child_ready[1], "0", 1); while (read(comm_pipes.parent_ready[0], &buf, 1) != 1) ; - exit(0); + _exit(0); } /* Wait until our child is ready. */ @@ -956,7 +956,7 @@ static void do_run_with_thp(test_fn fn, enum thp_run thp_run, size_t thpsize) log_test_result(KSFT_FAIL); goto munmap; } else if (!ret) { - exit(0); + _exit(0); } wait(&ret); /* Allow for sharing all pages again. */ @@ -1347,13 +1347,13 @@ static void do_test_anon_thp_collapse(char *mem, size_t size, switch (test) { case ANON_THP_COLLAPSE_UNSHARED: case ANON_THP_COLLAPSE_FULLY_SHARED: - exit(child_memcmp_fn(mem, size, &comm_pipes)); + _exit(child_memcmp_fn(mem, size, &comm_pipes)); break; case ANON_THP_COLLAPSE_LOWER_SHARED: - exit(child_memcmp_fn(mem, size / 2, &comm_pipes)); + _exit(child_memcmp_fn(mem, size / 2, &comm_pipes)); break; case ANON_THP_COLLAPSE_UPPER_SHARED: - exit(child_memcmp_fn(mem + size / 2, size / 2, + _exit(child_memcmp_fn(mem + size / 2, size / 2, &comm_pipes)); break; default: @@ -1911,10 +1911,5 @@ int main(int argc, char **argv) run_anon_thp_test_cases(); run_non_anon_test_cases(); - if (pmdsize) { - /* Only if THP is supported. */ - thp_restore_settings(); - } - ksft_finished(); } diff --git a/tools/testing/selftests/mm/folio_split_race_test.c b/tools/testing/selftests/mm/folio_split_race_test.c index ff026f183ac7..390fd97c1bd9 100644 --- a/tools/testing/selftests/mm/folio_split_race_test.c +++ b/tools/testing/selftests/mm/folio_split_race_test.c @@ -226,23 +226,6 @@ static uint64_t run_iteration(void) return reader_failures; } -static void thp_cleanup_handler(int signum) -{ - thp_restore_settings(); - /* - * Restore default handler and re-raise the signal to exit. - * This is to ensure the test process exits with the correct - * status code corresponding to the signal. - */ - signal(signum, SIG_DFL); - raise(signum); -} - -static void thp_settings_cleanup(void) -{ - thp_restore_settings(); -} - int main(void) { struct thp_settings current_settings; @@ -261,12 +244,6 @@ int main(void) ksft_exit_skip("Please run the test as root\n"); thp_save_settings(); - /* make sure thp settings are restored */ - if (atexit(thp_settings_cleanup) != 0) - ksft_exit_fail_msg("atexit failed\n"); - - signal(SIGINT, thp_cleanup_handler); - signal(SIGTERM, thp_cleanup_handler); thp_read_settings(¤t_settings); current_settings.shmem_enabled = SHMEM_ADVISE; diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c index 3fd0693cc45d..712e8f194b3d 100644 --- a/tools/testing/selftests/mm/khugepaged.c +++ b/tools/testing/selftests/mm/khugepaged.c @@ -72,7 +72,6 @@ struct file_info { }; static struct file_info finfo; -static bool skip_settings_restore; static int exit_status; static void success(const char *msg) @@ -93,26 +92,6 @@ static void skip(const char *msg) exit_status = KSFT_SKIP; } -static void restore_settings_atexit(void) -{ - if (skip_settings_restore) - return; - - ksft_print_msg("Restore THP and khugepaged settings..."); - thp_restore_settings(); - success("OK"); - - skip_settings_restore = true; - ksft_print_cnts(); - exit(exit_status); -} - -static void restore_settings(int sig) -{ - /* exit() will invoke the restore_settings_atexit handler. */ - exit(sig ? KSFT_FAIL : exit_status); -} - static void save_settings(void) { ksft_print_msg("Save THP and khugepaged settings..."); @@ -121,12 +100,6 @@ static void save_settings(void) thp_save_settings(); success("OK"); - - atexit(restore_settings_atexit); - signal(SIGTERM, restore_settings); - signal(SIGINT, restore_settings); - signal(SIGHUP, restore_settings); - signal(SIGQUIT, restore_settings); } static void get_finfo(const char *dir) @@ -846,8 +819,6 @@ static void collapse_fork(struct collapse_context *c, struct mem_ops *ops) ksft_print_msg("Share small page over fork()..."); if (!fork()) { /* Do not touch settings on child exit */ - skip_settings_restore = true; - if (ops->check_huge(p, 0)) success("OK"); else @@ -859,7 +830,7 @@ static void collapse_fork(struct collapse_context *c, struct mem_ops *ops) validate_memory(p, 0, page_size); ops->cleanup_area(p, hpage_pmd_size); - exit(exit_status); + _exit(exit_status); } wait(&wstatus); @@ -884,8 +855,6 @@ static void collapse_fork_compound(struct collapse_context *c, struct mem_ops *o ksft_print_msg("Share huge page over fork()..."); if (!fork()) { /* Do not touch settings on child exit */ - skip_settings_restore = true; - if (ops->check_huge(p, 1)) success("OK"); else @@ -908,7 +877,7 @@ static void collapse_fork_compound(struct collapse_context *c, struct mem_ops *o validate_memory(p, 0, hpage_pmd_size); ops->cleanup_area(p, hpage_pmd_size); - exit(exit_status); + _exit(exit_status); } wait(&wstatus); @@ -934,8 +903,6 @@ static void collapse_max_ptes_shared(struct collapse_context *c, struct mem_ops ksft_print_msg("Share huge page over fork()..."); if (!fork()) { /* Do not touch settings on child exit */ - skip_settings_restore = true; - if (ops->check_huge(p, 1)) success("OK"); else @@ -968,7 +935,7 @@ static void collapse_max_ptes_shared(struct collapse_context *c, struct mem_ops validate_memory(p, 0, hpage_pmd_size); ops->cleanup_area(p, hpage_pmd_size); - exit(exit_status); + _exit(exit_status); } wait(&wstatus); @@ -1261,5 +1228,5 @@ int main(int argc, char **argv) t->fn(t->ctx, t->ops); } - restore_settings(0); + ksft_finished(); } diff --git a/tools/testing/selftests/mm/thp_settings.c b/tools/testing/selftests/mm/thp_settings.c index e748ebfb3d4e..9bfa5dc42624 100644 --- a/tools/testing/selftests/mm/thp_settings.c +++ b/tools/testing/selftests/mm/thp_settings.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 #include <fcntl.h> #include <limits.h> +#include <signal.h> #include <stdio.h> #include <stdlib.h> #include <string.h> @@ -15,6 +16,7 @@ static struct thp_settings settings_stack[MAX_SETTINGS_DEPTH]; static int settings_index; static struct thp_settings saved_settings; static char dev_queue_read_ahead_path[PATH_MAX]; +static bool thp_settings_saved; static const char * const thp_enabled_strings[] = { "never", @@ -298,12 +300,35 @@ void thp_pop_settings(void) void thp_restore_settings(void) { - thp_write_settings(&saved_settings); + if (thp_settings_saved) + thp_write_settings(&saved_settings); +} + +static void thp_restore_settings_atexit(void) +{ + thp_restore_settings(); +} + +static void thp_restore_settings_sighandler(int sig) +{ + /* exit() will invoke the thp_restore_settings_atexit handler. */ + exit(KSFT_FAIL); } void thp_save_settings(void) { thp_read_settings(&saved_settings); + thp_settings_saved = true; + + /* + * setup exit hooks to make sure THP settings are restored on graceful + * and error exits and signals + */ + atexit(thp_restore_settings_atexit); + signal(SIGTERM, thp_restore_settings_sighandler); + signal(SIGINT, thp_restore_settings_sighandler); + signal(SIGHUP, thp_restore_settings_sighandler); + signal(SIGQUIT, thp_restore_settings_sighandler); } void thp_set_read_ahead_path(char *path) diff --git a/tools/testing/selftests/mm/uffd-wp-mremap.c b/tools/testing/selftests/mm/uffd-wp-mremap.c index 17186d4a4147..516c35d236e1 100644 --- a/tools/testing/selftests/mm/uffd-wp-mremap.c +++ b/tools/testing/selftests/mm/uffd-wp-mremap.c @@ -368,10 +368,6 @@ int main(int argc, char **argv) tc->swapout, tc->hugetlb); } - /* If THP is supported, restore original THP settings. */ - if (nr_thpsizes) - thp_restore_settings(); - i = ksft_get_fail_cnt(); if (i) ksft_exit_fail_msg("%d out of %d tests failed\n", -- 2.53.0
