* [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
2026-08-14 3:30 ` [PATCH v4 2/2] mm, memcg: fix memory.peak reset clobbering other fds' watermark Ridong Chen
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* [PATCH v4 2/2] mm, memcg: fix memory.peak reset clobbering other fds' watermark
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:30 ` Ridong Chen
1 sibling, 0 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>
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>
---
mm/memcontrol.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index ceb43b855122..5932046c637d 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4744,7 +4744,7 @@ static ssize_t peak_write(struct kernfs_open_file *of, char *buf, size_t nbytes,
loff_t off, struct page_counter *pc,
struct list_head *watchers)
{
- unsigned long usage;
+ unsigned long usage, old_watermark;
struct cgroup_of_peak *peer_ctx;
struct mem_cgroup *memcg = mem_cgroup_from_css(of_css(of));
struct cgroup_of_peak *ofp = of_peak(of);
@@ -4752,11 +4752,12 @@ static ssize_t peak_write(struct kernfs_open_file *of, char *buf, size_t nbytes,
spin_lock(&memcg->peaks_lock);
usage = page_counter_read(pc);
+ old_watermark = READ_ONCE(pc->local_watermark);
WRITE_ONCE(pc->local_watermark, usage);
list_for_each_entry(peer_ctx, watchers, list)
- if (usage > peer_ctx->value)
- WRITE_ONCE(peer_ctx->value, usage);
+ if (peer_ctx != ofp && old_watermark > peer_ctx->value)
+ WRITE_ONCE(peer_ctx->value, old_watermark);
/* initial write, register watcher */
if (ofp->value == OFP_PEAK_UNSET)
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread