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

Yafang Shao
2023-10-08T02:37:10Z

On Sat, Oct 7, 2023 at 11:55 PM Tejun Heo <tj@kernel.org> wrote:
>
> Hello,
>
> On Sat, Oct 07, 2023 at 02:02:58PM +0000, Yafang Shao wrote:
> > diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
> > index b307013b9c6c..65bde6eb41ef 100644
> > --- a/include/linux/cgroup.h
> > +++ b/include/linux/cgroup.h
> > @@ -71,6 +71,8 @@ struct css_task_iter {
> > extern struct file_system_type cgroup_fs_type;
> > extern struct cgroup_root cgrp_dfl_root;
> > extern struct css_set init_css_set;
> > +extern struct list_head cgroup_roots;
> > +extern spinlock_t css_set_lock;
>
> css_set_lock was already out here but why do we need to move cgrou_roots to
> this header?

Ah, shouldn't export cgrou_roots. Thanks for pointing it out.

>
> > +/**
> > + * task_cgroup_id_within_hierarchy - Retrieves the associated cgroup ID from
> > + * a task within a specific cgroup1 hierarchy.
> > + * @task: The task to be tested
> > + * @hierarchy_id: The hierarchy ID of a cgroup1
> > + *
> > + * We limit it to cgroup1 only.
> > + */
> > +u64 task_cgroup1_id_within_hierarchy(struct task_struct *tsk, int hierarchy_id)
> > +{
> ...
> > +}
> > +
> > +/**
> > + * task_ancestor_cgroup_id_within_hierarchy - Retrieves the associated ancestor
> > + * cgroup ID from a task within a specific cgroup1 hierarchy.
> > + * @task: The task to be tested
> > + * @hierarchy_id: The hierarchy ID of a cgroup1
> > + * @ancestor_level: level of ancestor to find starting from root
> > + *
> > + * We limit it to cgroup1 only.
> > + */
> > +u64 task_ancestor_cgroup1_id_within_hierarchy(struct task_struct *tsk, int hierarchy_id,
> > + int ancestor_level)
> > +{
> ...
> > +}
>
> I'd much prefer to have `struct group *task_get_cgroup1_within_hierarchy()`
> then the caller can do cgroup_ancestor() itself.

Good suggestion. will do it in the next version.

--
Regards
Yafang