Re: [PATCH v4 3/3] selftests/mm: restore enable_soft_offline in hugetlb-soft-offline

[Song Hu]

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 <husong@xxxxxxxxxx>
> 
> ---
> 
> 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);
>