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

Pu Lehui posted 4 patches 8 months, 2 weeks ago
[PATCH v1 3/4] selftests/mm: Extract read_sysfs and write_sysfs into vm_util
Posted by Pu Lehui 8 months, 2 weeks ago
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.

Signed-off-by: Pu Lehui <pulehui@huawei.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, ...)
 {
 	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
Re: [PATCH v1 3/4] selftests/mm: Extract read_sysfs and write_sysfs into vm_util
Posted by Lorenzo Stoakes 8 months, 2 weeks ago
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
>
Re: [PATCH v1 3/4] selftests/mm: Extract read_sysfs and write_sysfs into vm_util
Posted by Pu Lehui 8 months, 1 week ago

On 2025/5/30 19:48, Lorenzo Stoakes wrote:
> 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.

Hi Lorenzo, Andrew,

Yep, we can do it more. But, actually, I am not sure about the merge 
process of mm module. Do I need to resend the whole series or just the 
diff below?


 From 13ecf9d66a0068d520be955e3ca8d9c0dc9d8393 Mon Sep 17 00:00:00 2001
From: Pu Lehui <pulehui@huawei.com>
Date: Tue, 3 Jun 2025 06:50:43 +0000
Subject: [PATCH] selftests/mm: Use generic read_sysfs in thuge-gen test

Use generic read_sysfs in thuge-gen test.

Signed-off-by: Pu Lehui <pulehui@huawei.com>
---
  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..5a32ed0ba26c 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);

-	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

> 
>>   	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
>>
Re: [PATCH v1 3/4] selftests/mm: Extract read_sysfs and write_sysfs into vm_util
Posted by Andrew Morton 8 months, 1 week ago
On Tue, 3 Jun 2025 15:17:49 +0800 Pu Lehui <pulehui@huaweicloud.com> wrote:

> 
> 
> > Not a big deal though, perhaps a bit out of scope here, more of a nice-to-have.
> 
> ...
>
> Yep, we can do it more. But, actually, I am not sure about the merge 
> process of mm module. Do I need to resend the whole series or just the 
> diff below?
> 

A little diff like this is great, although this one didn't apply for me.

But if we're to get this series into 6.16-rc1, now is not the time to
be changing it.  Please send out a formal patch after -rc1?
Re: [PATCH v1 3/4] selftests/mm: Extract read_sysfs and write_sysfs into vm_util
Posted by Pu Lehui 8 months, 1 week ago

On 2025/6/4 10:36, Andrew Morton wrote:
> On Tue, 3 Jun 2025 15:17:49 +0800 Pu Lehui <pulehui@huaweicloud.com> wrote:
> 
>>
>>
>>> Not a big deal though, perhaps a bit out of scope here, more of a nice-to-have.
>>
>> ...
>>
>> Yep, we can do it more. But, actually, I am not sure about the merge
>> process of mm module. Do I need to resend the whole series or just the
>> diff below?
>>
> 
> A little diff like this is great, although this one didn't apply for me.
> 
> But if we're to get this series into 6.16-rc1, now is not the time to
> be changing it.  Please send out a formal patch after -rc1?

ok, I will handle this clean after -rc1.