Re: [PATCH bpf-next v1 5/5] bpf: add a selftest for cgroup hierarchical stats collection

From: Michal Koutný
Date: Fri Jun 03 2022 - 12:23:47 EST


On Fri, May 20, 2022 at 01:21:33AM +0000, Yosry Ahmed <yosryahmed@xxxxxxxxxx> wrote:
> +#define CGROUP_PATH(p, n) {.name = #n, .path = #p"/"#n}
> +
> +static struct {
> + const char *name, *path;

Please unify the order of path and name with the macro (slightly
confusing ;-).

> +SEC("tp_btf/mm_vmscan_memcg_reclaim_end")
> +int BPF_PROG(vmscan_end, struct lruvec *lruvec, struct scan_control *sc)
> +{
> [...]
> + struct cgroup *cgrp = task_memcg(current);
> [...]
> + /* cgrp may not have memory controller enabled */
> + if (!cgrp)
> + return 0;

Yes, the controller may not be enabled (for a cgroup).
Just noting that the task_memcg() implementation will fall back to
root_mem_cgroup in such a case (or nearest ancestor), you may want to
use cgroup_ss_mask() for proper detection.

Regards,
Michal