Skip to content

Filter perf_fds() by the builder's target_cpus - #307

Open
utpilla wants to merge 1 commit into
microsoft:mainfrom
utpilla:utpilla/Fix-perf-fds-target-cpu-filter
Open

Filter perf_fds() by the builder's target_cpus#307
utpilla wants to merge 1 commit into
microsoft:mainfrom
utpilla:utpilla/Fix-perf-fds-target-cpu-filter

Conversation

@utpilla

@utpilla utpilla commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

PerfSession::perf_fds() returned a perf fd for every CPU, ignoring the builder's target_cpus. A session created with a CPU mask (with_target_cpu) would hand back fds for CPUs the caller explicitly excluded. This filters the exposed fds down to the configured CPU set.

Motivation

Leader rings are created for every CPU because all-CPU events (kernel records, and no_cpu_mask events added via add_event) redirect into them. perf_fds() walked those leaders directly, so masked-out CPUs leaked into the returned list. A push-model consumer would then register fds for CPUs it never asked to watch.

@brianrob

Copy link
Copy Markdown
Member

@utpilla, can you share a bit more context on this change? A concern here is that even when we target a set of CPUs, there are some events (e.g. mmap events) that may come from other CPUs not in the target set that we still need. If we don't include them in perf_fds() then calls to parse_all or parse_until may be starved of these events unless/until one of the ring buffers in the target set hit the ready watermark.

@beaubelgrave

Copy link
Copy Markdown
Collaborator

Some of what you are asking about @brianrob is covered in issue #283

@utpilla

utpilla commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

@brianrob Quick context on why we filter: before this PR, perf_fds() returned an fd for every online CPU even when the session set target_cpus. So, a consumer that masks down to a subset (like a per-core session) would get an fd per CPU and register epoll/AsyncFd interest on CPUs it excluded. This PR makes perf_fds() match the configured set.

Your concern is valid though. It points at an inconsistency the filter exposed: sample events honor target_cpus, but kernel metadata (mmap/comm/fork) is captured on all CPUs (build() redirects kernel events with &None). So, metadata is all-CPU while the fds are target-only, and a purely fd-driven loop can miss cross-CPU mmap records until a target CPU trips its watermark. parse_all and parse_until are not affected since they drain every ring.

So, the real question becomes "should a target_cpus session receive kernel metadata from non-target CPUs?".

  • If no (the session is fully scoped to its CPUs), the filter is correct and this is just a docs/expectations clarification.
  • If yes, then the filter isn't the real fix. We'd need to line up the two behaviors: either collect metadata only on the target CPUs too or keep collecting it everywhere but give those rings a way to wake the fd loop.

What semantic would you lean towards?

@beaubelgrave

Copy link
Copy Markdown
Collaborator

For me, mmap/comm/fork are handled at the Exporter level, and the Exporter does not support per-CPU fds, etc. (Someone could still wire them up if they choose to).

At the PerfSession level, that does not require mmap/comm/fork (that is an Export or Machine reconstruction state thing), it is not a problem.

I'm personally fine with PerfSession and perf_fds being per-CPU only and treating the mmap/comm/fork problem as a separate concern that only really materializes if we move Export or other Export-like scenarios to per-CPU that require them (and at that point that scenario owns the solution they want to implement for that).

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.

3 participants