The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Guopeng Zhang <guopeng.zhang@linux.dev>
To: Michal Hocko <mhocko@suse.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>,
	Roman Gushchin <roman.gushchin@linux.dev>,
	Shakeel Butt <shakeel.butt@linux.dev>,
	Andrew Morton <akpm@linux-foundation.org>,
	Muchun Song <muchun.song@linux.dev>,
	cgroups@vger.kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org,
	Guopeng Zhang <zhangguopeng@kylinos.cn>
Subject: Re: [PATCH] mm: memcg: stop reclaim when a limit update is superseded
Date: Wed, 29 Jul 2026 14:15:25 +0800	[thread overview]
Message-ID: <f706d139-53a7-436d-adc5-c691577ce53b@linux.dev> (raw)
In-Reply-To: <amdkHvSMj95DUri5@tiehlicka>



在 2026/7/27 21:58, Michal Hocko 写道:
> On Mon 27-07-26 20:59:23, Guopeng Zhang wrote:
>>
>>
>> 在 2026/7/27 16:16, Michal Hocko 写道:
>>> On Fri 24-07-26 10:18:05, Guopeng Zhang wrote:
>>>> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
>>>>
>>>> kernfs serializes file operations only per open file, so separate open
>>>> files can update the same memory.high or memory.max file concurrently.
>>>> Both handlers store the new limit before synchronous reclaim, but
>>>> continue to use the writer's local target in the reclaim loop. If another
>>>> writer raises or removes the limit, the first writer can continue
>>>> reclaiming toward a stale target.
>>>>
>>>> For memory.max, this can leave the writer looping indefinitely once
>>>> reclaim retries are exhausted. The OOM path sees sufficient margin under
>>>> the current limit and returns true without killing, while the writer
>>>> still compares usage against its stale target and records another OOM
>>>> event.
>>>
>>> The current behavior is deliberate as described in b6e6edcfa4056.
>>> What is an actual problem you are trying to fix?
>>
>> Thanks for raising this. I agree that the behavior introduced by
>> b6e6edcfa405 is deliberate while the limit installed by the writer
>> remains current. The problem occurs when that limit is overwritten by a
>> later write through another open file.
>>
>> Writer A stores a low memory.max and enters synchronous reclaim. Writer
>> B then restores memory.max to "max". A still compares usage against its
>> local old target, while mem_cgroup_out_of_memory() checks the current
>> memory.max. Since 1378b37d03e8, the current-margin check in
>> mem_cgroup_out_of_memory() sees sufficient margin and returns true
>> without selecting a victim. Once the reclaim retries are exhausted, A
>> therefore loops indefinitely and increments the oom counter in
>> memory.events on every iteration.
>>
>> I reproduced this with a cgroup holding 128 MiB of anonymous memory and
>> with swapping disabled for the cgroup. Writer A lowered memory.max to
>> 32 MiB. After that value became visible, writer B restored memory.max
>> to "max" through another open file.
> 
> Is this trying to replicate any real workload? One would expect that
> writers to limit do some sort of coordination otherwise the exact
> behavior is not really well defined.
> 

No, this was not motivated by a reported production workload. We found
it through automated randomized testing for our cgroup observability
work and reduced it to the reproducer above.

>> On the unpatched kernel, A remained blocked after B's write, and the oom
>> counter in memory.events increased from 37333 to 13512861 during the
>> reproducer's one-second sampling interval. With the patch, the same
>> reproducer observed A return after B superseded A's limit.
>>
>> The new check does not change the behavior introduced by b6e6edcfa405
>> while the writer's target remains the active limit.  For the indefinite
>> loop described above, 1378b37d03e8 appears to be the more precise
>> Fixes: target for the memory.max hunk. Does that match your reading of
>> the history?
> 
> Well, to be really honest I am not really convinced this needs fixing.
> And if yes, your patch changes a well established behavior existing
> userspace might already depend on. While your described case doesn't
> look great it doesn't seem really harmful and the looping task is
> killable.

That makes sense. Without a concrete workload showing practical impact,
there is not enough justification to change the established behavior.
We can revisit this if such a workload turns up.

Andrew, please drop this patch from your queue.

Thanks,
Guopeng

  reply	other threads:[~2026-07-29  6:16 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-24  2:18 [PATCH] mm: memcg: stop reclaim when a limit update is superseded Guopeng Zhang
2026-07-24  3:32 ` Tao Cui
2026-07-24  6:53   ` Guopeng Zhang
2026-07-26  3:42 ` Andrew Morton
2026-07-26  5:57   ` Guopeng Zhang
2026-07-27  8:16 ` Michal Hocko
2026-07-27 12:59   ` Guopeng Zhang
2026-07-27 13:58     ` Michal Hocko
2026-07-29  6:15       ` Guopeng Zhang [this message]
2026-07-29  8:02         ` Michal Hocko
2026-07-29 19:11           ` Andrew Morton
2026-07-29 19:36             ` Johannes Weiner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f706d139-53a7-436d-adc5-c691577ce53b@linux.dev \
    --to=guopeng.zhang@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=cgroups@vger.kernel.org \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=muchun.song@linux.dev \
    --cc=roman.gushchin@linux.dev \
    --cc=shakeel.butt@linux.dev \
    --cc=zhangguopeng@kylinos.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox