On Tue, Oct 17, 2023 at 12:45:38PM +0000, Yafang Shao wrote: > #define for_each_root(root) \ > - list_for_each_entry((root), &cgroup_roots, root_list) > + list_for_each_entry_rcu((root), &cgroup_roots, root_list, \ > + !lockdep_is_held(&cgroup_mutex)) Shouldn't that be lockdep_is_held() without the leading negation? > @@ -1386,13 +1386,15 @@ static inline struct cgroup *__cset_cgroup_from_root(struct css_set *cset, > } > } > > - BUG_ON(!res_cgroup); > + WARN_ON_ONCE(!res_cgroup && lockdep_is_held(&cgroup_mutex)); This doesn't work. lockdep_is_held() is always true if !PROVE_LOCKING. > return res_cgroup; > } > > /* > * look up cgroup associated with current task's cgroup namespace on the > - * specified hierarchy > + * specified hierarchy. Umount synchronization is ensured via VFS layer, > + * so we don't have to hold cgroup_mutex to prevent the root from being > + * destroyed. > */ Yeah, as Michal said, let's not do it this way. Thanks. -- tejun
