[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <63c958ae-9db2-4da8-935b-a596cc8535d3@linux.dev>
Date: Mon, 29 Dec 2025 17:42:43 +0800
From: Qi Zheng <qi.zheng@...ux.dev>
To: Shakeel Butt <shakeel.butt@...ux.dev>, Yosry Ahmed <yosry.ahmed@...ux.dev>
Cc: 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, harry.yoo@...cle.com,
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 v2 00/28] Eliminate Dying Memory Cgroup
On 12/24/25 8:58 AM, Shakeel Butt wrote:
> On Wed, Dec 24, 2025 at 12:43:00AM +0000, Yosry Ahmed wrote:
> [...]
>>>
>>> I think you meant child's memcg here.
>>
>> Yes, sorry.
>>
>>>
>>>> before
>>>> reparenting, and using it to update the stats after reparenting? A grace
>>>> period only works if the entire scope of using the memcg is within the
>>>> RCU critical section.
>>>
>>> Yeah this is an issue.
>>>
>>>>
>>>> For example, __mem_cgroup_try_charge_swap() currently does this when
>>>> incrementing MEMCG_SWAP. While this specific example isn't problematic
>>>> because the reference won't be dropped until MEMCG_SWAP is decremented
>>>> again, the pattern of grabbing a ref to the memcg then updating a stat
>>>> could generally cause the problem.
>>>>
>>>> Most stats are updated using lruvec_stat_mod_folio(), which updates the
>>>> stats in the same RCU critical section as obtaining the memcg pointer
>>>> from the folio, so it can be fixed with a grace period. However, I think
>>>> it can be easily missed in the future if other code paths update memcg
>>>> stats in a different way. We should try to enforce that stat updates
>>>> cannot only happen from the same RCU critical section where the memcg
>>>> pointer is acquired.
>>>
>>> The core stats update functions are mod_memcg_state() and
>>> mod_memcg_lruvec_state(). If for v1 only, we add additional check for
>>> CSS_DYING and go to parent if CSS_DYING is set then shouldn't we avoid
>>> this issue?
>>
>> But this is still racy, right? The cgroup could become dying right after
>> we check CSS_DYING, no?
>
> We do reparenting in css_offline() callback and cgroup offlining
> happen somewhat like this:
>
> 1. Set CSS_DYING
> 2. Trigger percpu ref kill
> 3. Kernel makes sure css ref killed is seen by all CPUs and then trigger
> css_offline callback.
it seems that we can add the following to
mem_cgroup_css_free():
parent->vmstats->state_local += child->vmstats->state_local;
Right? I will continue to take a closer look.
Thanks,
Qi
>
> So, if in the stats update function we check CSS_DYING flag and the
> actual stats update within rcu, I think we are good.
Powered by blists - more mailing lists