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

[Emil Tsalapatis]

On Sat Feb 28, 2026 at 2:11 AM EST, Hui Zhu wrote:
> From: Hui Zhu <zhuhui@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>

I'm a bit confused, what is the exact scenario where we read -1 for
these stats? AFAICT bpf_mem_cgroup_page_state has to return -1 for them,
and it turn this is possible if the idx we're passing is not valid - but
the ones we do always should be. 

> ---
>  .../selftests/bpf/prog_tests/cgroup_iter_memcg.c       | 10 ++++++++++
>  1 file changed, 10 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 88fc3e83d2b7..9eadfbd3fdb9 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:
> @@ -144,6 +152,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_vm_events PGFAULT");
>  	ASSERT_GT(memcg_query->pgfault, 0, "final pgfault val");
>  
>  cleanup: