Re: [PATCH] cgroup: Keep favordynmods enabled once per-threadgroup rwsem is active

[Chen Ridong <chenridong@huaweicloud.com>]

On 2026/5/11 16:16, Guopeng Zhang wrote:
> cgroup_enable_per_threadgroup_rwsem is a one-way switch. Once it is
> enabled, cgroup.procs writes use the per-threadgroup rwsem and
> cgroup_threadgroup_change_begin()/end() use the same global state to
> decide whether to take and release the per-threadgroup rwsem.
> 
> The disable path warned that the per-threadgroup rwsem mechanism could not
> be disabled but still called rcu_sync_exit() and cleared
> CGRP_ROOT_FAVOR_DYNMODS. That partially disabled favordynmods while the
> global per-threadgroup rwsem mode remained enabled: cgroup.procs writes
> would continue to use the per-threadgroup rwsem, while
> cgroup_threadgroup_change_begin()/end() could observe the exited rcu_sync
> state. The root would also no longer report favordynmods.
> 
> Make the transition match the documented one-way semantics. Call
> rcu_sync_enter() only for the first favordynmods enable, and make later
> disable attempts a no-op after warning once the per-threadgroup rwsem mode
> has been enabled.
> 
> Fixes: 0568f89d4fb8 ("cgroup: replace global percpu_rwsem with per threadgroup resem when writing to cgroup.procs")
> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
> ---
> Manual AB test:
> 
> Before this patch:
>   enable favordynmods:
>     cgroup2 opts: rw,relatime,favordynmods
>   disable attempt:
>     cgroup2 opts: rw,relatime
>   dmesg:
>     cgroup: cgroup favordynmods: per threadgroup rwsem mechanism can't be disabled
> 
> After this patch:
>   enable favordynmods:
>     cgroup2 opts: rw,relatime,favordynmods
>   disable attempt:
>     cgroup2 opts: rw,relatime,favordynmods
>   dmesg:
>     cgroup: cgroup favordynmods: per threadgroup rwsem mechanism can't be disabled
> 
>  kernel/cgroup/cgroup.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
> index 6152add0c5eb..fd10fb5b3598 100644
> --- a/kernel/cgroup/cgroup.c
> +++ b/kernel/cgroup/cgroup.c
> @@ -1297,14 +1297,13 @@ void cgroup_favor_dynmods(struct cgroup_root *root, bool favor)
>  	 */
>  	percpu_down_write(&cgroup_threadgroup_rwsem);
>  	if (favor && !favoring) {
> -		cgroup_enable_per_threadgroup_rwsem = true;
> -		rcu_sync_enter(&cgroup_threadgroup_rwsem.rss);
> +		if (!cgroup_enable_per_threadgroup_rwsem) {

Is this branch redundant? I think if (favor && !favoring) alone should suffice —
or can the outer condition be true twice (i.e., can this block be entered
multiple times)?


> +			cgroup_enable_per_threadgroup_rwsem = true;
> +			rcu_sync_enter(&cgroup_threadgroup_rwsem.rss);
> +		}
>  		root->flags |= CGRP_ROOT_FAVOR_DYNMODS;
>  	} else if (!favor && favoring) {
> -		if (cgroup_enable_per_threadgroup_rwsem)
> -			pr_warn_once("cgroup favordynmods: per threadgroup rwsem mechanism can't be disabled\n");
> -		rcu_sync_exit(&cgroup_threadgroup_rwsem.rss);
> -		root->flags &= ~CGRP_ROOT_FAVOR_DYNMODS;
> +		pr_warn_once("cgroup favordynmods: per threadgroup rwsem mechanism can't be disabled\n");
>  	}
>  	percpu_up_write(&cgroup_threadgroup_rwsem);
>  }

-- 
Best regards,
Ridong