* [PATCH] selftests/mm: read memory information without popen
@ 2026-08-03 1:30 Warren Xiong
2026-08-04 9:48 ` Mike Rapoport
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Warren Xiong @ 2026-08-03 1:30 UTC (permalink / raw)
To: akpm, shuah
Cc: david, ljs, liam, vbabka, rppt, surenb, mhocko, linux-mm,
linux-kselftest, linux-kernel, Warren Xiong
read_memory_info() invokes two shell pipelines to obtain MemFree and
Hugepagesize from /proc/meminfo. It does not check whether popen()
returns NULL before passing the result to fgets(), and it does not call
pclose() when fgets() fails.
Open /proc/meminfo directly and obtain both values in a single pass.
This removes the unchecked NULL path, closes the file on all paths, and
avoids dependencies on external commands.
The compaction test continues to pass after this change.
Signed-off-by: Warren Xiong <warren.xiong@ugreen.com>
---
tools/testing/selftests/mm/compaction_test.c | 43 +++++++++++++++++-----------
1 file changed, 27 insertions(+), 16 deletions(-)
diff --git a/tools/testing/selftests/mm/compaction_test.c b/tools/testing/selftests/mm/compaction_test.c
index 5b58258..0df2000 100644
--- a/tools/testing/selftests/mm/compaction_test.c
+++ b/tools/testing/selftests/mm/compaction_test.c
@@ -7,6 +7,7 @@
* allocated.
*/
+#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/mman.h>
@@ -29,30 +30,40 @@ struct map_list {
int read_memory_info(unsigned long *memfree, unsigned long *hugepagesize)
{
- char buffer[256] = {0};
- char *cmd = "cat /proc/meminfo | grep -i memfree | grep -o '[0-9]*'";
- FILE *cmdfile = popen(cmd, "r");
+ char buffer[256];
+ bool memfree_found = false;
+ bool hugepagesize_found = false;
+ FILE *file;
+ int ret = -1;
- if (!(fgets(buffer, sizeof(buffer), cmdfile))) {
- ksft_print_msg("Failed to read meminfo: %s\n", strerror(errno));
+ file = fopen("/proc/meminfo", "r");
+ if (!file) {
+ ksft_print_msg("Failed to open /proc/meminfo: %s\n",
+ strerror(errno));
return -1;
}
- pclose(cmdfile);
+ while (fgets(buffer, sizeof(buffer), file)) {
+ if (sscanf(buffer, "MemFree: %lu kB", memfree) == 1)
+ memfree_found = true;
+ else if (sscanf(buffer, "Hugepagesize: %lu kB",
+ hugepagesize) == 1)
+ hugepagesize_found = true;
- *memfree = atoll(buffer);
- cmd = "cat /proc/meminfo | grep -i hugepagesize | grep -o '[0-9]*'";
- cmdfile = popen(cmd, "r");
-
- if (!(fgets(buffer, sizeof(buffer), cmdfile))) {
- ksft_print_msg("Failed to read meminfo: %s\n", strerror(errno));
- return -1;
+ if (memfree_found && hugepagesize_found) {
+ ret = 0;
+ break;
+ }
}
- pclose(cmdfile);
- *hugepagesize = atoll(buffer);
+ if (ferror(file))
+ ksft_print_msg("Failed to read /proc/meminfo: %s\n",
+ strerror(errno));
+ else if (ret)
+ ksft_print_msg("Failed to parse /proc/meminfo\n");
- return 0;
+ fclose(file);
+ return ret;
}
int prereq(void)
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] selftests/mm: read memory information without popen
2026-08-03 1:30 [PATCH] selftests/mm: read memory information without popen Warren Xiong
@ 2026-08-04 9:48 ` Mike Rapoport
2026-08-04 11:15 ` David Hildenbrand (Arm)
2026-08-04 12:16 ` [PATCH v2] " Warren Xiong
2 siblings, 0 replies; 6+ messages in thread
From: Mike Rapoport @ 2026-08-04 9:48 UTC (permalink / raw)
To: Warren Xiong
Cc: akpm, shuah, david, ljs, liam, vbabka, surenb, mhocko, linux-mm,
linux-kselftest, linux-kernel
On Mon, Aug 03, 2026 at 09:30:15AM +0800, Warren Xiong wrote:
> read_memory_info() invokes two shell pipelines to obtain MemFree and
> Hugepagesize from /proc/meminfo. It does not check whether popen()
> returns NULL before passing the result to fgets(), and it does not call
> pclose() when fgets() fails.
>
> Open /proc/meminfo directly and obtain both values in a single pass.
> This removes the unchecked NULL path, closes the file on all paths, and
> avoids dependencies on external commands.
>
> The compaction test continues to pass after this change.
>
> Signed-off-by: Warren Xiong <warren.xiong@ugreen.com>
Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> ---
> tools/testing/selftests/mm/compaction_test.c | 43 +++++++++++++++++-----------
> 1 file changed, 27 insertions(+), 16 deletions(-)
--
Sincerely yours,
Mike.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] selftests/mm: read memory information without popen
2026-08-03 1:30 [PATCH] selftests/mm: read memory information without popen Warren Xiong
2026-08-04 9:48 ` Mike Rapoport
@ 2026-08-04 11:15 ` David Hildenbrand (Arm)
2026-08-04 12:07 ` warren.xiong
2026-08-04 12:16 ` [PATCH v2] " Warren Xiong
2 siblings, 1 reply; 6+ messages in thread
From: David Hildenbrand (Arm) @ 2026-08-04 11:15 UTC (permalink / raw)
To: Warren Xiong, akpm, shuah
Cc: ljs, liam, vbabka, rppt, surenb, mhocko, linux-mm,
linux-kselftest, linux-kernel
On 8/3/26 03:30, Warren Xiong wrote:
> read_memory_info() invokes two shell pipelines to obtain MemFree and
> Hugepagesize from /proc/meminfo. It does not check whether popen()
> returns NULL before passing the result to fgets(), and it does not call
> pclose() when fgets() fails.
>
> Open /proc/meminfo directly and obtain both values in a single pass.
> This removes the unchecked NULL path, closes the file on all paths, and
> avoids dependencies on external commands.
>
> The compaction test continues to pass after this change.
>
> Signed-off-by: Warren Xiong <warren.xiong@ugreen.com>
> ---
> tools/testing/selftests/mm/compaction_test.c | 43 +++++++++++++++++-----------
> 1 file changed, 27 insertions(+), 16 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/compaction_test.c b/tools/testing/selftests/mm/compaction_test.c
> index 5b58258..0df2000 100644
> --- a/tools/testing/selftests/mm/compaction_test.c
> +++ b/tools/testing/selftests/mm/compaction_test.c
> @@ -7,6 +7,7 @@
> * allocated.
> */
>
> +#include <stdbool.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <sys/mman.h>
> @@ -29,30 +30,40 @@ struct map_list {
>
> int read_memory_info(unsigned long *memfree, unsigned long *hugepagesize)
> {
Looks much better indeed.
> - char buffer[256] = {0};
> - char *cmd = "cat /proc/meminfo | grep -i memfree | grep -o '[0-9]*'";
> - FILE *cmdfile = popen(cmd, "r");
> + char buffer[256];
> + bool memfree_found = false;
> + bool hugepagesize_found = false;
> + FILE *file;
> + int ret = -1;
>
> - if (!(fgets(buffer, sizeof(buffer), cmdfile))) {
> - ksft_print_msg("Failed to read meminfo: %s\n", strerror(errno));
> + file = fopen("/proc/meminfo", "r");
> + if (!file) {
> + ksft_print_msg("Failed to open /proc/meminfo: %s\n",
> + strerror(errno));
> return -1;
> }
>
> - pclose(cmdfile);
> + while (fgets(buffer, sizeof(buffer), file)) {
> + if (sscanf(buffer, "MemFree: %lu kB", memfree) == 1)
> + memfree_found = true;
> + else if (sscanf(buffer, "Hugepagesize: %lu kB",
> + hugepagesize) == 1)
> + hugepagesize_found = true;
Instead of these bools you could just have a hit-count.
Like the following on top:
diff --git a/tools/testing/selftests/mm/compaction_test.c b/tools/testing/selftests/mm/compaction_test.c
index 0df2000f4a2b1..30d4ace7155ae 100644
--- a/tools/testing/selftests/mm/compaction_test.c
+++ b/tools/testing/selftests/mm/compaction_test.c
@@ -7,7 +7,6 @@
* allocated.
*/
-#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/mman.h>
@@ -31,8 +30,7 @@ struct map_list {
int read_memory_info(unsigned long *memfree, unsigned long *hugepagesize)
{
char buffer[256];
- bool memfree_found = false;
- bool hugepagesize_found = false;
+ int found = 0;
FILE *file;
int ret = -1;
@@ -43,24 +41,19 @@ int read_memory_info(unsigned long *memfree, unsigned long *hugepagesize)
return -1;
}
- while (fgets(buffer, sizeof(buffer), file)) {
- if (sscanf(buffer, "MemFree: %lu kB", memfree) == 1)
- memfree_found = true;
- else if (sscanf(buffer, "Hugepagesize: %lu kB",
- hugepagesize) == 1)
- hugepagesize_found = true;
-
- if (memfree_found && hugepagesize_found) {
- ret = 0;
- break;
- }
+ while (fgets(buffer, sizeof(buffer), file) && found != 2) {
+ if (sscanf(buffer, "MemFree: %lu kB", memfree) == 1 ||
+ sscanf(buffer, "Hugepagesize: %lu kB", hugepagesize) == 1)
+ found++;
}
if (ferror(file))
ksft_print_msg("Failed to read /proc/meminfo: %s\n",
strerror(errno));
- else if (ret)
+ else if (found != 2)
ksft_print_msg("Failed to parse /proc/meminfo\n");
+ else
+ ret = 0;
fclose(file);
return ret;
--
Cheers,
David
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] selftests/mm: read memory information without popen
2026-08-04 11:15 ` David Hildenbrand (Arm)
@ 2026-08-04 12:07 ` warren.xiong
0 siblings, 0 replies; 6+ messages in thread
From: warren.xiong @ 2026-08-04 12:07 UTC (permalink / raw)
To: david, akpm, shuah
Cc: ljs, liam, vbabka, rppt, surenb, mhocko, linux-mm,
linux-kselftest, linux-kernel
--------------
warren.xiong
>On 8/3/26 03:30, Warren Xiong wrote:
>> read_memory_info() invokes two shell pipelines to obtain MemFree and
>> Hugepagesize from /proc/meminfo. It does not check whether popen()
>> returns NULL before passing the result to fgets(), and it does not call
>> pclose() when fgets() fails.
>>
>> Open /proc/meminfo directly and obtain both values in a single pass.
>> This removes the unchecked NULL path, closes the file on all paths, and
>> avoids dependencies on external commands.
>>
>> The compaction test continues to pass after this change.
>>
>> Signed-off-by: Warren Xiong <warren.xiong@ugreen.com>
>> ---
>> tools/testing/selftests/mm/compaction_test.c | 43 +++++++++++++++++-----------
>> 1 file changed, 27 insertions(+), 16 deletions(-)
>>
>> diff --git a/tools/testing/selftests/mm/compaction_test.c b/tools/testing/selftests/mm/compaction_test.c
>> index 5b58258..0df2000 100644
>> --- a/tools/testing/selftests/mm/compaction_test.c
>> +++ b/tools/testing/selftests/mm/compaction_test.c
>> @@ -7,6 +7,7 @@
>> * allocated.
>> */
>>
>> +#include <stdbool.h>
>> #include <stdio.h>
>> #include <stdlib.h>
>> #include <sys/mman.h>
>> @@ -29,30 +30,40 @@ struct map_list {
>>
>> int read_memory_info(unsigned long *memfree, unsigned long *hugepagesize)
>> {
>
>Looks much better indeed.
>
>> - char buffer[256] = {0};
>> - char *cmd = "cat /proc/meminfo | grep -i memfree | grep -o '[0-9]*'";
>> - FILE *cmdfile = popen(cmd, "r");
>> + char buffer[256];
>> + bool memfree_found = false;
>> + bool hugepagesize_found = false;
>> + FILE *file;
>> + int ret = -1;
>>
>> - if (!(fgets(buffer, sizeof(buffer), cmdfile))) {
>> - ksft_print_msg("Failed to read meminfo: %s\n", strerror(errno));
>> + file = fopen("/proc/meminfo", "r");
>> + if (!file) {
>> + ksft_print_msg("Failed to open /proc/meminfo: %s\n",
>> + strerror(errno));
>> return -1;
>> }
>>
>> - pclose(cmdfile);
>> + while (fgets(buffer, sizeof(buffer), file)) {
>> + if (sscanf(buffer, "MemFree: %lu kB", memfree) == 1)
>> + memfree_found = true;
>> + else if (sscanf(buffer, "Hugepagesize: %lu kB",
>> + hugepagesize) == 1)
>> + hugepagesize_found = true;
>Instead of these bools you could just have a hit-count.
Thanks for the review.
Agreed, a hit count is sufficient here and makes the code simpler. I
have incorporated this change and will send v2 shortly.
Thanks,
Warren
>
>Like the following on top:
>
>diff --git a/tools/testing/selftests/mm/compaction_test.c b/tools/testing/selftests/mm/compaction_test.c
>index 0df2000f4a2b1..30d4ace7155ae 100644
>--- a/tools/testing/selftests/mm/compaction_test.c
>+++ b/tools/testing/selftests/mm/compaction_test.c
>@@ -7,7 +7,6 @@
> * allocated.
> */
>
>-#include <stdbool.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <sys/mman.h>
>@@ -31,8 +30,7 @@ struct map_list {
> int read_memory_info(unsigned long *memfree, unsigned long *hugepagesize)
> {
> char buffer[256];
>- bool memfree_found = false;
>- bool hugepagesize_found = false;
>+ int found = 0;
> FILE *file;
> int ret = -1;
>
>@@ -43,24 +41,19 @@ int read_memory_info(unsigned long *memfree, unsigned long *hugepagesize)
> return -1;
> }
>
>- while (fgets(buffer, sizeof(buffer), file)) {
>- if (sscanf(buffer, "MemFree: %lu kB", memfree) == 1)
>- memfree_found = true;
>- else if (sscanf(buffer, "Hugepagesize: %lu kB",
>- hugepagesize) == 1)
>- hugepagesize_found = true;
>-
>- if (memfree_found && hugepagesize_found) {
>- ret = 0;
>- break;
>- }
>+ while (fgets(buffer, sizeof(buffer), file) && found != 2) {
>+ if (sscanf(buffer, "MemFree: %lu kB", memfree) == 1 ||
>+ sscanf(buffer, "Hugepagesize: %lu kB", hugepagesize) == 1)
>+ found++;
> }
>
> if (ferror(file))
> ksft_print_msg("Failed to read /proc/meminfo: %s\n",
> strerror(errno));
>- else if (ret)
>+ else if (found != 2)
> ksft_print_msg("Failed to parse /proc/meminfo\n");
>+ else
>+ ret = 0;
>
> fclose(file);
> return ret;
>
>
>--
>Cheers,
>
>David
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] selftests/mm: read memory information without popen
2026-08-03 1:30 [PATCH] selftests/mm: read memory information without popen Warren Xiong
2026-08-04 9:48 ` Mike Rapoport
2026-08-04 11:15 ` David Hildenbrand (Arm)
@ 2026-08-04 12:16 ` Warren Xiong
2026-08-04 12:21 ` David Hildenbrand (Arm)
2 siblings, 1 reply; 6+ messages in thread
From: Warren Xiong @ 2026-08-04 12:16 UTC (permalink / raw)
To: akpm, shuah
Cc: david, ljs, liam, vbabka, rppt, surenb, mhocko, linux-mm,
linux-kselftest, linux-kernel, Warren Xiong
read_memory_info() invokes two shell pipelines to obtain MemFree and
Hugepagesize from /proc/meminfo. It does not check whether popen()
returns NULL before passing the result to fgets(), and it does not call
pclose() when fgets() fails.
Open /proc/meminfo directly and obtain both values in a single pass.
This removes the unchecked NULL path, closes the file on all paths, and
avoids dependencies on external commands.
The compaction test continues to pass after this change.
Signed-off-by: Warren Xiong <warren.xiong@ugreen.com>
---
Changes in v2:
- Replace the two boolean flags with a hit counter, as suggested by
David Hildenbrand.
v1: https://lore.kernel.org/1785720615-5826-1-git-send-email-warren.xiong@ugreen.com/
tools/testing/selftests/mm/compaction_test.c | 38 +++++++++++++++-------------
1 file changed, 21 insertions(+), 17 deletions(-)
diff --git a/tools/testing/selftests/mm/compaction_test.c b/tools/testing/selftests/mm/compaction_test.c
index 5b58258..30d4ace 100644
--- a/tools/testing/selftests/mm/compaction_test.c
+++ b/tools/testing/selftests/mm/compaction_test.c
@@ -29,30 +29,34 @@ struct map_list {
int read_memory_info(unsigned long *memfree, unsigned long *hugepagesize)
{
- char buffer[256] = {0};
- char *cmd = "cat /proc/meminfo | grep -i memfree | grep -o '[0-9]*'";
- FILE *cmdfile = popen(cmd, "r");
+ char buffer[256];
+ int found = 0;
+ FILE *file;
+ int ret = -1;
- if (!(fgets(buffer, sizeof(buffer), cmdfile))) {
- ksft_print_msg("Failed to read meminfo: %s\n", strerror(errno));
+ file = fopen("/proc/meminfo", "r");
+ if (!file) {
+ ksft_print_msg("Failed to open /proc/meminfo: %s\n",
+ strerror(errno));
return -1;
}
- pclose(cmdfile);
-
- *memfree = atoll(buffer);
- cmd = "cat /proc/meminfo | grep -i hugepagesize | grep -o '[0-9]*'";
- cmdfile = popen(cmd, "r");
-
- if (!(fgets(buffer, sizeof(buffer), cmdfile))) {
- ksft_print_msg("Failed to read meminfo: %s\n", strerror(errno));
- return -1;
+ while (fgets(buffer, sizeof(buffer), file) && found != 2) {
+ if (sscanf(buffer, "MemFree: %lu kB", memfree) == 1 ||
+ sscanf(buffer, "Hugepagesize: %lu kB", hugepagesize) == 1)
+ found++;
}
- pclose(cmdfile);
- *hugepagesize = atoll(buffer);
+ if (ferror(file))
+ ksft_print_msg("Failed to read /proc/meminfo: %s\n",
+ strerror(errno));
+ else if (found != 2)
+ ksft_print_msg("Failed to parse /proc/meminfo\n");
+ else
+ ret = 0;
- return 0;
+ fclose(file);
+ return ret;
}
int prereq(void)
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] selftests/mm: read memory information without popen
2026-08-04 12:16 ` [PATCH v2] " Warren Xiong
@ 2026-08-04 12:21 ` David Hildenbrand (Arm)
0 siblings, 0 replies; 6+ messages in thread
From: David Hildenbrand (Arm) @ 2026-08-04 12:21 UTC (permalink / raw)
To: Warren Xiong, akpm, shuah
Cc: ljs, liam, vbabka, rppt, surenb, mhocko, linux-mm,
linux-kselftest, linux-kernel
On 8/4/26 14:16, Warren Xiong wrote:
> read_memory_info() invokes two shell pipelines to obtain MemFree and
> Hugepagesize from /proc/meminfo. It does not check whether popen()
> returns NULL before passing the result to fgets(), and it does not call
> pclose() when fgets() fails.
>
> Open /proc/meminfo directly and obtain both values in a single pass.
> This removes the unchecked NULL path, closes the file on all paths, and
> avoids dependencies on external commands.
>
> The compaction test continues to pass after this change.
>
> Signed-off-by: Warren Xiong <warren.xiong@ugreen.com>
> ---
> Changes in v2:
> - Replace the two boolean flags with a hit counter, as suggested by
> David Hildenbrand.
>
> v1: https://lore.kernel.org/1785720615-5826-1-git-send-email-warren.xiong@ugreen.com/
>
> tools/testing/selftests/mm/compaction_test.c | 38 +++++++++++++++-------------
> 1 file changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/compaction_test.c b/tools/testing/selftests/mm/compaction_test.c
> index 5b58258..30d4ace 100644
> --- a/tools/testing/selftests/mm/compaction_test.c
> +++ b/tools/testing/selftests/mm/compaction_test.c
> @@ -29,30 +29,34 @@ struct map_list {
>
> int read_memory_info(unsigned long *memfree, unsigned long *hugepagesize)
> {
> - char buffer[256] = {0};
> - char *cmd = "cat /proc/meminfo | grep -i memfree | grep -o '[0-9]*'";
> - FILE *cmdfile = popen(cmd, "r");
> + char buffer[256];
> + int found = 0;
> + FILE *file;
> + int ret = -1;
>
> - if (!(fgets(buffer, sizeof(buffer), cmdfile))) {
> - ksft_print_msg("Failed to read meminfo: %s\n", strerror(errno));
> + file = fopen("/proc/meminfo", "r");
> + if (!file) {
> + ksft_print_msg("Failed to open /proc/meminfo: %s\n",
> + strerror(errno));
> return -1;
> }
>
> - pclose(cmdfile);
> -
> - *memfree = atoll(buffer);
> - cmd = "cat /proc/meminfo | grep -i hugepagesize | grep -o '[0-9]*'";
> - cmdfile = popen(cmd, "r");
> -
> - if (!(fgets(buffer, sizeof(buffer), cmdfile))) {
> - ksft_print_msg("Failed to read meminfo: %s\n", strerror(errno));
> - return -1;
> + while (fgets(buffer, sizeof(buffer), file) && found != 2) {
> + if (sscanf(buffer, "MemFree: %lu kB", memfree) == 1 ||
> + sscanf(buffer, "Hugepagesize: %lu kB", hugepagesize) == 1)
> + found++;
Just as information: when proposing that I was assuming that we would never ever
have two times the same entry in /proc/meminfo, which i think we can reasonably
assume.
Thanks!
Acked-by: David Hildenbrand (Arm) <david@kernel.org>
--
Cheers,
David
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-08-04 12:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-03 1:30 [PATCH] selftests/mm: read memory information without popen Warren Xiong
2026-08-04 9:48 ` Mike Rapoport
2026-08-04 11:15 ` David Hildenbrand (Arm)
2026-08-04 12:07 ` warren.xiong
2026-08-04 12:16 ` [PATCH v2] " Warren Xiong
2026-08-04 12:21 ` David Hildenbrand (Arm)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox