* [PATCH] selftests/mm: Use generic read_sysfs in thuge-gen test
@ 2025-06-11 8:40 Pu Lehui
2025-06-11 9:18 ` Lorenzo Stoakes
0 siblings, 1 reply; 5+ messages in thread
From: Pu Lehui @ 2025-06-11 8:40 UTC (permalink / raw)
To: akpm, shuah, lorenzo.stoakes
Cc: linux-mm, linux-kselftest, linux-kernel, pulehui
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);
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] selftests/mm: Use generic read_sysfs in thuge-gen test
2025-06-11 8:40 [PATCH] selftests/mm: Use generic read_sysfs in thuge-gen test Pu Lehui
@ 2025-06-11 9:18 ` Lorenzo Stoakes
2025-06-11 9:40 ` Pu Lehui
0 siblings, 1 reply; 5+ messages in thread
From: Lorenzo Stoakes @ 2025-06-11 9:18 UTC (permalink / raw)
To: Pu Lehui; +Cc: akpm, shuah, linux-mm, linux-kselftest, linux-kernel, pulehui
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] selftests/mm: Use generic read_sysfs in thuge-gen test
2025-06-11 9:18 ` Lorenzo Stoakes
@ 2025-06-11 9:40 ` Pu Lehui
2025-06-11 9:48 ` Lorenzo Stoakes
0 siblings, 1 reply; 5+ messages in thread
From: Pu Lehui @ 2025-06-11 9:40 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: akpm, shuah, linux-mm, linux-kselftest, linux-kernel, pulehui
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
>>
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] selftests/mm: Use generic read_sysfs in thuge-gen test
2025-06-11 9:40 ` Pu Lehui
@ 2025-06-11 9:48 ` Lorenzo Stoakes
2025-06-11 9:55 ` Pu Lehui
0 siblings, 1 reply; 5+ messages in thread
From: Lorenzo Stoakes @ 2025-06-11 9:48 UTC (permalink / raw)
To: Pu Lehui; +Cc: akpm, shuah, linux-mm, linux-kselftest, linux-kernel, pulehui
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
> > >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] selftests/mm: Use generic read_sysfs in thuge-gen test
2025-06-11 9:48 ` Lorenzo Stoakes
@ 2025-06-11 9:55 ` Pu Lehui
0 siblings, 0 replies; 5+ messages in thread
From: Pu Lehui @ 2025-06-11 9:55 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: akpm, shuah, linux-mm, linux-kselftest, linux-kernel, pulehui
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
>>>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-06-11 9:55 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-11 8:40 [PATCH] selftests/mm: Use generic read_sysfs in thuge-gen test Pu Lehui
2025-06-11 9:18 ` Lorenzo Stoakes
2025-06-11 9:40 ` Pu Lehui
2025-06-11 9:48 ` Lorenzo Stoakes
2025-06-11 9:55 ` Pu Lehui
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).