* Re: [Devel] [PATCH vz10 v2] cgroup: use proper lockdep condition for ve_nsproxy dereference
2026-08-12 16:42 ` [Devel] [PATCH vz10 v2] cgroup: use proper lockdep condition for ve_nsproxy dereference Konstantin Khorenko
@ 2026-08-12 16:48 ` Pavel Tikhomirov
2026-08-12 17:02 ` [Devel] [PATCH RHEL10 COMMIT] " Konstantin Khorenko
1 sibling, 0 replies; 3+ messages in thread
From: Pavel Tikhomirov @ 2026-08-12 16:48 UTC (permalink / raw)
On 8/12/26 18:42, Konstantin Khorenko wrote:
> 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) ||
--
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.
^ permalink raw reply [flat|nested] 3+ messages in thread* [Devel] [PATCH RHEL10 COMMIT] cgroup: use proper lockdep condition for ve_nsproxy dereference
2026-08-12 16:42 ` [Devel] [PATCH vz10 v2] cgroup: use proper lockdep condition for ve_nsproxy dereference Konstantin Khorenko
2026-08-12 16:48 ` Pavel Tikhomirov
@ 2026-08-12 17:02 ` Konstantin Khorenko
1 sibling, 0 replies; 3+ messages in thread
From: Konstantin Khorenko @ 2026-08-12 17:02 UTC (permalink / raw)
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) ||
^ permalink raw reply [flat|nested] 3+ messages in thread