On Mon, Oct 9, 2023 at 10:45 PM Michal Koutný <mkoutny@xxxxxxxx> wrote: > > On Sat, Oct 07, 2023 at 02:02:57PM +0000, Yafang Shao <laoar.shao@xxxxxxxxx> wrote: > > The task cannot modify cgroups if we have already acquired the > > css_set_lock, thus eliminating the need to hold the cgroup_mutex. Following > > this change, task_cgroup_from_root() can be employed in non-sleepable contexts. > > IIRC, cset_cgroup_from_root() needs cgroup_mutex to make sure the `root` > doesn't disappear under hands (synchronization with > cgroup_destroy_root(). 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? > However, as I look at it now, cgroup_mutex won't synchronize against > cgroup_kill_sb(), it may worked by accident(?) nowadays (i.e. callers > pinned the root implicitly in another way). > > Still, it'd be good to have an annotation that ensures, the root is around > when using it. (RCU read lock might be fine but you'd need > cgroup_get_live() if passing it out of the RCU read section.) > > Basically, the code must be made safe against cgroup v1 unmounts. What we aim to protect against is changes to the `root_list`, which occur exclusively during cgroup setup and destroy paths. 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. -- Regards Yafang
