[PATCH bpf-next v2 0/3] Fix test_cgroup_iter_memcg issues found during back-porting

[PATCH bpf-next v2 0/3] Fix test_cgroup_iter_memcg issues found during back-porting

[Hui Zhu]

From: Hui Zhu <zhuhui@kylinos.cn>

While back-porting "mm: bpf kfuncs to access memcg data", I
encountered issues with test_cgroup_iter_memcg, specifically
in test_kmem.

These patches are my fixes for the problems I encountered.

Changelog:
v2:
According to the comments of JP Kobryn, added bpf_core_enum_value()
usage in the BPF program to handle cross-kernel enum value differences
at load-time instead of compile-time.
Dropped the mm/memcontrol.c patch.
Modified test_kmem handling: instead of skipping when nokmem is set,
verify that kmem value is zero as expected.
According to the comments of bot, fixed assertion message: changed
"bpf_mem_cgroup_page_state" to "bpf_mem_cgroup_vm_events" for PGFAULT
check.

Hui Zhu (3):
  bpf: Use bpf_core_enum_value for stats in cgroup_iter_memcg
  selftests/bpf: Check bpf_mem_cgroup_page_state return value
  bpf: selftests: Skip kmem test when cgroup.memory=nokmem is set

 .../bpf/prog_tests/cgroup_iter_memcg.c        | 42 +++++++++++++++++++
 .../selftests/bpf/progs/cgroup_iter_memcg.c   | 41 ++++++++++++++----
 2 files changed, 76 insertions(+), 7 deletions(-)

-- 
2.43.0

[PATCH bpf-next v2 1/3] bpf: Use bpf_core_enum_value for stats in cgroup_iter_memcg

[Hui Zhu]

From: Hui Zhu <zhuhui@kylinos.cn>

Replace hardcoded enum values with bpf_core_enum_value() calls in
cgroup_iter_memcg test to improve portability across different
kernel versions.

The change adds runtime enum value resolution for:
- node_stat_item: NR_ANON_MAPPED, NR_SHMEM, NR_FILE_PAGES,
  NR_FILE_MAPPED
- memcg_stat_item: MEMCG_KMEM
- vm_event_item: PGFAULT

This ensures the BPF program can adapt to enum value changes
between kernel versions, returning early if any enum value is
unavailable (returns 0).

Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
---
 .../selftests/bpf/progs/cgroup_iter_memcg.c   | 41 +++++++++++++++----
 1 file changed, 34 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/cgroup_iter_memcg.c b/tools/testing/selftests/bpf/progs/cgroup_iter_memcg.c
index 59fb70a3cc50..b020951dd7e6 100644
--- a/tools/testing/selftests/bpf/progs/cgroup_iter_memcg.c
+++ b/tools/testing/selftests/bpf/progs/cgroup_iter_memcg.c
@@ -15,6 +15,8 @@ int cgroup_memcg_query(struct bpf_iter__cgroup *ctx)
 	struct cgroup *cgrp = ctx->cgroup;
 	struct cgroup_subsys_state *css;
 	struct mem_cgroup *memcg;
+	int ret = 1;
+	int idx;
 
 	if (!cgrp)
 		return 1;
@@ -26,14 +28,39 @@ int cgroup_memcg_query(struct bpf_iter__cgroup *ctx)
 
 	bpf_mem_cgroup_flush_stats(memcg);
 
-	memcg_query.nr_anon_mapped = bpf_mem_cgroup_page_state(memcg, NR_ANON_MAPPED);
-	memcg_query.nr_shmem = bpf_mem_cgroup_page_state(memcg, NR_SHMEM);
-	memcg_query.nr_file_pages = bpf_mem_cgroup_page_state(memcg, NR_FILE_PAGES);
-	memcg_query.nr_file_mapped = bpf_mem_cgroup_page_state(memcg, NR_FILE_MAPPED);
-	memcg_query.memcg_kmem = bpf_mem_cgroup_page_state(memcg, MEMCG_KMEM);
-	memcg_query.pgfault = bpf_mem_cgroup_vm_events(memcg, PGFAULT);
+	idx = bpf_core_enum_value(enum node_stat_item, NR_ANON_MAPPED);
+	if (idx == 0)
+		goto out;
+	memcg_query.nr_anon_mapped = bpf_mem_cgroup_page_state(memcg, idx);
 
+	idx = bpf_core_enum_value(enum node_stat_item, NR_SHMEM);
+	if (idx == 0)
+		goto out;
+	memcg_query.nr_shmem = bpf_mem_cgroup_page_state(memcg, idx);
+
+	idx = bpf_core_enum_value(enum node_stat_item, NR_FILE_PAGES);
+	if (idx == 0)
+		goto out;
+	memcg_query.nr_file_pages = bpf_mem_cgroup_page_state(memcg, idx);
+
+	idx = bpf_core_enum_value(enum node_stat_item, NR_FILE_MAPPED);
+	if (idx == 0)
+		goto out;
+	memcg_query.nr_file_mapped = bpf_mem_cgroup_page_state(memcg, idx);
+
+	idx = bpf_core_enum_value(enum memcg_stat_item, MEMCG_KMEM);
+	if (idx == 0)
+		goto out;
+	memcg_query.memcg_kmem = bpf_mem_cgroup_page_state(memcg, idx);
+
+	idx = bpf_core_enum_value(enum vm_event_item, PGFAULT);
+	if (idx == 0)
+		goto out;
+	memcg_query.pgfault = bpf_mem_cgroup_vm_events(memcg, idx);
+
+	ret = 0;
+out:
 	bpf_put_mem_cgroup(memcg);
 
-	return 0;
+	return ret;
 }
-- 
2.43.0

Re: [PATCH bpf-next v2 1/3] bpf: Use bpf_core_enum_value for stats in cgroup_iter_memcg

[JP Kobryn (Meta)]

On 2/12/26 11:23 PM, Hui Zhu wrote:
> From: Hui Zhu <zhuhui@kylinos.cn>
> 
> Replace hardcoded enum values with bpf_core_enum_value() calls in
> cgroup_iter_memcg test to improve portability across different
> kernel versions.
> 
> The change adds runtime enum value resolution for:
> - node_stat_item: NR_ANON_MAPPED, NR_SHMEM, NR_FILE_PAGES,
>    NR_FILE_MAPPED
> - memcg_stat_item: MEMCG_KMEM
> - vm_event_item: PGFAULT
> 
> This ensures the BPF program can adapt to enum value changes
> between kernel versions, returning early if any enum value is
> unavailable (returns 0).
> 
> Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
> ---
>   .../selftests/bpf/progs/cgroup_iter_memcg.c   | 41 +++++++++++++++----
>   1 file changed, 34 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/progs/cgroup_iter_memcg.c b/tools/testing/selftests/bpf/progs/cgroup_iter_memcg.c
> index 59fb70a3cc50..b020951dd7e6 100644
> --- a/tools/testing/selftests/bpf/progs/cgroup_iter_memcg.c
> +++ b/tools/testing/selftests/bpf/progs/cgroup_iter_memcg.c
> @@ -15,6 +15,8 @@ int cgroup_memcg_query(struct bpf_iter__cgroup *ctx)
>   	struct cgroup *cgrp = ctx->cgroup;
>   	struct cgroup_subsys_state *css;
>   	struct mem_cgroup *memcg;
> +	int ret = 1;
> +	int idx;
>   
>   	if (!cgrp)
>   		return 1;
> @@ -26,14 +28,39 @@ int cgroup_memcg_query(struct bpf_iter__cgroup *ctx)
>   
>   	bpf_mem_cgroup_flush_stats(memcg);
>   
> -	memcg_query.nr_anon_mapped = bpf_mem_cgroup_page_state(memcg, NR_ANON_MAPPED);
> -	memcg_query.nr_shmem = bpf_mem_cgroup_page_state(memcg, NR_SHMEM);
> -	memcg_query.nr_file_pages = bpf_mem_cgroup_page_state(memcg, NR_FILE_PAGES);
> -	memcg_query.nr_file_mapped = bpf_mem_cgroup_page_state(memcg, NR_FILE_MAPPED);
> -	memcg_query.memcg_kmem = bpf_mem_cgroup_page_state(memcg, MEMCG_KMEM);
> -	memcg_query.pgfault = bpf_mem_cgroup_vm_events(memcg, PGFAULT);
> +	idx = bpf_core_enum_value(enum node_stat_item, NR_ANON_MAPPED);
> +	if (idx == 0)
> +		goto out;
> +	memcg_query.nr_anon_mapped = bpf_mem_cgroup_page_state(memcg, idx);
>   
> +	idx = bpf_core_enum_value(enum node_stat_item, NR_SHMEM);
> +	if (idx == 0)
> +		goto out;
> +	memcg_query.nr_shmem = bpf_mem_cgroup_page_state(memcg, idx);
> +
> +	idx = bpf_core_enum_value(enum node_stat_item, NR_FILE_PAGES);
> +	if (idx == 0)
> +		goto out;
> +	memcg_query.nr_file_pages = bpf_mem_cgroup_page_state(memcg, idx);
> +
> +	idx = bpf_core_enum_value(enum node_stat_item, NR_FILE_MAPPED);
> +	if (idx == 0)
> +		goto out;
> +	memcg_query.nr_file_mapped = bpf_mem_cgroup_page_state(memcg, idx);
> +
> +	idx = bpf_core_enum_value(enum memcg_stat_item, MEMCG_KMEM);
> +	if (idx == 0)
> +		goto out;
> +	memcg_query.memcg_kmem = bpf_mem_cgroup_page_state(memcg, idx);
> +
> +	idx = bpf_core_enum_value(enum vm_event_item, PGFAULT);
> +	if (idx == 0)
> +		goto out;

This is getting messy. Most of these values should be on any system
regardless of boot-params. I was expecting you would make the call
inline.

Let's simplify this whole series. This selftest only needs to exercise
bpf_mem_cgroup_page_state() and bpf_mem_cgroup_vm_events(). I think we
can remove the kmem subtest altogether and still have the coverage we
need. You could then call bpf_core_enum_value() inline as an argument to
the kfuncs above and not need to check if they exist (just let them give
you the host-specific co-re value).

[PATCH bpf-next v2 2/3] selftests/bpf: Check bpf_mem_cgroup_page_state return value

[Hui Zhu]

From: Hui Zhu <zhuhui@kylinos.cn>

When back-porting test_progs to different kernel versions, I encountered
an issue where the test_cgroup_iter_memcg test would falsely pass even
when bpf_mem_cgroup_page_state() failed.

This patch adds explicit checks to ensure bpf_mem_cgroup_page_state()
doesn't return -1 before validating the actual statistics values.

Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
---
 .../selftests/bpf/prog_tests/cgroup_iter_memcg.c     | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_iter_memcg.c b/tools/testing/selftests/bpf/prog_tests/cgroup_iter_memcg.c
index a5afd16705f0..897b17b58df3 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_iter_memcg.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_iter_memcg.c
@@ -53,6 +53,8 @@ static void test_anon(struct bpf_link *link, struct memcg_query *memcg_query)
 	if (!ASSERT_OK(read_stats(link), "read stats"))
 		goto cleanup;
 
+	ASSERT_NEQ(memcg_query->nr_anon_mapped, (unsigned long)-1,
+		  "bpf_mem_cgroup_page_state NR_ANON_MAPPED");
 	ASSERT_GT(memcg_query->nr_anon_mapped, 0, "final anon mapped val");
 
 cleanup:
@@ -88,6 +90,10 @@ static void test_file(struct bpf_link *link, struct memcg_query *memcg_query)
 	if (!ASSERT_OK(read_stats(link), "read stats"))
 		goto cleanup_map;
 
+	ASSERT_NEQ(memcg_query->nr_file_pages, (unsigned long)-1,
+		  "bpf_mem_cgroup_page_state NR_FILE_PAGES");
+	ASSERT_NEQ(memcg_query->nr_file_mapped, (unsigned long)-1,
+		  "bpf_mem_cgroup_page_state NR_FILE_MAPPED");
 	ASSERT_GT(memcg_query->nr_file_pages, 0, "final file value");
 	ASSERT_GT(memcg_query->nr_file_mapped, 0, "final file mapped value");
 
@@ -119,6 +125,8 @@ static void test_shmem(struct bpf_link *link, struct memcg_query *memcg_query)
 	if (!ASSERT_OK(read_stats(link), "read stats"))
 		goto cleanup;
 
+	ASSERT_NEQ(memcg_query->nr_shmem, (unsigned long)-1,
+		  "bpf_mem_cgroup_page_state NR_SHMEM");
 	ASSERT_GT(memcg_query->nr_shmem, 0, "final shmem value");
 
 cleanup:
@@ -143,6 +151,8 @@ static void test_kmem(struct bpf_link *link, struct memcg_query *memcg_query)
 	if (!ASSERT_OK(read_stats(link), "read stats"))
 		goto cleanup;
 
+	ASSERT_NEQ(memcg_query->memcg_kmem, (unsigned long)-1,
+		  "bpf_mem_cgroup_vm_events MEMCG_KMEM");
 	ASSERT_GT(memcg_query->memcg_kmem, 0, "kmem value");
 
 cleanup:
@@ -170,6 +180,8 @@ static void test_pgfault(struct bpf_link *link, struct memcg_query *memcg_query)
 	if (!ASSERT_OK(read_stats(link), "read stats"))
 		goto cleanup;
 
+	ASSERT_NEQ(memcg_query->pgfault, (unsigned long)-1,
+		  "bpf_mem_cgroup_page_state PGFAULT");
 	ASSERT_GT(memcg_query->pgfault, 0, "final pgfault val");
 
 cleanup:
-- 
2.43.0

Re: [PATCH bpf-next v2 2/3] selftests/bpf: Check bpf_mem_cgroup_page_state return value

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2258 bytes --]

> diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_iter_memcg.c b/tools/testing/selftests/bpf/prog_tests/cgroup_iter_memcg.c
> --- a/tools/testing/selftests/bpf/prog_tests/cgroup_iter_memcg.c
> +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_iter_memcg.c

[ ... ]

> @@ -143,6 +151,8 @@ static void test_kmem(struct bpf_link *link, struct memcg_query *memcg_query)
>  	if (!ASSERT_OK(read_stats(link), "read stats"))
>  		goto cleanup;
>
> +	ASSERT_NEQ(memcg_query->memcg_kmem, (unsigned long)-1,
> +		  "bpf_mem_cgroup_vm_events MEMCG_KMEM");

Should this assertion string say "bpf_mem_cgroup_page_state MEMCG_KMEM"
instead?  The BPF program in progs/cgroup_iter_memcg.c populates
memcg_kmem using bpf_mem_cgroup_page_state(), not
bpf_mem_cgroup_vm_events():

    idx = bpf_core_enum_value(enum memcg_stat_item, MEMCG_KMEM);
    ...
    memcg_query.memcg_kmem = bpf_mem_cgroup_page_state(memcg, idx);

> @@ -170,6 +180,8 @@ static void test_pgfault(struct bpf_link *link, struct memcg_query *memcg_query)
>  	if (!ASSERT_OK(read_stats(link), "read stats"))
>  		goto cleanup;
>
> +	ASSERT_NEQ(memcg_query->pgfault, (unsigned long)-1,
> +		  "bpf_mem_cgroup_page_state PGFAULT");

Similarly, should this say "bpf_mem_cgroup_vm_events PGFAULT"?  The BPF
program retrieves pgfault using bpf_mem_cgroup_vm_events():

    idx = bpf_core_enum_value(enum vm_event_item, PGFAULT);
    ...
    memcg_query.pgfault = bpf_mem_cgroup_vm_events(memcg, idx);

It looks like the two assertion description strings for test_kmem and
test_pgfault were swapped.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/21978734425

AI-authorship-score: low
AI-authorship-explanation: The commit describes a specific back-porting scenario, and the copy-paste error in swapped assertion strings is characteristic of human authorship.
issues-found: 2
issue-severity-score: low
issue-severity-explanation: Two assertion description strings are swapped between test_kmem and test_pgfault, producing misleading diagnostics on test failure but not affecting test correctness or system stability.

[PATCH bpf-next v2 3/3] bpf: selftests: Skip kmem test when cgroup.memory=nokmem is set

[Hui Zhu]

From: Hui Zhu <zhuhui@kylinos.cn>

When cgroup.memory=nokmem is set in the kernel command line, kmem
accounting is disabled. This causes the test_kmem subtest in
cgroup_iter_memcg to fail because it expects non-zero kmem values.

Fix this by checking /proc/cmdline for the nokmem parameter. If
found, verify that kmem value is zero and return early, skipping
the pipe creation test that would otherwise fail.

Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
---
 .../bpf/prog_tests/cgroup_iter_memcg.c        | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_iter_memcg.c b/tools/testing/selftests/bpf/prog_tests/cgroup_iter_memcg.c
index 897b17b58df3..2b9c148cebf0 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_iter_memcg.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_iter_memcg.c
@@ -134,11 +134,41 @@ static void test_shmem(struct bpf_link *link, struct memcg_query *memcg_query)
 	shm_unlink("/tmp_shmem");
 }
 
+static bool cmdline_has(const char *arg)
+{
+	char cmdline[4096];
+	int fd;
+	ssize_t len;
+	bool ret = false;
+
+	fd = open("/proc/cmdline", O_RDONLY);
+	if (fd < 0)
+		return false;
+
+	len = read(fd, cmdline, sizeof(cmdline) - 1);
+	close(fd);
+	if (len < 0)
+		return false;
+
+	cmdline[len] = '\0';
+	if (strstr(cmdline, arg))
+		ret = true;
+
+	return ret;
+}
+
 #define NR_PIPES 64
 static void test_kmem(struct bpf_link *link, struct memcg_query *memcg_query)
 {
 	int fds[NR_PIPES][2], i;
 
+	if (cmdline_has("cgroup.memory=nokmem")) {
+		if (!ASSERT_OK(read_stats(link), "read stats"))
+			return;
+		ASSERT_EQ(memcg_query->memcg_kmem, 0, "kmem value");
+		return;
+	}
+
 	/*
 	 * Increase kmem value by creating pipes which will allocate some
 	 * kernel buffers.
-- 
2.43.0

Re: [PATCH bpf-next v2 3/3] bpf: selftests: Skip kmem test when cgroup.memory=nokmem is set

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2355 bytes --]

> diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_iter_memcg.c b/tools/testing/selftests/bpf/prog_tests/cgroup_iter_memcg.c
> --- a/tools/testing/selftests/bpf/prog_tests/cgroup_iter_memcg.c
> +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_iter_memcg.c

[ ... ]

> +static bool cmdline_has(const char *arg)
> +{

[ ... ]

> +	cmdline[len] = '\0';
> +	if (strstr(cmdline, arg))
> +		ret = true;
> +
> +	return ret;
> +}

[ ... ]

> +	if (cmdline_has("cgroup.memory=nokmem")) {

The kernel's cgroup.memory= parameter accepts a comma-separated list
of options.  The parser in mm/memcontrol.c cgroup_memory() splits the
value on commas with strsep():

mm/memcontrol.c:cgroup_memory() {
    while ((token = strsep(&s, ",")) != NULL) {
        ...
        if (!strcmp(token, "nokmem"))
            cgroup_memory_nokmem = true;
        ...
    }
}

This means valid command lines include configurations like
"cgroup.memory=nosocket,nokmem" or "cgroup.memory=nobpf,nokmem".

Since cmdline_has() uses strstr() to match the full literal string
"cgroup.memory=nokmem", it only matches when nokmem is the first (or
only) option.  For "cgroup.memory=nosocket,nokmem", strstr() won't
find "cgroup.memory=nokmem" as a substring, so the check returns
false even though nokmem is active.

Would it be more robust to search for just "nokmem" within the
cgroup.memory= value, or to split on commas similar to how the kernel
parses it?

> +		if (!ASSERT_OK(read_stats(link), "read stats"))
> +			return;
> +		ASSERT_EQ(memcg_query->memcg_kmem, 0, "kmem value");
> +		return;
> +	}


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/21978734425

AI-authorship-score: low
AI-authorship-explanation: The patch addresses a specific real-world back-porting issue, shows iterative development across versions in response to human reviewer feedback, and uses standard kernel C patterns with no hallmarks of AI generation.
issues-found: 1
issue-severity-score: low
issue-severity-explanation: The strstr-based cmdline check misses nokmem when it is not the first option in a comma-separated cgroup.memory= value, causing the test to fail on those configurations instead of being skipped.