Re: [PATCH] selftests/mm: Use generic read_sysfs in thuge-gen test

On 2025/6/11 17:48, Lorenzo Stoakes wrote:

On Wed, Jun 11, 2025 at 05:40:06PM +0800, Pu Lehui wrote:

...

On 2025/6/11 17:18, Lorenzo Stoakes wrote:

...

On Wed, Jun 11, 2025 at 08:40:11AM +0000, Pu Lehui wrote:

...

From: Pu Lehui pulehui@huawei.com

As generic read_sysfs is available in vm_utils, let's use is in thuge-gen test.

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

With the fix below:

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

will send new soon, thanks!

...

...

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.

Hi Lorenzo,

Thanks for review. Right, we should explicit that warning when ps != getpagesize(). How about the following modify?

Looks good to me!

...

diff --git a/tools/testing/selftests/mm/thuge-gen.c b/tools/testing/selftests/mm/thuge-gen.c index e11dfbfa661b..8e2b08dc5762 100644 --- a/tools/testing/selftests/mm/thuge-gen.c +++ b/tools/testing/selftests/mm/thuge-gen.c @@ -85,7 +85,8 @@ unsigned long read_free(unsigned long ps) snprintf(buf, sizeof(buf), "/sys/kernel/mm/hugepages/hugepages-%lukB/free_hugepages", ps >> 10);

•   read_sysfs(buf, &val);
  

•   if (read_sysfs(buf, &val) && ps != getpagesize())
  

•           ksft_print_msg("missing %s\n", buf);
  
     return val;
  

  }

...

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