linux-kernel - Re: [PATCH] selftests/mm: Use generic read

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <caad6357-a4a3-469f-a824-4d7a36a0e629@lucifer.local>
Date: Wed, 11 Jun 2025 10:18:13 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Pu Lehui <pulehui@...weicloud.com>
Cc: akpm@...ux-foundation.org, shuah@...nel.org, linux-mm@...ck.org,
        linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org,
        pulehui@...wei.com
Subject: Re: [PATCH] selftests/mm: Use generic read_sysfs in thuge-gen test

On Wed, Jun 11, 2025 at 08:40:11AM +0000, Pu Lehui wrote:
> From: Pu Lehui <pulehui@...wei.com>
>
> As generic read_sysfs is available in vm_utils, let's
> use is in thuge-gen test.
>
> Signed-off-by: Pu Lehui <pulehui@...wei.com>

It generally looks good, just one point about a warning below to address.

> ---
>  tools/testing/selftests/mm/thuge-gen.c | 37 +++++++-------------------
>  1 file changed, 9 insertions(+), 28 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/thuge-gen.c b/tools/testing/selftests/mm/thuge-gen.c
> index 95b6f043a3cb..e11dfbfa661b 100644
> --- a/tools/testing/selftests/mm/thuge-gen.c
> +++ b/tools/testing/selftests/mm/thuge-gen.c
> @@ -77,40 +77,19 @@ void show(unsigned long ps)
>  	system(buf);
>  }
>
> -unsigned long thuge_read_sysfs(int warn, char *fmt, ...)
> +unsigned long read_free(unsigned long ps)
>  {
> -	char *line = NULL;
> -	size_t linelen = 0;
> -	char buf[100];
> -	FILE *f;
> -	va_list ap;
>  	unsigned long val = 0;
> +	char buf[100];
>
> -	va_start(ap, fmt);
> -	vsnprintf(buf, sizeof buf, fmt, ap);
> -	va_end(ap);
> +	snprintf(buf, sizeof(buf),
> +		 "/sys/kernel/mm/hugepages/hugepages-%lukB/free_hugepages",
> +		 ps >> 10);
> +	read_sysfs(buf, &val);

We're losing all of the 'warn' logic here so if we can't find
/sys/kernel/mm/hugepages/hugepages-%lukB/free_hugepages when ps != getpagesize()
we no longer print a message about it.

Should we reinstate that?

Other than this, we're ignoring errors, which by default means we return 0, but
this is what we were doing anyway. It's only this case I think that matters.

>
> -	f = fopen(buf, "r");
> -	if (!f) {
> -		if (warn)
> -			ksft_print_msg("missing %s\n", buf);
> -		return 0;
> -	}
> -	if (getline(&line, &linelen, f) > 0) {
> -		sscanf(line, "%lu", &val);
> -	}
> -	fclose(f);
> -	free(line);
>  	return val;
>  }
>
> -unsigned long read_free(unsigned long ps)
> -{
> -	return thuge_read_sysfs(ps != getpagesize(),
> -			  "/sys/kernel/mm/hugepages/hugepages-%lukB/free_hugepages",
> -			  ps >> 10);
> -}
> -
>  void test_mmap(unsigned long size, unsigned flags)
>  {
>  	char *map;
> @@ -173,6 +152,7 @@ void test_shmget(unsigned long size, unsigned flags)
>  void find_pagesizes(void)
>  {
>  	unsigned long largest = getpagesize();
> +	unsigned long shmmax_val = 0;
>  	int i;
>  	glob_t g;
>
> @@ -195,7 +175,8 @@ void find_pagesizes(void)
>  	}
>  	globfree(&g);
>
> -	if (thuge_read_sysfs(0, "/proc/sys/kernel/shmmax") < NUM_PAGES * largest)
> +	read_sysfs("/proc/sys/kernel/shmmax", &shmmax_val);
> +	if (shmmax_val < NUM_PAGES * largest)
>  		ksft_exit_fail_msg("Please do echo %lu > /proc/sys/kernel/shmmax",
>  				   largest * NUM_PAGES);
>
> --
> 2.34.1
>