Re: [PATCH v1 3/4] selftests/mm: Extract read_sysfs and write_sysfs into vm_util

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Thu, May 29, 2025 at 03:56:49PM +0000, Pu Lehui wrote:
> From: Pu Lehui <pulehui@huawei.com>
>
> Extract read_sysfs and write_sysfs into vm_util. Meanwhile, rename
> the function in thuge-gen that has the same name as read_sysfs.

Nice!

>
> Signed-off-by: Pu Lehui <pulehui@huawei.com>

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> ---
>  tools/testing/selftests/mm/ksm_tests.c | 32 ++--------------------
>  tools/testing/selftests/mm/thuge-gen.c |  6 ++--
>  tools/testing/selftests/mm/vm_util.c   | 38 ++++++++++++++++++++++++++
>  tools/testing/selftests/mm/vm_util.h   |  2 ++
>  4 files changed, 45 insertions(+), 33 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/ksm_tests.c b/tools/testing/selftests/mm/ksm_tests.c
> index dcdd5bb20f3d..e80deac1436b 100644
> --- a/tools/testing/selftests/mm/ksm_tests.c
> +++ b/tools/testing/selftests/mm/ksm_tests.c
> @@ -58,40 +58,12 @@ int debug;
>
>  static int ksm_write_sysfs(const char *file_path, unsigned long val)
>  {
> -	FILE *f = fopen(file_path, "w");
> -
> -	if (!f) {
> -		fprintf(stderr, "f %s\n", file_path);
> -		perror("fopen");
> -		return 1;
> -	}
> -	if (fprintf(f, "%lu", val) < 0) {
> -		perror("fprintf");
> -		fclose(f);
> -		return 1;
> -	}
> -	fclose(f);
> -
> -	return 0;
> +	return write_sysfs(file_path, val);
>  }
>
>  static int ksm_read_sysfs(const char *file_path, unsigned long *val)
>  {
> -	FILE *f = fopen(file_path, "r");
> -
> -	if (!f) {
> -		fprintf(stderr, "f %s\n", file_path);
> -		perror("fopen");
> -		return 1;
> -	}
> -	if (fscanf(f, "%lu", val) != 1) {
> -		perror("fscanf");
> -		fclose(f);
> -		return 1;
> -	}
> -	fclose(f);
> -
> -	return 0;
> +	return read_sysfs(file_path, val);
>  }
>
>  static void ksm_print_sysfs(void)
> diff --git a/tools/testing/selftests/mm/thuge-gen.c b/tools/testing/selftests/mm/thuge-gen.c
> index a41bc1234b37..95b6f043a3cb 100644
> --- a/tools/testing/selftests/mm/thuge-gen.c
> +++ b/tools/testing/selftests/mm/thuge-gen.c
> @@ -77,7 +77,7 @@ void show(unsigned long ps)
>  	system(buf);
>  }
>
> -unsigned long read_sysfs(int warn, char *fmt, ...)
> +unsigned long thuge_read_sysfs(int warn, char *fmt, ...)
>  {

I wonder if we could update these to use the newly shared functions?

Not a big deal though, perhaps a bit out of scope here, more of a nice-to-have.

>  	char *line = NULL;
>  	size_t linelen = 0;
> @@ -106,7 +106,7 @@ unsigned long read_sysfs(int warn, char *fmt, ...)
>
>  unsigned long read_free(unsigned long ps)
>  {
> -	return read_sysfs(ps != getpagesize(),
> +	return thuge_read_sysfs(ps != getpagesize(),
>  			  "/sys/kernel/mm/hugepages/hugepages-%lukB/free_hugepages",
>  			  ps >> 10);
>  }
> @@ -195,7 +195,7 @@ void find_pagesizes(void)
>  	}
>  	globfree(&g);
>
> -	if (read_sysfs(0, "/proc/sys/kernel/shmmax") < NUM_PAGES * largest)
> +	if (thuge_read_sysfs(0, "/proc/sys/kernel/shmmax") < NUM_PAGES * largest)
>  		ksft_exit_fail_msg("Please do echo %lu > /proc/sys/kernel/shmmax",
>  				   largest * NUM_PAGES);
>
> diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c
> index 1357e2d6a7b6..d899c272e0ee 100644
> --- a/tools/testing/selftests/mm/vm_util.c
> +++ b/tools/testing/selftests/mm/vm_util.c
> @@ -486,3 +486,41 @@ int close_procmap(struct procmap_fd *procmap)
>  {
>  	return close(procmap->fd);
>  }
> +
> +int write_sysfs(const char *file_path, unsigned long val)
> +{
> +	FILE *f = fopen(file_path, "w");
> +
> +	if (!f) {
> +		fprintf(stderr, "f %s\n", file_path);
> +		perror("fopen");
> +		return 1;
> +	}
> +	if (fprintf(f, "%lu", val) < 0) {
> +		perror("fprintf");
> +		fclose(f);
> +		return 1;
> +	}
> +	fclose(f);
> +
> +	return 0;
> +}
> +
> +int read_sysfs(const char *file_path, unsigned long *val)
> +{
> +	FILE *f = fopen(file_path, "r");
> +
> +	if (!f) {
> +		fprintf(stderr, "f %s\n", file_path);
> +		perror("fopen");
> +		return 1;
> +	}
> +	if (fscanf(f, "%lu", val) != 1) {
> +		perror("fscanf");
> +		fclose(f);
> +		return 1;
> +	}
> +	fclose(f);
> +
> +	return 0;
> +}
> diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h
> index 9211ba640d9c..f84c7c4680ea 100644
> --- a/tools/testing/selftests/mm/vm_util.h
> +++ b/tools/testing/selftests/mm/vm_util.h
> @@ -87,6 +87,8 @@ int open_procmap(pid_t pid, struct procmap_fd *procmap_out);
>  int query_procmap(struct procmap_fd *procmap);
>  bool find_vma_procmap(struct procmap_fd *procmap, void *address);
>  int close_procmap(struct procmap_fd *procmap);
> +int write_sysfs(const char *file_path, unsigned long val);
> +int read_sysfs(const char *file_path, unsigned long *val);
>
>  static inline int open_self_procmap(struct procmap_fd *procmap_out)
>  {
> --
> 2.34.1
>