It's excess and can actually be confusing. Only init of root container pidns ever updates ve_nsproxy, so we don't need ANY lock to access it if we are this init of root container pidns.
So actually we can temporarily release op_sem around access to ve_nsproxy and it would still be safe. That is exactly what unconditional "1" here means. Nacked-by: Pavel Tikhomirov <[email protected]> On 6/25/26 20:16, Eva Kurchatova wrote: > From: Konstantin Khorenko <[email protected]> > > cgroup_join_vz_slice() and cgroup_leave_vz_slice() dereference > ve->ve_nsproxy with rcu_dereference_protected(..., 1), which > unconditionally silences lockdep without naming the actual lock that > makes the access safe. > > Both functions are only called from ve_start_container(), which runs > under ve->op_sem write-lock. The pointer is stable because > ve_grab_context() publishes it via rcu_assign_pointer() earlier in the > same function, and ve_drop_context() - the only path that clears it - > cannot run concurrently since it also requires op_sem. > > Replace the unconditional "1" with lockdep_is_held(&ve->op_sem) so > that lockdep can actually verify the locking at runtime. This is > consistent with how ve_grab_context(), ve_drop_context(), and > ve_stop_ns() already annotate the same pointer. > > Fixes: f74e12f9f249a ("ve/cgroup: add vz.slice cgroup to put kernel threads > to") > https://virtuozzo.atlassian.net/browse/VSTOR-128317 > > Feature: ve: ve generic structures > Signed-off-by: Konstantin Khorenko <[email protected]> > --- > kernel/cgroup/cgroup.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c > index 5585545304a3..0bd3aff89dbd 100644 > --- a/kernel/cgroup/cgroup.c > +++ b/kernel/cgroup/cgroup.c > @@ -2252,7 +2252,8 @@ int cgroup_join_vz_slice(struct ve_struct *ve) > struct cgroup *cgrp; > int ret; > > - cset = rcu_dereference_protected(ve->ve_nsproxy, > 1)->cgroup_ns->root_cset; > + cset = rcu_dereference_protected(ve->ve_nsproxy, > + > lockdep_is_held(&ve->op_sem))->cgroup_ns->root_cset; > cgrp = __cset_cgroup_from_root(cset, &cgrp_dfl_root); > > if (!is_virtualized_cgroup(cgrp) || > @@ -2287,7 +2288,8 @@ int cgroup_leave_vz_slice(struct ve_struct *ve) > struct css_set *cset; > struct cgroup *cgrp; > > - cset = rcu_dereference_protected(ve->ve_nsproxy, > 1)->cgroup_ns->root_cset; > + cset = rcu_dereference_protected(ve->ve_nsproxy, > + > lockdep_is_held(&ve->op_sem))->cgroup_ns->root_cset; > cgrp = __cset_cgroup_from_root(cset, &cgrp_dfl_root); > > if (!is_virtualized_cgroup(cgrp) || -- Best regards, Pavel Tikhomirov Senior Software Developer, Virtuozzo. _______________________________________________ Devel mailing list [email protected] https://lists.openvz.org/mailman/listinfo/devel
