mm, memcg: fix memory.peak reset clobbering other fds' watermark
Get series with b4
b4 am [email protected]Versions newest first · linked by author and title
- v4 Latest posting 2/22 patches 0 repliesNew 1 of 2 applied
- v4 posting 1/22 patches 5 repliesSuperseded
- v32 patches 3 repliesSuperseded
Earlier versions · 2
| Version | Posted | Patches | Replies | Status |
|---|---|---|---|---|
| v2 | 2 patches | 8 replies | Superseded | |
| v1 | 1 patch | 4 replies | Superseded |
Changes in v4 from the cover letter
- Switch to guard(spinlock) in the peak readers, suggested by Muchun.
Patches in v4 posting 2/2 1 of 2 applied
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/2 | memcg: acquire peaks_lock when reading memory.peakmm/ | +2 | 0 replies | |
| 2/2 | mm, memcg: fix memory.peak reset clobbering other fds' watermarkmm/ | +4 | 0 replies | Applied |
Reviews trailers per patch
All 2 patches have a Reviewed-by or Acked-by.
| Patch | Johannes Weiner | Shakeel Butt | Muchun Song | Tao Cui |
|---|---|---|---|---|
| 1/2 | A-b | A-b | R-b | |
| 2/2 | A-b | A-b | A-b |
Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes. Scroll the table sideways for all 4 reviewers.
Cover letter [PATCH v4 RESEND 0/2] mm, memcg: fix memory.peak reset clobbering other fds' watermark
From: Ridong Chen <[email protected]>
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/[email protected]?part=1
[2] https://lore.kernel.org/all/[email protected]/
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(-)
Signature · 1 line
2.34.1