OpenVZ / Virtuozzo kernel development (devel@openvz.org)
 help / color / mirror / Atom feed
From: Konstantin Khorenko <khorenko@virtuozzo.com>
Subject: [Devel] [PATCH RHEL10 COMMIT] cgroup: use proper lockdep condition for ve_nsproxy dereference
Date: Wed, 12 Aug 2026 19:02:33 +0200	[thread overview]
Message-ID: <202608121702.67CH2XcT178922@f0.sw.ru> (raw)
In-Reply-To: <20260812164222.173835-1-khorenko@virtuozzo.com>

The commit is pushed to "branch-rh10-6.12.0-211.39.1.16.x.vz10-ovz" and will appear at git at bitbucket.org:openvz/vzkernel.git
after rh10-6.12.0-211.39.1.16.3.vz10
------>
commit a721e402767b5cad877eb794873bb3579d877d6b
Author: Konstantin Khorenko <khorenko@virtuozzo.com>
Date:   Wed Aug 12 18:42:22 2026 +0200

    cgroup: use proper lockdep condition for ve_nsproxy dereference
    
    cgroup_mark_ve_roots(), cgroup_unmark_ve_roots(), 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.
    
    All four are only called from ve_start_container() and ve_exit_ns(), both
    of which run under ve->op_sem write-lock. The pointer is stable there
    because ve_grab_context() publishes it via rcu_assign_pointer() 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(), ve_stop_ns() and
    ve_exit_ns() already annotate the same pointer. Store the result in a
    local variable to keep the lines readable, and extend the existing
    comment to mention op_sem.
    
    No functional change: the condition is only evaluated under
    CONFIG_PROVE_RCU.
    
    https://virtuozzo.atlassian.net/browse/VSTOR-128317
    
    Feature: ve: ve generic structures
    Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
    Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
 kernel/cgroup/cgroup.c | 54 ++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 44 insertions(+), 10 deletions(-)

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 4db77090f1be1..948a2355df833 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -2179,15 +2179,21 @@ static inline bool ve_check_root_cgroups(struct css_set *cset)
 int cgroup_mark_ve_roots(struct ve_struct *ve)
 {
 	struct cgrp_cset_link *link;
+	struct nsproxy *ve_nsproxy;
 	struct css_set *cset;
 	struct cgroup *cgrp;
 
 	/*
-	 * It's safe to use ve->ve_nsproxy->cgroup_ns->root_cset here without extra
-	 * locking as we do it from container init at container start after
-	 * ve_grab_context and only container init can tear those down.
+	 * It's safe to use ve->ve_nsproxy->cgroup_ns->root_cset here without
+	 * extra locking as we do it from container init at container start
+	 * after ve_grab_context and only container init can tear those down.
+	 * Still ve->op_sem is held here which serializes ve->ve_nsproxy usage
+	 * against ve_drop_context(), so let lockdep verify the context without
+	 * external knowledge.
 	 */
-	cset = rcu_dereference_protected(ve->ve_nsproxy, 1)->cgroup_ns->root_cset;
+	ve_nsproxy = rcu_dereference_protected(ve->ve_nsproxy,
+					       lockdep_is_held(&ve->op_sem));
+	cset = ve_nsproxy->cgroup_ns->root_cset;
 	BUG_ON(!cset);
 
 	spin_lock_irq(&css_set_lock);
@@ -2217,15 +2223,21 @@ int cgroup_mark_ve_roots(struct ve_struct *ve)
 void cgroup_unmark_ve_roots(struct ve_struct *ve)
 {
 	struct cgrp_cset_link *link;
+	struct nsproxy *ve_nsproxy;
 	struct css_set *cset;
 	struct cgroup *cgrp;
 
 	/*
-	 * It's safe to use ve->ve_nsproxy->cgroup_ns->root_cset here without extra
-	 * locking as we do it from container init at container start after
-	 * ve_grab_context and only container init can tear those down.
+	 * It's safe to use ve->ve_nsproxy->cgroup_ns->root_cset here without
+	 * extra locking as we do it from container init at container start
+	 * after ve_grab_context and only container init can tear those down.
+	 * Still ve->op_sem is held here which serializes ve->ve_nsproxy usage
+	 * against ve_drop_context(), so let lockdep verify the context without
+	 * external knowledge.
 	 */
-	cset = rcu_dereference_protected(ve->ve_nsproxy, 1)->cgroup_ns->root_cset;
+	ve_nsproxy = rcu_dereference_protected(ve->ve_nsproxy,
+					       lockdep_is_held(&ve->op_sem));
+	cset = ve_nsproxy->cgroup_ns->root_cset;
 	BUG_ON(!cset);
 
 	spin_lock_irq(&css_set_lock);
@@ -2247,12 +2259,23 @@ void cgroup_unmark_ve_roots(struct ve_struct *ve)
 
 int cgroup_join_vz_slice(struct ve_struct *ve)
 {
+	struct nsproxy *ve_nsproxy;
 	struct kernfs_node *kn;
 	struct css_set *cset;
 	struct cgroup *cgrp;
 	int ret;
 
-	cset = rcu_dereference_protected(ve->ve_nsproxy, 1)->cgroup_ns->root_cset;
+	/*
+	 * It's safe to use ve->ve_nsproxy->cgroup_ns->root_cset here without
+	 * extra locking as we do it from container init at container start
+	 * after ve_grab_context and only container init can tear those down.
+	 * Still ve->op_sem is held here which serializes ve->ve_nsproxy usage
+	 * against ve_drop_context(), so let lockdep verify the context without
+	 * external knowledge.
+	 */
+	ve_nsproxy = rcu_dereference_protected(ve->ve_nsproxy,
+					       lockdep_is_held(&ve->op_sem));
+	cset = ve_nsproxy->cgroup_ns->root_cset;
 	cgrp = __cset_cgroup_from_root(cset, &cgrp_dfl_root);
 
 	if (!is_virtualized_cgroup(cgrp) ||
@@ -2286,11 +2309,22 @@ int cgroup_join_vz_slice(struct ve_struct *ve)
 
 int cgroup_leave_vz_slice(struct ve_struct *ve)
 {
+	struct nsproxy *ve_nsproxy;
 	struct css_set *cset;
 	struct cgroup *cgrp;
 	int ret;
 
-	cset = rcu_dereference_protected(ve->ve_nsproxy, 1)->cgroup_ns->root_cset;
+	/*
+	 * It's safe to use ve->ve_nsproxy->cgroup_ns->root_cset here without
+	 * extra locking as we do it from container init at container start
+	 * after ve_grab_context and only container init can tear those down.
+	 * Still ve->op_sem is held here which serializes ve->ve_nsproxy usage
+	 * against ve_drop_context(), so let lockdep verify the context without
+	 * external knowledge.
+	 */
+	ve_nsproxy = rcu_dereference_protected(ve->ve_nsproxy,
+					       lockdep_is_held(&ve->op_sem));
+	cset = ve_nsproxy->cgroup_ns->root_cset;
 	cgrp = __cset_cgroup_from_root(cset, &cgrp_dfl_root);
 
 	if (!is_virtualized_cgroup(cgrp) ||

      parent reply	other threads:[~2026-08-12 17:02 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <32f00c03-fe20-47a1-b629-0f95f4a6e343@virtuozzo.com>
2026-08-12 16:42 ` [Devel] [PATCH vz10 v2] " Konstantin Khorenko
2026-08-12 16:48   ` Pavel Tikhomirov
2026-08-12 17:02   ` Konstantin Khorenko [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=202608121702.67CH2XcT178922@f0.sw.ru \
    --to=khorenko@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox