The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Shakeel Butt <shakeel.butt@linux.dev>
To: Eduard Zingerman <eddyz87@gmail.com>
Cc: Ziyang Men <ziyang.meme@gmail.com>,
	 Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	 Andrii Nakryiko <andrii@kernel.org>,
	Kumar Kartikeya Dwivedi <memxor@gmail.com>,
	bpf@vger.kernel.org,  Martin KaFai Lau <martin.lau@linux.dev>,
	Song Liu <song@kernel.org>,
	 Yonghong Song <yonghong.song@linux.dev>,
	Jiri Olsa <jolsa@kernel.org>,
	 Emil Tsalapatis <emil@etsalapatis.com>,
	Shuah Khan <shuah@kernel.org>,
	 Roman Gushchin <roman.gushchin@linux.dev>,
	kernel-team@meta.com, linux-mm@kvack.org,
	 cgroups@vger.kernel.org, linux-kselftest@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/3] selftests/bpf: compare BPF and memory.stat memcg stat readers
Date: Tue, 7 Jul 2026 17:31:23 -0700	[thread overview]
Message-ID: <ak2Wpk6OnhTyon65@linux.dev> (raw)
In-Reply-To: <bc47c75a00acab57e7fea72612e0f6f089ddecc9.camel@gmail.com>

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?

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.

  reply	other threads:[~2026-07-08  0:31 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-04  4:56 [PATCH 0/3] selftests/bpf: compare BPF and memory.stat memcg stat readers Ziyang Men
2026-07-04  4:56 ` [PATCH 1/3] selftests/bpf: add memcg_stat_reader BPF-vs-memory.stat benchmark Ziyang Men
2026-07-04  4:56 ` [PATCH 2/3] selftests/bpf: add memcg_stat_churn BPF-vs-memory.stat benchmark under churn Ziyang Men
2026-07-04  5:39   ` bot+bpf-ci
2026-07-04  4:56 ` [PATCH 3/3] selftests/bpf: add memcg_stat_churn_percpu BPF-vs-memory.stat benchmark under cross-CPU churn Ziyang Men
2026-07-04  5:58   ` bot+bpf-ci
2026-07-07  0:17 ` [PATCH 0/3] selftests/bpf: compare BPF and memory.stat memcg stat readers Eduard Zingerman
2026-07-07  1:50   ` Shakeel Butt
2026-07-07  9:21     ` Eduard Zingerman
2026-07-07 23:27       ` Ziyang Men
2026-07-07 23:50         ` Eduard Zingerman
2026-07-08  0:31           ` Shakeel Butt [this message]
2026-07-08  0:33             ` Eduard Zingerman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ak2Wpk6OnhTyon65@linux.dev \
    --to=shakeel.butt@linux.dev \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=cgroups@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=emil@etsalapatis.com \
    --cc=jolsa@kernel.org \
    --cc=kernel-team@meta.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=martin.lau@linux.dev \
    --cc=memxor@gmail.com \
    --cc=roman.gushchin@linux.dev \
    --cc=shuah@kernel.org \
    --cc=song@kernel.org \
    --cc=yonghong.song@linux.dev \
    --cc=ziyang.meme@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox