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 --- 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);
- 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);
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
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
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
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
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
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
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
linux-kselftest-mirror@lists.linaro.org
-
Lorenzo Stoakes -
Pu Lehui