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

[Pu Lehui]

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 <pule...@huawei.com>

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

Signed-off-by: Pu Lehui <pule...@huawei.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.


Hi Lorenzo,

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

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