On Wed, Oct 18, 2023 at 5:35 PM Tejun Heo <tj@xxxxxxxxxx> wrote: > > 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? right. will fix it. > > > @@ -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. will use mutex_is_locked() instead. > > > 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. sure, will do it. -- Regards Yafang
