On Mon, Sep 18, 2023 at 10:45 PM Michal Koutný <mkoutny@xxxxxxxx> wrote:
>
> On Sun, Sep 03, 2023 at 02:27:56PM +0000, Yafang Shao <laoar.shao@xxxxxxxxx> wrote:
> > static inline bool task_under_cgroup_hierarchy(struct task_struct *task,
> > struct cgroup *ancestor)
> > {
> > struct css_set *cset = task_css_set(task);
> > + struct cgroup *cgrp;
> > + bool ret = false;
> > + int ssid;
> > +
> > + if (ancestor->root == &cgrp_dfl_root)
> > + return cgroup_is_descendant(cset->dfl_cgrp, ancestor);
> > +
> > + for (ssid = 0; ssid < CGROUP_SUBSYS_COUNT; ssid++) {
>
> This loop were better an iteration over cset->cgrp_links to handle any
> v1 hierarchy (under css_set_lock :-/).
Agree. That is better.
>
> > + if (!ancestor->subsys[ssid])
> > + continue;
> >
> > - return cgroup_is_descendant(cset->dfl_cgrp, ancestor);
> > + cgrp = task_css(task, ssid)->cgroup;
>
> Does this pass on a lockdep-enabled kernel?
Yes, the lockdep is enabled.
>
> See conditions in task_css_set_check(), it seems at least RCU read lock
> would be needed (if not going through cgrp_links mentioned above).
All the call sites of it are already under RCU protection, so we don't
need to explicitly set RCU read lock here.
--
Regards
Yafang
