Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mm: memcg: stop reclaim when a limit update is superseded
@ 2026-07-24  2:18 Guopeng Zhang
  2026-07-24  3:32 ` Tao Cui
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Guopeng Zhang @ 2026-07-24  2:18 UTC (permalink / raw)
  To: Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt,
	Andrew Morton
  Cc: Muchun Song, cgroups, linux-mm, linux-kernel, Guopeng Zhang,
	Guopeng Zhang

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.

Check the current limit at the start of each reclaim iteration and stop
if it no longer matches the writer's target.

Fixes: 8c8c383c04f6 ("mm: memcontrol: try harder to set a new memory.high")
Fixes: b6e6edcfa405 ("mm: memcontrol: reclaim and OOM kill when shrinking memory.max below usage")
Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
---
Reproducer:

Populate a cgroup with anonymous memory and disable swapping. Lower
memory.max from one open file, then restore it to "max" through another
open file after the new limit becomes visible.

Without the patch, the first writer remains blocked and repeatedly
increments the OOM event counter. With the patch, it returns normally.

 mm/memcontrol.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 8319ad8c5c23..638bdc766616 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4798,6 +4798,9 @@ static ssize_t memory_high_write(struct kernfs_open_file *of,
 		unsigned long nr_pages = page_counter_read(&memcg->memory);
 		unsigned long reclaimed;
 
+		if (high != READ_ONCE(memcg->memory.high))
+			break;
+
 		if (nr_pages <= high)
 			break;
 
@@ -4853,6 +4856,9 @@ static ssize_t memory_max_write(struct kernfs_open_file *of,
 	for (;;) {
 		unsigned long nr_pages = page_counter_read(&memcg->memory);
 
+		if (max != READ_ONCE(memcg->memory.max))
+			break;
+
 		if (nr_pages <= max)
 			break;
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] mm: memcg: stop reclaim when a limit update is superseded
  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-27  8:16 ` Michal Hocko
  2 siblings, 1 reply; 12+ messages in thread
From: Tao Cui @ 2026-07-24  3:32 UTC (permalink / raw)
  To: Guopeng Zhang, Johannes Weiner, Michal Hocko, Roman Gushchin,
	Shakeel Butt, Andrew Morton
  Cc: cui.tao, Muchun Song, cgroups, linux-mm, linux-kernel,
	Guopeng Zhang



在 2026/7/24 10:18, Guopeng Zhang 写道:
> 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.
> 
> Check the current limit at the start of each reclaim iteration and stop
> if it no longer matches the writer's target.
> 

Fix looks correct to me.

Acked-by: Tao Cui <cuitao@kylinos.cn>

Nit: the message lumps both paths together, but only memory.max loops
indefinitely. memory.high has no OOM path, so it just spins
MAX_RECLAIM_RETRIES times and breaks on its own. Worth a line to avoid
conflating the severity.

> Fixes: 8c8c383c04f6 ("mm: memcontrol: try harder to set a new memory.high")
> Fixes: b6e6edcfa405 ("mm: memcontrol: reclaim and OOM kill when shrinking memory.max below usage")
> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
> ---
> Reproducer:
> 
> Populate a cgroup with anonymous memory and disable swapping. Lower
> memory.max from one open file, then restore it to "max" through another
> open file after the new limit becomes visible.
> 
> Without the patch, the first writer remains blocked and repeatedly
> increments the OOM event counter. With the patch, it returns normally.
> 
>  mm/memcontrol.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 8319ad8c5c23..638bdc766616 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -4798,6 +4798,9 @@ static ssize_t memory_high_write(struct kernfs_open_file *of,
>  		unsigned long nr_pages = page_counter_read(&memcg->memory);
>  		unsigned long reclaimed;
>  
> +		if (high != READ_ONCE(memcg->memory.high))
> +			break;
> +
>  		if (nr_pages <= high)
>  			break;
>  
> @@ -4853,6 +4856,9 @@ static ssize_t memory_max_write(struct kernfs_open_file *of,
>  	for (;;) {
>  		unsigned long nr_pages = page_counter_read(&memcg->memory);
>  
> +		if (max != READ_ONCE(memcg->memory.max))
> +			break;
> +
>  		if (nr_pages <= max)
>  			break;
>  



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] mm: memcg: stop reclaim when a limit update is superseded
  2026-07-24  3:32 ` Tao Cui
@ 2026-07-24  6:53   ` Guopeng Zhang
  0 siblings, 0 replies; 12+ messages in thread
From: Guopeng Zhang @ 2026-07-24  6:53 UTC (permalink / raw)
  To: Tao Cui, Johannes Weiner, Michal Hocko, Roman Gushchin,
	Shakeel Butt, Andrew Morton
  Cc: Muchun Song, cgroups, linux-mm, linux-kernel, Guopeng Zhang



在 2026/7/24 11:32, Tao Cui 写道:
> 
> 
> 在 2026/7/24 10:18, Guopeng Zhang 写道:
>> 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.
>>
>> Check the current limit at the start of each reclaim iteration and stop
>> if it no longer matches the writer's target.
>>
> 
> Fix looks correct to me.
> 
> Acked-by: Tao Cui <cuitao@kylinos.cn>
> 
> Nit: the message lumps both paths together, but only memory.max loops
> indefinitely. memory.high has no OOM path, so it just spins
> MAX_RECLAIM_RETRIES times and breaks on its own. Worth a line to avoid
> conflating the severity.
> 
Hi,

Thanks for the review and Ack.

The message separates the two cases: the first paragraph describes the
stale-target reclaim behavior common to both, while the "For memory.max"
paragraph describes the OOM-based indefinite loop.

One detail is that memory.high is not limited to MAX_RECLAIM_RETRIES
iterations. The retry counter is decremented only when reclaim makes no
progress:

        if (!reclaimed && !nr_retries--)
                break;

If reclaim continues to make progress while pages are refaulted,
nr_retries is not decremented and the loop can still fail to converge.

Thanks,
Guopeng
>> Fixes: 8c8c383c04f6 ("mm: memcontrol: try harder to set a new memory.high")
>> Fixes: b6e6edcfa405 ("mm: memcontrol: reclaim and OOM kill when shrinking memory.max below usage")
>> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
>> ---
>> Reproducer:
>>
>> Populate a cgroup with anonymous memory and disable swapping. Lower
>> memory.max from one open file, then restore it to "max" through another
>> open file after the new limit becomes visible.
>>
>> Without the patch, the first writer remains blocked and repeatedly
>> increments the OOM event counter. With the patch, it returns normally.
>>
>>  mm/memcontrol.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index 8319ad8c5c23..638bdc766616 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -4798,6 +4798,9 @@ static ssize_t memory_high_write(struct kernfs_open_file *of,
>>  		unsigned long nr_pages = page_counter_read(&memcg->memory);
>>  		unsigned long reclaimed;
>>  
>> +		if (high != READ_ONCE(memcg->memory.high))
>> +			break;
>> +
>>  		if (nr_pages <= high)
>>  			break;
>>  
>> @@ -4853,6 +4856,9 @@ static ssize_t memory_max_write(struct kernfs_open_file *of,
>>  	for (;;) {
>>  		unsigned long nr_pages = page_counter_read(&memcg->memory);
>>  
>> +		if (max != READ_ONCE(memcg->memory.max))
>> +			break;
>> +
>>  		if (nr_pages <= max)
>>  			break;
>>  
> 



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] mm: memcg: stop reclaim when a limit update is superseded
  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-26  3:42 ` Andrew Morton
  2026-07-26  5:57   ` Guopeng Zhang
  2026-07-27  8:16 ` Michal Hocko
  2 siblings, 1 reply; 12+ messages in thread
From: Andrew Morton @ 2026-07-26  3:42 UTC (permalink / raw)
  To: Guopeng Zhang
  Cc: Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt,
	Muchun Song, cgroups, linux-mm, linux-kernel, Guopeng Zhang

On Fri, 24 Jul 2026 10:18:05 +0800 Guopeng Zhang <guopeng.zhang@linux.dev> 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.
> 
> Check the current limit at the start of each reclaim iteration and stop
> if it no longer matches the writer's target.

Thanks.

> Fixes: 8c8c383c04f6 ("mm: memcontrol: try harder to set a new memory.high")

Dec 2019

> Fixes: b6e6edcfa405 ("mm: memcontrol: reclaim and OOM kill when shrinking memory.max below usage")

Mar 2016

Having two Fixes: targets is confusing.  The Fixes: identifies which
kernel versions should be patched, so how is anyone to figure out
whether their 2018 kernel actually needs the patch?

I assume one of the two hunks is for 2019 and the other is for 2016, so
two separate patches each with a single Fixes: would be more useful.

That being said, both versions are so ancient that it probably doesn't
matter.  In fact the bug seems so obscure that I'm not planning on
adding cc:stable to this anyway.  Maintainers, please advise.

> Reproducer:
> 
> Populate a cgroup with anonymous memory and disable swapping. Lower
> memory.max from one open file, then restore it to "max" through another
> open file after the new limit becomes visible.
> 
> Without the patch, the first writer remains blocked and repeatedly
> increments the OOM event counter. With the patch, it returns normally.

This is nice info - I'll promote it into the permanent changelog.


I'll queue the patch for 7.3-rc1 and shall await maintainer input,
please.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] mm: memcg: stop reclaim when a limit update is superseded
  2026-07-26  3:42 ` Andrew Morton
@ 2026-07-26  5:57   ` Guopeng Zhang
  0 siblings, 0 replies; 12+ messages in thread
From: Guopeng Zhang @ 2026-07-26  5:57 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt,
	Muchun Song, cgroups, linux-mm, linux-kernel, Guopeng Zhang



在 2026/7/26 11:42, Andrew Morton 写道:
> On Fri, 24 Jul 2026 10:18:05 +0800 Guopeng Zhang <guopeng.zhang@linux.dev> 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.
>>
>> Check the current limit at the start of each reclaim iteration and stop
>> if it no longer matches the writer's target.
> 
> Thanks.
> 
>> Fixes: 8c8c383c04f6 ("mm: memcontrol: try harder to set a new memory.high")
> 
> Dec 2019
> 
>> Fixes: b6e6edcfa405 ("mm: memcontrol: reclaim and OOM kill when shrinking memory.max below usage")
> 
> Mar 2016
> 
> Having two Fixes: targets is confusing.  The Fixes: identifies which
> kernel versions should be patched, so how is anyone to figure out
> whether their 2018 kernel actually needs the patch?
> 
> I assume one of the two hunks is for 2019 and the other is for 2016, so
> two separate patches each with a single Fixes: would be more useful.
> 

Thanks, that makes sense.

For similar cases where separate code paths were introduced by different
commits and have different affected-version ranges, I will split the
fixes into separate patches, each with its own Fixes tag.

> That being said, both versions are so ancient that it probably doesn't
> matter.  In fact the bug seems so obscure that I'm not planning on
> adding cc:stable to this anyway.  Maintainers, please advise.
> 
>> Reproducer:
>>
>> Populate a cgroup with anonymous memory and disable swapping. Lower
>> memory.max from one open file, then restore it to "max" through another
>> open file after the new limit becomes visible.
>>
>> Without the patch, the first writer remains blocked and repeatedly
>> increments the OOM event counter. With the patch, it returns normally.
> 
> This is nice info - I'll promote it into the permanent changelog.
> 
> 
> I'll queue the patch for 7.3-rc1 and shall await maintainer input,
> please.
> 



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] mm: memcg: stop reclaim when a limit update is superseded
  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-26  3:42 ` Andrew Morton
@ 2026-07-27  8:16 ` Michal Hocko
  2026-07-27 12:59   ` Guopeng Zhang
  2 siblings, 1 reply; 12+ messages in thread
From: Michal Hocko @ 2026-07-27  8:16 UTC (permalink / raw)
  To: Guopeng Zhang
  Cc: Johannes Weiner, Roman Gushchin, Shakeel Butt, Andrew Morton,
	Muchun Song, cgroups, linux-mm, linux-kernel, Guopeng Zhang

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?
-- 
Michal Hocko
SUSE Labs


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] mm: memcg: stop reclaim when a limit update is superseded
  2026-07-27  8:16 ` Michal Hocko
@ 2026-07-27 12:59   ` Guopeng Zhang
  2026-07-27 13:58     ` Michal Hocko
  0 siblings, 1 reply; 12+ messages in thread
From: Guopeng Zhang @ 2026-07-27 12:59 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Johannes Weiner, Roman Gushchin, Shakeel Butt, Andrew Morton,
	Muchun Song, cgroups, linux-mm, linux-kernel, Guopeng Zhang



在 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.

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?

Thanks,
Guopeng




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] mm: memcg: stop reclaim when a limit update is superseded
  2026-07-27 12:59   ` Guopeng Zhang
@ 2026-07-27 13:58     ` Michal Hocko
  2026-07-29  6:15       ` Guopeng Zhang
  0 siblings, 1 reply; 12+ messages in thread
From: Michal Hocko @ 2026-07-27 13:58 UTC (permalink / raw)
  To: Guopeng Zhang
  Cc: Johannes Weiner, Roman Gushchin, Shakeel Butt, Andrew Morton,
	Muchun Song, cgroups, linux-mm, linux-kernel, Guopeng Zhang

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.

> 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.
-- 
Michal Hocko
SUSE Labs


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] mm: memcg: stop reclaim when a limit update is superseded
  2026-07-27 13:58     ` Michal Hocko
@ 2026-07-29  6:15       ` Guopeng Zhang
  2026-07-29  8:02         ` Michal Hocko
  0 siblings, 1 reply; 12+ messages in thread
From: Guopeng Zhang @ 2026-07-29  6:15 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Johannes Weiner, Roman Gushchin, Shakeel Butt, Andrew Morton,
	Muchun Song, cgroups, linux-mm, linux-kernel, Guopeng Zhang



在 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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] mm: memcg: stop reclaim when a limit update is superseded
  2026-07-29  6:15       ` Guopeng Zhang
@ 2026-07-29  8:02         ` Michal Hocko
  2026-07-29 19:11           ` Andrew Morton
  0 siblings, 1 reply; 12+ messages in thread
From: Michal Hocko @ 2026-07-29  8:02 UTC (permalink / raw)
  To: Guopeng Zhang
  Cc: Johannes Weiner, Roman Gushchin, Shakeel Butt, Andrew Morton,
	Muchun Song, cgroups, linux-mm, linux-kernel, Guopeng Zhang

On Wed 29-07-26 14:15:25, Guopeng Zhang wrote:
> 
> 
> 在 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:
[...]
> > 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.

This is an important detail to be mentioned in the changelog. Describing
motivation for a change is really important, especially if it has direct
impact in user interface behavior.
-- 
Michal Hocko
SUSE Labs


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] mm: memcg: stop reclaim when a limit update is superseded
  2026-07-29  8:02         ` Michal Hocko
@ 2026-07-29 19:11           ` Andrew Morton
  2026-07-29 19:36             ` Johannes Weiner
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Morton @ 2026-07-29 19:11 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Guopeng Zhang, Johannes Weiner, Roman Gushchin, Shakeel Butt,
	Muchun Song, cgroups, linux-mm, linux-kernel, Guopeng Zhang

On Wed, 29 Jul 2026 10:02:22 +0200 Michal Hocko <mhocko@suse.com> wrote:

> > > 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.
> 
> This is an important detail to be mentioned in the changelog. Describing
> motivation for a change is really important, especially if it has direct
> impact in user interface behavior.

I've been adding details to the changelog as they are revealed to us. 
Below is the state of play.

I await maintainer guidance on how to proceed with this!

The worst-case effects look pretty bad actually.  Should I add cc:stable?


From: Guopeng Zhang <zhangguopeng@kylinos.cn>
Subject: mm: memcg: stop reclaim when a limit update is superseded
Date: Fri, 24 Jul 2026 10:18:05 +0800

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.

Check the current limit at the start of each reclaim iteration and stop if
it no longer matches the writer's target.

Reproducer:

Populate a cgroup with anonymous memory and disable swapping.  Lower
memory.max from one open file, then restore it to "max" through another
open file after the new limit becomes visible.

Without the patch, the first writer remains blocked and repeatedly
increments the OOM event counter.  With the patch, it returns normally.

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.

Link: https://lore.kernel.org/20260724021805.1234583-1-guopeng.zhang@linux.dev
Fixes: 8c8c383c04f6 ("mm: memcontrol: try harder to set a new memory.high")
Fixes: b6e6edcfa405 ("mm: memcontrol: reclaim and OOM kill when shrinking memory.max below usage")
Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
Acked-by: Tao Cui <cuitao@kylinos.cn>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Muchun Song <muchun.song@linux.dev>
Cc: Roman Gushchin <roman.gushchin@linux.dev>
Cc: Shakeel Butt <shakeel.butt@linux.dev>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/memcontrol.c |    6 ++++++
 1 file changed, 6 insertions(+)

--- a/mm/memcontrol.c~mm-memcg-stop-reclaim-when-a-limit-update-is-superseded
+++ a/mm/memcontrol.c
@@ -4837,6 +4837,9 @@ static ssize_t memory_high_write(struct
 		unsigned long nr_pages = page_counter_read(&memcg->memory);
 		unsigned long reclaimed;
 
+		if (high != READ_ONCE(memcg->memory.high))
+			break;
+
 		if (nr_pages <= high)
 			break;
 
@@ -4892,6 +4895,9 @@ static ssize_t memory_max_write(struct k
 	for (;;) {
 		unsigned long nr_pages = page_counter_read(&memcg->memory);
 
+		if (max != READ_ONCE(memcg->memory.max))
+			break;
+
 		if (nr_pages <= max)
 			break;
 
_



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] mm: memcg: stop reclaim when a limit update is superseded
  2026-07-29 19:11           ` Andrew Morton
@ 2026-07-29 19:36             ` Johannes Weiner
  0 siblings, 0 replies; 12+ messages in thread
From: Johannes Weiner @ 2026-07-29 19:36 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Michal Hocko, Guopeng Zhang, Roman Gushchin, Shakeel Butt,
	Muchun Song, cgroups, linux-mm, linux-kernel, Guopeng Zhang

On Wed, Jul 29, 2026 at 12:11:37PM -0700, Andrew Morton wrote:
> On Wed, 29 Jul 2026 10:02:22 +0200 Michal Hocko <mhocko@suse.com> wrote:
> 
> > > > 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.
> > 
> > This is an important detail to be mentioned in the changelog. Describing
> > motivation for a change is really important, especially if it has direct
> > impact in user interface behavior.
> 
> I've been adding details to the changelog as they are revealed to us. 
> Below is the state of play.
> 
> I await maintainer guidance on how to proceed with this!
> 
> The worst-case effects look pretty bad actually.  Should I add cc:stable?
> 
> 
> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
> Subject: mm: memcg: stop reclaim when a limit update is superseded
> Date: Fri, 24 Jul 2026 10:18:05 +0800
> 
> 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.
> 
> Check the current limit at the start of each reclaim iteration and stop if
> it no longer matches the writer's target.
> 
> Reproducer:
> 
> Populate a cgroup with anonymous memory and disable swapping.  Lower
> memory.max from one open file, then restore it to "max" through another
> open file after the new limit becomes visible.
> 
> Without the patch, the first writer remains blocked and repeatedly
> increments the OOM event counter.  With the patch, it returns normally.
> 
> 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.
> 
> Link: https://lore.kernel.org/20260724021805.1234583-1-guopeng.zhang@linux.dev
> Fixes: 8c8c383c04f6 ("mm: memcontrol: try harder to set a new memory.high")
> Fixes: b6e6edcfa405 ("mm: memcontrol: reclaim and OOM kill when shrinking memory.max below usage")
> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
> Acked-by: Tao Cui <cuitao@kylinos.cn>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Muchun Song <muchun.song@linux.dev>
> Cc: Roman Gushchin <roman.gushchin@linux.dev>
> Cc: Shakeel Butt <shakeel.butt@linux.dev>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

This looks good to me. If the culprits had been more recent, it might
have made sense to split this in two, one for each Fixes. But it's 4.6
and 5.5, so I'd assume any still alive backport target would want them
both at this point anyway.

Acked-by: Johannes Weiner <hannes@cmpxchg.org>


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-07-29 19:36 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2026-07-29  8:02         ` Michal Hocko
2026-07-29 19:11           ` Andrew Morton
2026-07-29 19:36             ` Johannes Weiner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox