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

[Chen Ridong]

On 2026/5/11 17:53, Guopeng Zhang wrote:
> 
> 
> 在 2026/5/11 17:05, Chen Ridong 写道:
>>
>>
>> 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@xxxxxxxxxx>
>>> ---
>>> 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)?
>> Hi Ridong,
> 
> Thanks for taking a look.
> 
> I don't think the inner check is redundant. `favoring` is per-root, while
> `cgroup_enable_per_threadgroup_rwsem` is global.
> 
> For example, root A may have already enabled favordynmods:
> 

This functionality is only available for cgroup v2, right?

Never mind — I see Tj has already replied.

-- 
Best regards,
Ridong