Export per NUMA node local DRAM latency (#599) - #599
Open
abhiShedge wants to merge 3 commits into
Open
Conversation
|
@abhiShedge has exported this pull request. If you are a Meta employee, you can view the originating Diff in D116837250. |
abhiShedge
added a commit
to abhiShedge/dynolog
that referenced
this pull request
Aug 21, 2026
Summary:
Publishes the per NUMA node latency computed in the previous commit. Adds
`KernelInfo::memLatLocalNode`, forwards it through
`KernelMonitor::sumOverHistory`, and emits `mem_lat_local_node<n>_ns`.
Three details differ from the existing per-socket latency keys, because a
socket is not a node here:
- The array is indexed by NUMA node, not socket. AMD's NPS setting splits one
socket into several nodes, so the existing `memLat*CPU[socket]` arrays
cannot represent it.
- The forwarding and accumulation loops are bounded by node count, not
`cpuSocketsInt`. Reusing the socket loops would silently carry only node 0
on a single-socket part, which is exactly the target configuration.
- Emission sits outside the `cpuSockets > 1` block. That block exists to skip
per-socket keys on single-socket hosts, but a single-socket host is precisely
where this metric is interesting.
Only nodes that actually reported a latency publish a key, so hosts that do not
compute the breakdown add no ODS keys and emission stays limited to the AMD
hosts that do.
The host-wide `memLatLocalAvg` stays 0 on AMD, matching existing behavior;
that signal already ships as `l3_to_{near,far}_dram_lat_avg_ns`.
Differential Revision: D116837250
abhiShedge
force-pushed
the
export-D116837250
branch
from
August 21, 2026 18:23
694247b to
cc4d5e4
Compare
Summary: A CPU socket is not always a NUMA node. AMD's NPS BIOS setting splits a single socket into several NUMA nodes -- on Venice under NPS2 one socket presents two DRAM nodes -- so a node cannot be derived from `getSocketCoreMapFromSysfs()`. Reporting per NUMA node memory metrics needs the real node topology. Add `getCpuToNumaNodeMapFromSysfs()`, which reads `sys/devices/system/node/node<N>/cpulist`. Two details worth noting: - `cpulist` uses inclusive ranges (`0-123,248-371`). Parsing it with a plain integer conversion would silently keep only the lower bound of each range, so ranges are expanded explicitly. - A memory-only node, such as a CXL node, has an empty `cpulist` and therefore contributes no entries. Callers deriving a node count from this map see only nodes that actually have CPUs, which is what per-CPU attribution needs. Also expose `PmuDeviceManager::getRootDir()` so callers that read sysfs themselves honor the mock tree used by tests. Differential Revision: D116798426
Summary: The amd_l3 sampled-latency counters are per CCX, and the Dram_Near data source already means "this CCX's own NUMA node". The shared uncore path sums every CCX instance into a single host sample before any average is taken, so the node breakdown is lost and only a host-wide latency is available. Read the `L3DramLat` group a second time through the new keyed API, which preserves the device key. The key carries the servicing CPU, and a CCX belongs to exactly one NUMA node, so the values can be bucketed per node and published as `L3DramLat::node<N>` metric frames alongside the untouched host frame. `calcAvgL3ToNearDramLatNanoSecForNode()` then reads a node's frame with the existing latency math. Notes: - The per node average sums the accumulators and the request counts separately before dividing, so busier CCXs weigh proportionally. Averaging the per-CCX ratios would be wrong. - The node topology comes from sysfs once at configure time; it is fixed for the lifetime of the boot. A memory-only node (CXL) has no CPUs and so never becomes an empty bucket. - The host path, its metric names, and the perf event count are unchanged, so the shipped `l3_to_*_dram_lat_avg_ns` keys are unaffected. - `saveArchSpecificMetricFrames()` is a no-op hook on the base, so Intel and ARM are unaffected. Differential Revision: D116837219
Summary:
Publishes the per NUMA node latency computed in the previous commit. Adds
`KernelInfo::memLatLocalNode`, forwards it through
`KernelMonitor::sumOverHistory`, and emits `mem_lat_local_node<n>_ns`.
Three details differ from the existing per-socket latency keys, because a
socket is not a node here:
- The array is indexed by NUMA node, not socket. AMD's NPS setting splits one
socket into several nodes, so the existing `memLat*CPU[socket]` arrays
cannot represent it.
- The forwarding and accumulation loops are bounded by node count, not
`cpuSocketsInt`. Reusing the socket loops would silently carry only node 0
on a single-socket part, which is exactly the target configuration.
- Emission sits outside the `cpuSockets > 1` block. That block exists to skip
per-socket keys on single-socket hosts, but a single-socket host is precisely
where this metric is interesting.
Only nodes that actually reported a latency publish a key, so hosts that do not
compute the breakdown add no ODS keys and emission stays limited to the AMD
hosts that do.
The host-wide `memLatLocalAvg` stays 0 on AMD, matching existing behavior;
that signal already ships as `l3_to_{near,far}_dram_lat_avg_ns`.
Differential Revision: D116837250
abhiShedge
force-pushed
the
export-D116837250
branch
from
August 21, 2026 20:45
cc4d5e4 to
5cc2ef4
Compare
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.
Summary:
Publishes the per NUMA node latency computed in the previous commit. Adds
KernelInfo::memLatLocalNode, forwards it throughKernelMonitor::sumOverHistory, and emitsmem_lat_local_node<n>_ns.Three details differ from the existing per-socket latency keys, because a
socket is not a node here:
socket into several nodes, so the existing
memLat*CPU[socket]arrayscannot represent it.
cpuSocketsInt. Reusing the socket loops would silently carry only node 0on a single-socket part, which is exactly the target configuration.
cpuSockets > 1block. That block exists to skipper-socket keys on single-socket hosts, but a single-socket host is precisely
where this metric is interesting.
Only nodes that actually reported a latency publish a key, so hosts that do not
compute the breakdown add no ODS keys and emission stays limited to the AMD
hosts that do.
The host-wide
memLatLocalAvgstays 0 on AMD, matching existing behavior;that signal already ships as
l3_to_{near,far}_dram_lat_avg_ns.Differential Revision: D116837250