On Tue, Oct 10, 2023 at 4:29 PM Michal Koutný <mkoutny@xxxxxxxx> wrote: > > Hi Yafang. > > On Tue, Oct 10, 2023 at 11:58:14AM +0800, Yafang Shao <laoar.shao@xxxxxxxxx> 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@xxxxxxxx/ > > 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
