Re: [PATCH bpf-next v1 6/6] bpf: selftests: selftests for memcg stat kfuncs

[Shakeel Butt <shakeel.butt@linux.dev>]

On Thu, Dec 18, 2025 at 05:57:50PM -0800, Roman Gushchin wrote:
> From: JP Kobryn <inwardvessel@gmail.com>
> 
> Add test coverage for the kfuncs that fetch memcg stats. Using some common
> stats, test scenarios ensuring that the given stat increases by some
> arbitrary amount. The stats selected cover the three categories represented
> by the enums: node_stat_item, memcg_stat_item, vm_event_item.
> 
> Since only a subset of all stats are queried, use a static struct made up
> of fields for each stat. Write to the struct with the fetched values when
> the bpf program is invoked and read the fields in the user mode program for
> verification.
> 
> Signed-off-by: JP Kobryn <inwardvessel@gmail.com>

Need your signoff

[...]
> +
> +#define NR_PIPES 64
> +static void test_kmem(struct bpf_link *link, struct memcg_query *memcg_query)
> +{
> +	int fds[NR_PIPES][2], i;
> +
> +	/*
> +	 * Increase kmem value by creating pipes which will allocate some
> +	 * kernel buffers.
> +	 */
> +	for (i = 0; i < NR_PIPES; i++) {
> +		if (!ASSERT_OK(pipe(fds[i]), "pipe"))
> +			goto cleanup;
> +	}
> +
> +	if (!ASSERT_OK(read_stats(link), "read stats"))
> +		goto cleanup;
> +
> +	ASSERT_GT(memcg_query->memcg_kmem, 0, "kmem value");
> +
> +cleanup:
> +	for (i = 0; i < NR_PIPES; i++) {

Instead of from 0 to NR_PIPES, we need to go from i-1 to (and equal to) 0
otherwise we can potentially close() junk values.

> +		close(fds[i][0]);
> +		close(fds[i][1]);
> +	}
> +}
> +

[...]

> +
> +SEC("iter.s/cgroup")
> +int cgroup_memcg_query(struct bpf_iter__cgroup *ctx)
> +{
> +	struct cgroup *cgrp = ctx->cgroup;
> +	struct cgroup_subsys_state *css;
> +	struct mem_cgroup *memcg;
> +
> +	if (!cgrp)
> +		return 1;
> +
> +	css = &cgrp->self;
> +	if (!css)

Will css ever be NULL here?

> +		return 1;
> +
> +	memcg = bpf_get_mem_cgroup(css);
> +	if (!memcg)
> +		return 1;
> +
> +	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);
> +
> +	bpf_put_mem_cgroup(memcg);
> +
> +	return 0;
> +}
> -- 
> 2.52.0
>