Re: [RFC PATCH bpf-next 2/8] cgroup: Add new helpers for cgroup1 hierarchy

Yafang Shao
2023-10-09T13:11:58Z

On Mon, Oct 9, 2023 at 7:32 PM Michal Koutný <mkoutny@suse.com> wrote:
>
> Hello.
>
> On Sat, Oct 07, 2023 at 02:02:58PM +0000, Yafang Shao <laoar.shao@gmail.com> wrote:
> > Two new helpers are added for cgroup1 hierarchy:
> >
> > - task_cgroup1_id_within_hierarchy
> > Retrieves the associated cgroup ID of a task within a specific cgroup1
> > hierarchy. The cgroup1 hierarchy is identified by its hierarchy ID.
> > - task_ancestor_cgroup1_id_within_hierarchy
> > Retrieves the associated ancestor cgroup ID of a task whithin a
> > specific cgroup1 hierarchy. The specific ancestor level is determined by
> > its ancestor level.
> >
> > These helper functions have been added to facilitate the tracing of tasks
> > within a particular container or cgroup in BPF programs. It's important to
> > note that these helpers are designed specifically for cgroup1.
>
> Are this helpers need for any 3rd party task?

Yes, for example, we can check if the *next* task in sched_switch
belongs to a specific cgroup.

Hao also pointed out the use case of a 3rd partry task[1].

[1]. https://lore.kernel.org/bpf/CA+khW7hah317_beZ7QDA1R=sWi5q7TanRC+efMhivPKtWzbA4w@mail.gmail.com/

>
> I *think* operating on `current` would be simpler wrt assumptions needed
> for object presense.
>
>
> > +u64 task_cgroup1_id_within_hierarchy(struct task_struct *tsk, int hierarchy_id)
> > +{
> > + struct cgroup_root *root;
> > + struct cgroup *cgrp;
> > + u64 cgid = 0;
> > +
> > + spin_lock_irq(&css_set_lock);
> > + list_for_each_entry(root, &cgroup_roots, root_list) {
>
> This should be for_each_root() macro for better uniform style.

will use it in the next version.

--
Regards
Yafang