Hi Usama, 在 2026/8/25 16:57, Song Hu 写道: > hugetlb-soft-offline toggles /proc/sys/vm/enable_soft_offline between 1 > and 0 (test_soft_offline_common(1) then (0)) and leaves it at 0 when it > finishes, silently disabling soft offlining for the whole system after > the run. > > Save the original value before the test and restore it from an > atexit() handler, as hugepage_restore_settings_atexit() in > hugepage_settings.c already does. Use read_num()/write_num() from > vm_util instead of hand-rolled popen()/fopen() helpers. > > The restore handler must not call write_num(): on failure it > re-enters exit() through ksft_exit_fail_msg(), which is undefined > behavior from inside an atexit handler. A non-root run hits it > directly - the restore write fails the same way the write that > triggered the exit did. Restore with plain open()/write(), best > effort. >
Just a note on patch 3/3: v4 changed the restore handler, so your Reviewed-by/Tested-by no longer apply and were dropped. write_num() calls ksft_exit_fail_msg() on failure, which re-enters exit() from inside the atexit handler - undefined behavior that a non-root run hits directly. v4 restores with plain open()/write() instead. Verified on 7.2.0-rc7-next-20260810: 2/2 pass and the sysctl is restored afterwards. Would you mind taking another look when you have time? Thanks, Song > Signed-off-by: Song Hu <[email protected]> > > --- > > Changes since v3: the restore handler no longer uses write_num(), > whose failure path calls exit() from inside an atexit handler - > undefined behavior on a non-root run. Restore with plain > open()/write(), best effort. > > .../selftests/mm/hugetlb-soft-offline.c | 49 +++++++++++-------- > 1 file changed, 28 insertions(+), 21 deletions(-) > > diff --git a/tools/testing/selftests/mm/hugetlb-soft-offline.c > b/tools/testing/selftests/mm/hugetlb-soft-offline.c > index bc202e4ed2bd..4af9d3db7b5b 100644 > --- a/tools/testing/selftests/mm/hugetlb-soft-offline.c > +++ b/tools/testing/selftests/mm/hugetlb-soft-offline.c > @@ -11,6 +11,7 @@ > > #define _GNU_SOURCE > #include <errno.h> > +#include <fcntl.h> > #include <stdlib.h> > #include <stdio.h> > #include <string.h> > @@ -23,6 +24,7 @@ > #include <sys/types.h> > > #include "kselftest.h" > +#include "vm_util.h" > #include "hugepage_settings.h" > > #ifndef MADV_SOFT_OFFLINE > @@ -31,6 +33,8 @@ > > #define EPREFIX " !!! " > > +#define ENABLE_SOFT_OFFLINE_PATH "/proc/sys/vm/enable_soft_offline" > + > static int do_soft_offline(int fd, size_t len, int expect_errno) > { > char *filemap = NULL; > @@ -77,26 +81,29 @@ static int do_soft_offline(int fd, size_t len, int > expect_errno) > return ret; > } > > -static int set_enable_soft_offline(int value) > -{ > - char cmd[256] = {0}; > - FILE *cmdfile = NULL; > - > - if (value != 0 && value != 1) > - return -EINVAL; > +static unsigned long orig_enable_soft_offline = -1UL; > > - sprintf(cmd, "echo %d > /proc/sys/vm/enable_soft_offline", value); > - cmdfile = popen(cmd, "r"); > +/* > + * Runs from an atexit handler, so it must not call anything that > + * exits on failure: write_num() would re-enter exit() through > + * ksft_exit_fail_msg(). > + */ > +static void restore_enable_soft_offline(void) > +{ > + char buf[24]; > + int fd, len; > > - if (cmdfile) > - ksft_print_msg("enable_soft_offline => %d\n", value); > - else { > - ksft_perror(EPREFIX "failed to set enable_soft_offline"); > - return errno; > - } > + if (orig_enable_soft_offline == -1UL) > + return; > > - pclose(cmdfile); > - return 0; > + len = snprintf(buf, sizeof(buf), "%lu", orig_enable_soft_offline); > + fd = open(ENABLE_SOFT_OFFLINE_PATH, O_WRONLY); > + if (fd < 0) > + return; > + if (write(fd, buf, len) != len) > + ksft_print_msg("failed to restore enable_soft_offline: %s\n", > + strerror(errno)); > + close(fd); > } > > static int create_hugetlbfs_file(struct statfs *file_stat) > @@ -145,10 +152,7 @@ static void test_soft_offline_common(int > enable_soft_offline) > hugepagesize_kb = file_stat.f_bsize / 1024; > ksft_print_msg("Hugepagesize is %ldkB\n", hugepagesize_kb); > > - if (set_enable_soft_offline(enable_soft_offline) != 0) { > - close(fd); > - ksft_exit_fail_msg("Failed to set enable_soft_offline\n"); > - } > + write_num(ENABLE_SOFT_OFFLINE_PATH, enable_soft_offline); > > nr_hugepages_before = hugetlb_nr_default_pages(); > > @@ -192,6 +196,9 @@ int main(int argc, char **argv) > > ksft_set_plan(2); > > + orig_enable_soft_offline = read_num(ENABLE_SOFT_OFFLINE_PATH); > + atexit(restore_enable_soft_offline); > + > test_soft_offline_common(1); > test_soft_offline_common(0); >
