fix(memtrack): drop rss_stat samples from recycled mm_structs - #479
Draft
not-matthias wants to merge 1 commit into
Conversation
Performance comparison unavailableA heads-up: this makes performance comparison unavailable when comparing benchmarks before and after this change:
Comparing |
not-matthias
force-pushed
the
cod-3285-investigate-contradictory-rss-samples-jvm-duckdb
branch
from
July 28, 2026 13:36
bf1b373 to
8e2a087
Compare
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
force-pushed
the
cod-3285-investigate-contradictory-rss-samples-jvm-duckdb
branch
from
July 28, 2026 13:46
8e2a087 to
522f7c8
Compare
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #453.
Production artifacts showed
rss_statcollapsing to ~0 while the folio-rmapreconstruction 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_pidkey.rss_statownership is keyed on the kernel'smm_id, which is a hash of themm_structpointer that the tracepoint neverexposes, so the entry cannot be removed when the mm it describes goes away. Once
a tracked process execs, its old
mm_structis freed while the pid lives on, andthe 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 == owneronlycatches the owner's own teardown, and
size > found->sizeonly rejects foreignincreases — 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_mmis now maintained by therss_stathook too, sothe check also holds when only
rss_statis attached and the rmap hooks neverrun. 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 comparisonseparates the two. That also removes the need for the parser-side quarantine that
was being considered as the alternative.
testdata/rss/stale_mm_owner.creproduces the collapse deterministically ratherthan 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_mmlifetime. It is a plain hashmap, 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