Skip to content

fix(memtrack): drop rss_stat samples from recycled mm_structs - #479

Draft
not-matthias wants to merge 1 commit into
cod-3231-support-disabling-allocator-trackingfrom
cod-3285-investigate-contradictory-rss-samples-jvm-duckdb
Draft

fix(memtrack): drop rss_stat samples from recycled mm_structs#479
not-matthias wants to merge 1 commit into
cod-3231-support-disabling-allocator-trackingfrom
cod-3285-investigate-contradictory-rss-samples-jvm-duckdb

Conversation

@not-matthias

Copy link
Copy Markdown
Member

Stacked on #453.

Production artifacts showed rss_stat collapsing to ~0 while the folio-rmap
reconstruction stayed high — 570 MB → 0 for 30 s on a JVM heap, 1 GB → 8 KB for
19 ms on DuckDB. The memory never moved.

Root cause is the mm_to_pid key. rss_stat ownership is keyed on the kernel's
mm_id, which is a hash of the mm_struct pointer that the tracepoint never
exposes, so the entry cannot be removed when the mm it describes goes away. Once
a tracked process execs, its old mm_struct is freed while the pid lives on, and
the next allocation from that slab slot hashes to the same id. The new address
space's near-zero counters are then emitted as the live owner's absolute RSS,
from the context of whichever task populated (dup_mmap) or tore down
(exit_mmap) the recycled mm. Both existing guards miss it: cur == owner only
catches the owner's own teardown, and size > found->size only rejects foreign
increases — a recycled mm's counters are lower, which is exactly what that
guard permits. rmap is unaffected because it keys ownership on the raw pointer
and drops it on exec.

The fix records the owner's mm alongside its pid when an entry is seeded in
context, and requires it to still be the owner's current mm before attributing an
out-of-context update. pid_mm is now maintained by the rss_stat hook too, so
the check also holds when only rss_stat is attached and the rmap hooks never
run. Genuine external reclaim is unaffected — the owner still holds the mm.

Also considered filtering all out-of-context updates, which an earlier
investigation had concluded was the only option because the spurious sample was
thought indistinguishable from valid reclaim at this hook. It is distinguishable:
the stale entry describes a recycled mm_struct, and one pointer comparison
separates the two. That also removes the need for the parser-side quarantine that
was being considered as the alternative.

testdata/rss/stale_mm_owner.c reproduces the collapse deterministically rather
than relying on the production artifact. It pins one CPU so the mm freed at exec
stays at the head of the slab freelist, execs a child into a 256 MiB-resident
image, then forks 256 short-lived children onto that slot. Every one of them
poisons the sample; the fixture reported 257 collapsed samples on each of four
runs before the fix, and none after. The test asserts both halves of the
contradiction: no absolute anon sample for the live pid falls below a quarter of
its peak, and the reconstructed rmap total shows the pages stayed resident.

Reviewers may want to look closely at the pid_mm lifetime. It is a plain hash
map, so an eviction now makes the out-of-context path fail closed and drop a
genuine reclaim sample rather than misattribute it — the safe direction, but a
behaviour change worth confirming.

Refs COD-3285

@codspeed-hq

codspeed-hq Bot commented Jul 28, 2026

Copy link
Copy Markdown

Performance comparison unavailable

A heads-up: this makes performance comparison unavailable when comparing benchmarks before and after this change:

  • Cycle estimation changes how benchmark performance is calculated. It was enabled on only one side of the comparison (base off → head on), so the values are not directly comparable. View runner releases
  • Allocator-time exclusion changes benchmark values by removing allocator time from the measurement. It was enabled on only one side of the comparison (base off → head on), so the values are not directly comparable. View runner releases

Comparing cod-3285-investigate-contradictory-rss-samples-jvm-duckdb (522f7c8) with cod-3089-collect-rss-in-memtrack (8a000e0)

Open in CodSpeed

@not-matthias
not-matthias force-pushed the cod-3285-investigate-contradictory-rss-samples-jvm-duckdb branch from bf1b373 to 8e2a087 Compare July 28, 2026 13:36
rss_stat ownership is keyed on the kernel's mm_id, a hash of the mm_struct
pointer that the tracepoint never exposes, so an entry cannot be removed when
the mm it describes goes away. Once a tracked process execs, its old mm_struct
is freed while the pid lives on, and the next allocation from that slab slot
hashes to the same id: the new address space's near-zero counters are emitted
as the live owner's absolute RSS, from the context of whichever task populated
or tore down the recycled mm. The owner then reads ~0 until its next in-context
fault, while the folio-rmap reconstruction stays high because it keys ownership
on the raw pointer and drops it on exec.

Record the owner's mm alongside its pid when an entry is seeded in context, and
require it to still be the owner's current mm before attributing an
out-of-context update. pid_mm is now maintained by this hook too, so the check
also holds when only rss_stat is attached and the rmap hooks never run.

Genuine external reclaim is unaffected: the owner still holds the mm, so the
comparison passes.

Add a fixture that reproduces the collapse deterministically. It pins one CPU so
the mm freed at exec stays at the head of the slab freelist, execs a child into
a 256 MiB-resident image, then forks 256 short-lived children onto that slot.
The test asserts both halves of the contradiction: no absolute anon sample for
the live pid falls below a quarter of its peak, and the reconstructed rmap total
shows the pages never moved.

Refs COD-3285
@not-matthias
not-matthias force-pushed the cod-3285-investigate-contradictory-rss-samples-jvm-duckdb branch from 8e2a087 to 522f7c8 Compare July 28, 2026 13:46
@not-matthias
not-matthias changed the base branch from cod-3089-collect-rss-in-memtrack to cod-3231-support-disabling-allocator-tracking July 28, 2026 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant