Re: [RFC PATCH bpf-next 1/8] cgroup: Don't have to hold cgroup_mutex in task_cgroup_from_root()

Yafang Shao
2023-10-10T11:59:36Z

On Tue, Oct 10, 2023 at 4:29 PM Michal Koutný <mkoutny@suse.com> wrote:
>
> Hi Yafang.
>
> On Tue, Oct 10, 2023 at 11:58:14AM +0800, Yafang Shao <laoar.shao@gmail.com> wrote:
> > current_cgns_cgroup_from_root() doesn't hold the cgroup_mutext as
> > well. Could this potentially lead to issues, such as triggering the
> > BUG_ON() in __cset_cgroup_from_root(), if the root has already been
> > destroyed?
>
> current_cgns_cgroup_from_root() is a tricky one, see also
> https://lore.kernel.org/r/20230502133847.14570-3-mkoutny@suse.com/
>
> I argued there with RCU read lock but when I look at it now, it may not be
> sufficient for the cgroup returned from current_cgns_cgroup_from_root().
>
> The 2nd half still applies, umount synchronization is ensured via VFS
> layer, so the cgroup_root nor its cgroup won't go away in the
> only caller cgroup_show_path().

Thanks for your explanation.

>
>
> > Would it be beneficial to introduce a dedicated root_list_lock
> > specifically for this purpose? This approach could potentially reduce
> > the need for the broader cgroup_mutex in other scenarios.
>
> It may be a convenience lock but v2 (cgrp_dfl_root could make do just
> fine without it).

Right. cgroup2 doesn't require it.

>
> I'm keeping this dicussuion to illustrate the difficulties of adding the
> BPF support for cgroup v1. That is a benefit I see ;-)

A potentially more effective approach might be to employ RCU
protection for the cgroup_list. That said, use
list_for_each_entry_rcu/list_add_rcu/list_del_rcu instead.

--
Regards

Yafang