Skip to content

fix(memtrack): attach lifecycle and syscall hooks without perf_event_open - #536

Merged
GuillaumeLagrange merged 3 commits into
mainfrom
cod-3492-memtrack-classic-tracepoints-cannot-attach-inside-the
Sep 16, 2026
Merged

GuillaumeLagrange merged 3 commits into
mainfrom
cod-3492-memtrack-classic-tracepoints-cannot-attach-inside-the

Conversation

@GuillaumeLagrange

Copy link
Copy Markdown
Contributor

No description provided.

…open

Classic `tracepoint/*` programs attach through tracefs and
perf_event_open, which a delegated BPF token cannot authorize: the token
only covers bpf(). Inside a token-delegated user namespace every
tracepoint attach failed on the first one, sched_process_exec, so
memtrack could not run at all there.

Move the seven classic tracepoints to BTF-typed programs that attach via
bpf() alone:

- sched_process_exec/exit -> tp_btf (same tracepoint, raw attach)
- sys_enter/exit_mmap, sys_enter_munmap, sys_enter/exit_brk ->
  fentry/fexit on the arch syscall wrapper, `__x64_sys_*` or
  `__arm64_sys_*` selected from `__TARGET_ARCH_*`

Hook points and observed values are unchanged: fexit receives the same
return value as sys_exit's `ret`, and the register-based args match
sys_enter's `args[]`. As a side effect the run no longer flips every
task onto the syscall tracepoint slow path, which classic `syscalls/*`
tracepoints do system-wide. fentry is now required even with allocator
tracking disabled; the x86_64 floor stays at 5.5 while aarch64 needs
6.4 for BPF trampolines.

Known limitation: `tracepoint/kmem/rss_stat` (physical mode) is still a
classic tracepoint. Its TP_fast_assign-derived fields (mm_id, curr,
size) have no tp_btf equivalent, so where perf_event_open is denied it
fails to attach and only logs a warning. Physical RSS then comes from
the folio rmap hooks alone: complete on kernels >= 6.15, missing
PUD-mapped THP on 6.8-6.14, and absent below 6.8.

Refs COD-3492
Co-Authored-By: Claude <noreply@anthropic.com>
@GuillaumeLagrange
GuillaumeLagrange marked this pull request as ready for review September 16, 2026 08:42
@codspeed

codspeed Bot commented Sep 16, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

✅ 17 untouched benchmarks


Comparing cod-3492-memtrack-classic-tracepoints-cannot-attach-inside-the (af3867e) with main (37eba9e)

Open in CodSpeed

@greptile-apps

greptile-apps Bot commented Sep 16, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

The PR should not merge until older persisted memtrack artifacts containing mapping events can be read without silently truncating their event streams.

Fix All in Claude CodeFindings

  1. P1 Legacy artifacts truncate silently
Fix with agent prompt
### Issue 1
crates/runner-shared/src/artifacts/memtrack/mod.rs:69
Removing the `Mmap`, `Munmap`, and `Brk` variants breaks reading artifacts produced by earlier memtrack versions. When the stream reaches one of these now-unknown variants, deserialization fails and `.ok()` turns that error into the end of the iterator. The resulting profile silently loses that event and every event after it. Please retain support for these legacy variants or skip them without ending the stream.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

This PR avoids attaching memory-mapping syscall hooks, moves process exec/exit tracking to BTF tracepoints, and removes the unused mmap/munmap/brk event model. It also relaxes a timing-test threshold to account for parent-process preemption.

  • Removes mmap, munmap, and brk probes and their event payloads.
  • Renumbers the remaining internal eBPF event identifiers.
  • Retains lifecycle tracking through fork, exec, and exit hooks.
  • Updates memtrack tests and writer benchmarks for the reduced event set.
  • Leaves a backward-compatibility problem when reading artifacts containing the removed variants.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
    P[Tracked process tree] --> A[Allocator uprobes]
    P --> L[BTF lifecycle hooks]
    P --> R[Optional RSS and rmap hooks]
    A --> B[eBPF ring buffer]
    L --> B
    R --> B
    B --> U[Rust event parser]
    U --> W[MessagePack artifact writer]
    W --> D[Persisted MemtrackArtifact]
    D --> S[Artifact stream reader]
Loading

Reviews (2) · Last reviewed commit: "test(exec-harness): allow a 1ms shortfal..."

Comment thread crates/memtrack/src/ebpf/c/allocator.h Outdated

@not-matthias not-matthias left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread crates/memtrack/src/ebpf/c/allocator.h
Comment thread crates/memtrack/src/ebpf/c/allocator.h
Nothing reads them: they were emitted into the artifact and no consumer
ever modelled them, so the syscall hooks only cost overhead.

Refs COD-3492
The round clock starts after the spawn, so a preempted parent can miss
the start of the child's sleep.
Comment thread crates/runner-shared/src/artifacts/memtrack/mod.rs
@GuillaumeLagrange
GuillaumeLagrange merged commit af3867e into main Sep 16, 2026
48 checks passed
@GuillaumeLagrange
GuillaumeLagrange deleted the cod-3492-memtrack-classic-tracepoints-cannot-attach-inside-the branch September 16, 2026 10:49
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.

2 participants