linux-kernel - Re: [PATCH v3 28/30] mm: memcontrol: prepare for reparenting state

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6860146b-be12-4d5f-bec1-bbcec1dffbc6@linux.dev>
Date: Thu, 29 Jan 2026 16:50:39 +0800
From: Qi Zheng <qi.zheng@...ux.dev>
To: Harry Yoo <harry.yoo@...cle.com>
Cc: Shakeel Butt <shakeel.butt@...ux.dev>, hannes@...xchg.org,
 hughd@...gle.com, mhocko@...e.com, roman.gushchin@...ux.dev,
 muchun.song@...ux.dev, david@...nel.org, lorenzo.stoakes@...cle.com,
 ziy@...dia.com, yosry.ahmed@...ux.dev, imran.f.khan@...cle.com,
 kamalesh.babulal@...cle.com, axelrasmussen@...gle.com, yuanchu@...gle.com,
 weixugc@...gle.com, chenridong@...weicloud.com, mkoutny@...e.com,
 akpm@...ux-foundation.org, hamzamahfooz@...ux.microsoft.com,
 apais@...ux.microsoft.com, lance.yang@...ux.dev, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org, cgroups@...r.kernel.org,
 Qi Zheng <zhengqi.arch@...edance.com>
Subject: Re: [PATCH v3 28/30] mm: memcontrol: prepare for reparenting
 state_local



On 1/29/26 10:10 AM, Harry Yoo wrote:
> On Mon, Jan 19, 2026 at 11:34:53AM +0800, Qi Zheng wrote:
>>
>>
>> On 1/18/26 11:20 AM, Shakeel Butt wrote:
>>> On Wed, Jan 14, 2026 at 07:32:55PM +0800, Qi Zheng wrote:
>>>> From: Qi Zheng <zhengqi.arch@...edance.com>
>>>>
>>>> To resolve the dying memcg issue, we need to reparent LRU folios of child
>>>> memcg to its parent memcg. The following counts are all non-hierarchical
>>>> and need to be reparented to prevent the counts of parent memcg overflow.
>>>>
>>>> 1. memcg->vmstats->state_local[i]
>>>> 2. pn->lruvec_stats->state_local[i]
>>>>
>>>> This commit implements the specific function, which will be used during
>>>> the reparenting process.
>>>
>>> Please add more explanation which was discussed in the email chain at
>>> https://lore.kernel.org/all/5dsb6q2r4xsi24kk5gcnckljuvgvvp6nwifwvc4wuho5hsifeg@5ukg2dq6ini5/
>>
>> OK, will do.
>>
>>>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>>>> index 70583394f421f..7aa32b97c9f17 100644
>>>> --- a/mm/memcontrol.c
>>>> +++ b/mm/memcontrol.c
>>>> @@ -225,6 +225,28 @@ static inline struct obj_cgroup *__memcg_reparent_objcgs(struct mem_cgroup *memc
>>>>    	return objcg;
>>>>    }
>>>> +#ifdef CONFIG_MEMCG_V1
>>>> +static void __mem_cgroup_flush_stats(struct mem_cgroup *memcg, bool force);
>>>> +
>>>> +static inline void reparent_state_local(struct mem_cgroup *memcg, struct mem_cgroup *parent)
>>>> +{
>>>> +	if (cgroup_subsys_on_dfl(memory_cgrp_subsys))
>>>> +		return;
>>>> +
>>>> +	synchronize_rcu();
>>>
>>> Hmm synchrinuze_rcu() is a heavy hammer here. Also you would need rcu
>>> read lock in mod_memcg_state() & mod_memcg_lruvec_state() for this
>>> synchronize_rcu().
>>
>> Since these two functions require memcg or lruvec, they are already
>> within the critical section of the RCU lock.
> 
> What happens if someone grabbed a refcount and then release the rcu read
> lock before percpu refkill and then call mod_memcg[_lruvec]_state()?
> 
> In this case, can we end up reparenting in the middle of non-hierarchical
> stat update because they don't have RCU grace period?
> 
> Something like
> 
> T1				T2
> 
> 				- rcu_read_lock()
> 				- get memcg refcnt
> 				- rcu_read_unlock()
> 
> 				- call mod_memcg_state()
> 				- CSS_IS_DYING is not set
> - Set CSS_IS_DYING
> - Trigger percpu refkill
> 				
> - Trigger offline_css()
>    -> reparent non-hierarchical	- update non-hierarchical stats
>       stats
> 				- put memcg refcount

Good catch, I think you are right.

The rcu lock should be added to mod_memcg_state() and
mod_memcg_lruvec_state().

I will update to v4 as soon as possible.

Thanks,
Qi

> 
>>> Hmm instead of synchronize_rcu() here, we can use queue_rcu_work() in
>>> css_killed_ref_fn(). It would be as simple as the following:
>>
>> It does look much simpler, will do.
>>
>> Thanks,
>> Qi
>