Re: [PATCH bpf-next v6 8/8] selftests/bpf: add a selftest for cgroup hierarchical stats collection

From: Hao Luo
Date: Mon Aug 01 2022 - 19:00:14 EST


On Mon, Aug 1, 2022 at 3:00 PM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Mon, Aug 1, 2022 at 10:54 AM Hao Luo <haoluo@xxxxxxxxxx> wrote:
> >
[...]
> > ---
> > .../prog_tests/cgroup_hierarchical_stats.c | 358 ++++++++++++++++++
> > .../bpf/progs/cgroup_hierarchical_stats.c | 218 +++++++++++
> > 2 files changed, 576 insertions(+)
> > create mode 100644 tools/testing/selftests/bpf/prog_tests/cgroup_hierarchical_stats.c
> > create mode 100644 tools/testing/selftests/bpf/progs/cgroup_hierarchical_stats.c
> >
>
> [...]
>
> > +extern void cgroup_rstat_updated(struct cgroup *cgrp, int cpu) __ksym;
> > +extern void cgroup_rstat_flush(struct cgroup *cgrp) __ksym;
> > +
> > +static struct cgroup *task_memcg(struct task_struct *task)
> > +{
> > + return task->cgroups->subsys[memory_cgrp_id]->cgroup;
>
> memory_cgrp_id is kernel-defined internal enum which actually can
> change based on kernel configuration (i.e., which cgroup subsystems
> are enabled or not), is that right?
>
> In practice you wouldn't hard-code it, it's better to use
> bpf_core_enum_value() to capture enum's value in CO-RE-relocatable
> way.
>
> So it might be a good idea to demonstrate that here.
>

Makes sense. Will use bpf_core_enum_value. Thanks!

> > +}
> > +
> > +static uint64_t cgroup_id(struct cgroup *cgrp)
> > +{
> > + return cgrp->kn->id;
> > +}
> > +
>
> [...]