On Tue, 2026-07-07 at 17:31 -0700, Shakeel Butt wrote: > On Tue, Jul 07, 2026 at 04:50:11PM -0700, Eduard Zingerman wrote: > > On Tue, 2026-07-07 at 16:27 -0700, Ziyang Men wrote: > > > But the patch also carries functional value: alongside that comparison, it > > > checks the correctness of the stats the kfuncs return. > > > > > > Let me first answer the main question -- what these tests add over what we > > > already have -- and then lay out a plan. > > > > > > First, the static test (memcg_stat_reader) vs the existing > > > cgroup_iter_memcg. > > > > > > The existing test calls the kfuncs, but for each value it only checks > > > whether it > > > is greater than zero. For example, in prog_tests/cgroup_iter_memcg.c: > > > > > > memset(map, 1, len); /* dirty some anon */ > > > if (!ASSERT_OK(read_stats(link), "read stats")) > > > goto cleanup; > > > ASSERT_GT(memcg_query->nr_anon_mapped, 0, "final anon mapped val"); > > > > > > It never checks the value is actually correct -- i.e. compares it against > > > the > > > value in cgroupfs -- only that it is non-zero. > > > > > > Besides, it also walks a single cgroup: > > > > > > .cgroup.order = BPF_CGROUP_ITER_SELF_ONLY, > > > > > > and reads only five fields. > > > > Arguably one of the the cgroup_iter_memcg.c tests can be extended to > > allocate some mem and check if the value is reflected in the stats. > > But there is a line between MM tests and BPF tests. > > All BPF kfuncs except iterator logic itself are thin wrappers on > > top of the existing MM functionality. Hence, I don't think that > > BPF selftests are a place to stress-test these things. > > That is actually a good discussion point. Where does such kind of tests (i.e. > testing that bpf based memcg stats read functionality is equivalent to > traditional memcg stats reading). As more subsystems are exposed to bpf, > similar > questions would arise more often. > > In this particular case, IIUC you want only tests for bpf related code > (wrappers > & iterator) to be present in bpf selftests, right?
Yes, I think this makes most sense. > Personally I don't have strong opinion where this test should live. > Functionality wise as it is testing rstat infra, I think cgroup selftests > might > be better home for this.
