feat(memtrack): collect RSS via rss_stat and folio-rmap reconstruction - #453
Conversation
Merging this PR will not alter performance
|
Greptile SummaryThis PR adds RSS collection to memtrack. The main changes are:
Confidence Score: 4/5This is close, but the optional rmap startup path should be fixed before merging.
Files Needing Attention: crates/memtrack/src/ebpf/memtrack/rmap.rs and crates/memtrack/src/ebpf/memtrack/tracking.rs
|
| Filename | Overview |
|---|---|
| crates/memtrack/src/ebpf/memtrack/rmap.rs | Adds rmap support detection based on kernel version, but the PUD tier can be enabled when the actual fentry targets are unavailable. |
| crates/memtrack/src/ebpf/memtrack/tracking.rs | Attaches lifecycle, RSS, and rmap programs; optional PUD rmap attach errors still stop tracker startup. |
| crates/memtrack/tests/shared.rs | Adds RSS/rmap test helpers that pass rmap enablement directly instead of mutating process-wide environment state. |
Prompt To Fix All With AI
### Issue 1
crates/memtrack/src/ebpf/memtrack/rmap.rs:54
**PUD hooks can abort startup**
This treats every 6.15+ kernel as having the PUD rmap fentry targets, but those targets depend on kernel config and architecture support. When `CODSPEED_MEMTRACK_TRACK_RMAP=1` runs on a 6.15+ kernel without the PUD pair, `CoreAndPud` keeps those programs enabled and `attach_tracepoints()` propagates the first missing-target attach error. That stops the tracker instead of falling back to the working PTE/PMD rmap hooks. Please detect the actual PUD BTF targets, or make only the PUD attaches best-effort while keeping core rmap support active.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (29): Last reviewed commit: "ci(memtrack): run bpf tests on arm and a..." | Re-trigger Greptile
b670b0a to
2f41984
Compare
41945a5 to
64688a9
Compare
b02f3cb to
411b713
Compare
e00fb6b to
a68424a
Compare
e48a0a0 to
f305358
Compare
|
@greptileai pause reviews for now until i enable them again |
|
Understood — pausing reviews. I'll hold off until you re-enable them. Tip: You can customize Greptile's behavior for this repo with |
f4df079 to
921e469
Compare
fa0526f to
e3a67b4
Compare
e3a67b4 to
2024266
Compare
29fa3da to
e5b96d3
Compare
205b479 to
0b9565e
Compare
|
@greptileai review again, focus on the eBPF code (mostly RMAP tracking), be exhaustive |
GuillaumeLagrange
left a comment
There was a problem hiding this comment.
olgtm, obviously quite a big chunk to chew, but looks mostly fine to me. Main point of concern is the maintainability of "which function can be track for which kernel version" which IMO could be improved.
7573f98 to
b5a8e9e
Compare
Set AllowShortFunctionsOnASingleLine: None in .clang-format and apply it, reformatting the allocator uprobe macros accordingly.
A forked child's inherited RSS is invisible to rss_stat: the fork-time counter copies fire outside the child's context, and anon COW faults are counter-neutral, so a child that only touches inherited memory never reports anything on its own. A fork event carrying the parent pid lets consumers seed the child from the parent's last absolutes; exec and exit mark where the address space is replaced or torn down.
Sample the kernel's per-mm resident counter through the kmem:rss_stat tracepoint, emitting absolute byte values per mm member. Adds the EVENT_TYPE_RSS contract, MemtrackEventKind::Rss, the parser arm, and a writer bench case. An rss_stat update from reclaim or another process's madvise fires in the actor's context; track (mm_id, member) -> owning pid so those updates reach the owner. External events may only lower a counter, so stale reads and mm_id collisions cannot invent peaks.
Attach fentry hooks on the folio-rmap add/remove functions, emitting signed page-count deltas per MM_* bucket so anon, file, and shmem RSS can be reconstructed over time. Gated behind CODSPEED_MEMTRACK_TRACK_RMAP; the programs stay autoload-off by default so the skeleton loads on any kernel, with the PUD pair (only present since v6.15) gated separately from the core set so rmap still works on older kernels. Adds the EVENT_TYPE_RMAP contract, MemtrackEventKind::Rmap, parser arm, and bench case. Recover the owning pid for rmap events run by another task (reclaim, process_madvise, khugepaged, KSM) from the mm_struct pointer, and maintain the ownership maps across exec and thread-group exit. The same ownership binding also validates external (curr==0) rss_stat updates, so a stale mm can no longer attribute a counter to the wrong pid.
Add the rss_tests integration suite: per-workload RSS/rmap reconstruction snapshots against /proc ground truth, fork-seeded child RSS, exec/exit resets, foreign-actor rmap attribution (reclaim, external madvise), and mm-ownership across CLONE_VM and exec. Extend tests/shared.rs with the tracker/fixture helpers these tests need and move compile_c_source into it for reuse. The suite needs two surfaces the production paths don't: a tracker mode that skips the allocator probes and exec-mapping watcher, and readers for the mm-ownership maps.
Add an aarch64 lane to the bpf-tests matrix and run the rss integration tests alongside the existing test binaries.
b5a8e9e to
f6e60e6
Compare
|
@greptileai review |
Summary
Adds RSS (resident set size) collection to
memtrack, in two layers:kmem:rss_stattracepoint — absolute per-mm resident bytes (anon/file/shmem/swap), latest-wins.fentryhooks on the anon folio-rmap add/remove functions emit signed page-count deltas, so anon RSS can be rebuilt over time asΣ(add − remove) × PAGE_SIZE.The reconstruction is a total anon RSS delta-sum, not a per-vaddr resident map — the kernel remove hook (
folio_remove_rmap_ptes) carries no address, so removals can't be attributed to a vaddr (the add hooks' faulting vaddr is emitted for observability only).Commits
feat(memtrack): track RSS via kmem:rss_stat tracepoint— the baseline:EVENT_TYPE_RSScontract,MemtrackEventKind::Rss, parser arm, writer bench case, gated integration test.feat(memtrack): reconstruct anon RSS from gated folio rmap fentry hooks—EVENT_TYPE_RMAP_ANON+RmapAnonevent, fivefentryprograms (add_new / add_ptes / remove_ptes / remove_pmd / remove_pud), CO-RE folio helpers, and the load/attach gating.test(memtrack): validate anon RSS reconstruction against rss_stat— ramps anon RSS viammap/munmapand asserts the reconstructed estimate tracks therss_statMM_ANONPAGES peak within 25%.What's on by default vs gated
rss_stattracepoint: always on. EmittingRssevents is the intended new default behavior introduced by this change — the RSS tracepoint is not gated.RmapAnonfolio-rmapfentryprograms: off by default, gated behindCODSPEED_MEMTRACK_TRACK_RMAP=1. When the flag is unset they areset_autoload(false)before load and never attached, so:fentryBTF target would otherwise fail the whole load), and--mode memory) and out of the existing test suites — noRmapAnonevents are produced by default.Verification
Run in a privileged,
--pid=hostcontainer sharing the host kernel (7.0.12):real anon amplitude = 64 MiB, estimated peak = 64 MiB(ratio 1.00).rss_tests, flag unset): ✅ passes — noRmapAnonevents, folio-rmap programs stay unloaded.cargo fmt, andclippyclean.folio_*_rmap*functions verified to match theBPF_PROGarg layouts.Review notes (draft)
track_commandordering: the shared test helper spawns the child beforeenable()/track(root_pid). In practice the child'sfork→execve→ld.so→libc-initfar outlasts the two BPF-map updates, so tracking is armed before the workload allocates (both fixtures captured full event streams). Flagging in case we'd prefer a leading settle-usleepin the fixtures or an enable-before-spawn change in the helper.PAGE_SIZE: hardcoded to 4096 (correct on x86_64). On a 16K/64K-page arm64 runner the estimate would needsysconf(_SC_PAGESIZE);rss_statis already in bytes and unaffected. Happy to switch tosysconfif these tests run on arm64 CI.