* [PATCH v4 0/2] mm, memcg: fix memory.peak reset clobbering other fds' watermark
@ 2026-08-14 3:30 Ridong Chen
2026-08-14 3:30 ` [PATCH v4 1/2] memcg: acquire peaks_lock when reading memory.peak Ridong Chen
[not found] ` <20260814033005.2481920-3-ridong.chen@linux.dev>
0 siblings, 2 replies; 4+ messages in thread
From: Ridong Chen @ 2026-08-14 3:30 UTC (permalink / raw)
To: Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt,
Andrew Morton
Cc: Muchun Song, David Finkel, Tejun Heo,
open list:CONTROL GROUP - MEMORY RESOURCE CONTROLLER (MEMCG),
open list:CONTROL GROUP - MEMORY RESOURCE CONTROLLER (MEMCG),
linux-kernel, Tao Cui, Ridong Chen, Ridong Chen
From: Ridong Chen <chenridong@xiaomi.com>
The memory.peak / memory.swap.peak per-fd watermark tracking has two
issues. Each open fd is a watcher and reads back max(its own value, the
shared local_watermark); both bugs live in that scheme.
Worst case for both is the same and is userspace-visible: a reader of
memory.peak (or memory.swap.peak) gets a value lower than the true peak,
so a tool that sizes or bills a cgroup by its peak usage under-reports it.
Patch 1 (read side) fixes the race Sashiko pointed out in the v1 review
[1]: peak_show() inspects local_watermark and the per-fd values without
holding peaks_lock, so a reader that races an unrelated peak_write()
reset briefly observes the lowered value. Transient. It takes peaks_lock
in the show path.
Patch 2 (write side) fixes peak_write(): on a reset it stores the
current usage into the other watchers instead of the old watermark, so
once usage has dropped from a peak a reset on one fd drags every other
fd's peak down too, even fds that never reset.
---
Changes since v3:
- Switch to guard(spinlock) in the peak readers, suggested by Muchun.
Changes since v2:
- Spell out the worst-case userspace-visible effect, per Andrew's
Go back to v1 [2].
Changes since v1:
- New patch 1: hold peaks_lock in the peak readers (Sashiko).
- Patch 2: floor the peers with max(usage, local_watermark), mirroring
peak_show(), and skip the writing fd (Johannes Weiner).
[1] https://sashiko.dev/#/patchset/20260730115314.1069089-1-ridong.chen@linux.dev?part=1
[2] https://lore.kernel.org/all/20260730115314.1069089-1-ridong.chen@linux.dev/
Ridong Chen (2):
memcg: acquire peaks_lock when reading memory.peak
mm, memcg: fix memory.peak reset clobbering other fds' watermark
mm/memcontrol.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v4 1/2] memcg: acquire peaks_lock when reading memory.peak
2026-08-14 3:30 [PATCH v4 0/2] mm, memcg: fix memory.peak reset clobbering other fds' watermark Ridong Chen
@ 2026-08-14 3:30 ` Ridong Chen
2026-08-14 3:42 ` Muchun Song
[not found] ` <20260814033005.2481920-3-ridong.chen@linux.dev>
1 sibling, 1 reply; 4+ messages in thread
From: Ridong Chen @ 2026-08-14 3:30 UTC (permalink / raw)
To: Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt,
Andrew Morton
Cc: Muchun Song, David Finkel, Tejun Heo,
open list:CONTROL GROUP - MEMORY RESOURCE CONTROLLER (MEMCG),
open list:CONTROL GROUP - MEMORY RESOURCE CONTROLLER (MEMCG),
linux-kernel, Tao Cui, Ridong Chen, Ridong Chen
From: Ridong Chen <chenridong@xiaomi.com>
Sashiko reported that a reader can transiently observe a lower peak
within a race window [1]. peak_show() returns
max(local_watermark, ofp->value), but peak_write() updates those two
under peaks_lock while the reader takes no lock. The interleaving is:
writer (reset on fd A) reader (fd B)
---------------------- -------------
usage = page_counter_read(pc)
WRITE_ONCE(local_watermark, usage)
// watermark lowered to usage
lw = READ_ONCE(local_watermark)
// sees the lowered usage
val = READ_ONCE(ofp->value)
// B's value not updated yet
return max(lw, val)
// both low -> low peak
WRITE_ONCE(peer_ctx->value, usage)
// B updated, but too late
Fix it by acquiring peaks_lock when reading the peak, so the reader sees
a consistent snapshot of local_watermark and the per-fd values. The same
race applies to memory.swap.peak, which shares peaks_lock and the
peak_write() path, so take the lock there as well.
[1] https://sashiko.dev/#/patchset/20260730115314.1069089-1-ridong.chen@linux.dev?part=1
Fixes: c6f53ed8f213 ("mm, memcg: cg2 memory{.swap,}.peak write handlers")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
---
mm/memcontrol.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 17da1f43b7d3..ceb43b855122 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4714,6 +4714,7 @@ static int memory_peak_show(struct seq_file *sf, void *v)
{
struct mem_cgroup *memcg = mem_cgroup_from_css(seq_css(sf));
+ guard(spinlock)(&memcg->peaks_lock);
return peak_show(sf, v, &memcg->memory);
}
@@ -5859,6 +5860,7 @@ static int swap_peak_show(struct seq_file *sf, void *v)
{
struct mem_cgroup *memcg = mem_cgroup_from_css(seq_css(sf));
+ guard(spinlock)(&memcg->peaks_lock);
return peak_show(sf, v, &memcg->swap);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v4 1/2] memcg: acquire peaks_lock when reading memory.peak
2026-08-14 3:30 ` [PATCH v4 1/2] memcg: acquire peaks_lock when reading memory.peak Ridong Chen
@ 2026-08-14 3:42 ` Muchun Song
0 siblings, 0 replies; 4+ messages in thread
From: Muchun Song @ 2026-08-14 3:42 UTC (permalink / raw)
To: Ridong Chen
Cc: Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt,
Andrew Morton, David Finkel, Tejun Heo,
open list:CONTROL GROUP - MEMORY RESOURCE CONTROLLER (MEMCG),
open list:CONTROL GROUP - MEMORY RESOURCE CONTROLLER (MEMCG),
linux-kernel, Tao Cui, Ridong Chen
> On Aug 14, 2026, at 11:30, Ridong Chen <ridong.chen@linux.dev> wrote:
>
> From: Ridong Chen <chenridong@xiaomi.com>
>
> Sashiko reported that a reader can transiently observe a lower peak
> within a race window [1]. peak_show() returns
> max(local_watermark, ofp->value), but peak_write() updates those two
> under peaks_lock while the reader takes no lock. The interleaving is:
>
> writer (reset on fd A) reader (fd B)
> ---------------------- -------------
> usage = page_counter_read(pc)
> WRITE_ONCE(local_watermark, usage)
> // watermark lowered to usage
> lw = READ_ONCE(local_watermark)
> // sees the lowered usage
> val = READ_ONCE(ofp->value)
> // B's value not updated yet
> return max(lw, val)
> // both low -> low peak
> WRITE_ONCE(peer_ctx->value, usage)
> // B updated, but too late
>
> Fix it by acquiring peaks_lock when reading the peak, so the reader sees
> a consistent snapshot of local_watermark and the per-fd values. The same
> race applies to memory.swap.peak, which shares peaks_lock and the
> peak_write() path, so take the lock there as well.
>
> [1] https://sashiko.dev/#/patchset/20260730115314.1069089-1-ridong.chen@linux.dev?part=1
> Fixes: c6f53ed8f213 ("mm, memcg: cg2 memory{.swap,}.peak write handlers")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
Reviewed-by: Muchun Song <muchun.song@linux.dev>
Thanks
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v4 2/2] mm, memcg: fix memory.peak reset clobbering other fds' watermark
[not found] ` <20260814033005.2481920-3-ridong.chen@linux.dev>
@ 2026-08-15 0:38 ` Shakeel Butt
0 siblings, 0 replies; 4+ messages in thread
From: Shakeel Butt @ 2026-08-15 0:38 UTC (permalink / raw)
To: Ridong Chen
Cc: Johannes Weiner, Michal Hocko, Roman Gushchin, Andrew Morton,
Muchun Song, David Finkel, Tejun Heo,
open list:CONTROL GROUP - MEMORY RESOURCE CONTROLLER (MEMCG),
open list:CONTROL GROUP - MEMORY RESOURCE CONTROLLER (MEMCG),
linux-kernel, Tao Cui, Ridong Chen
On Fri, Aug 14, 2026 at 11:30:05AM +0800, Ridong Chen wrote:
> From: Ridong Chen <chenridong@xiaomi.com>
>
> Writing to memory.peak resets the peak for that fd only. Each fd is a
> watcher and reads back max(its own value, the shared local_watermark).
>
> peak_write() resets by lowering local_watermark to the current usage.
> To keep the other watchers' peaks it then walks the watcher list, but it
> stores the current usage into them instead of the old watermark. So once
> usage has dropped from a peak, a reset on one fd wrongly drags every
> other fd's peak down too, even fds that never reset.
>
> Reproduced on 7.2.0-rc5-next under QEMU, two fds A and B on one cgroup:
> B sees the peak (410624 KB), usage drops, then A resets -- and B's peak
> collapses to 1060 KB although B never reset. With this patch B keeps
> reading 410624 KB.
>
> Fix: save the old watermark before lowering it and use that to floor the
> other watchers, so a reset only affects the fd that issued it.
>
> Fixes: c6f53ed8f213 ("mm, memcg: cg2 memory{.swap,}.peak write handlers")
> Closes: https://sashiko.dev/#/patchset/20260807090000.1532495-1-ridong.chen@linux.dev
> Assisted-by: Claude:claude-opus-4-8
> Acked-by: Tao Cui <cuitao@kylinos.cn>
> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-08-15 0:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-14 3:30 [PATCH v4 0/2] mm, memcg: fix memory.peak reset clobbering other fds' watermark Ridong Chen
2026-08-14 3:30 ` [PATCH v4 1/2] memcg: acquire peaks_lock when reading memory.peak Ridong Chen
2026-08-14 3:42 ` Muchun Song
[not found] ` <20260814033005.2481920-3-ridong.chen@linux.dev>
2026-08-15 0:38 ` [PATCH v4 2/2] mm, memcg: fix memory.peak reset clobbering other fds' watermark Shakeel Butt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox