Skip to content

Derive per NUMA node DRAM latency on AMD (#598) - #598

Open
abhiShedge wants to merge 2 commits into
facebookincubator:mainfrom
abhiShedge:export-D116837219
Open

Derive per NUMA node DRAM latency on AMD (#598)#598
abhiShedge wants to merge 2 commits into
facebookincubator:mainfrom
abhiShedge:export-D116837219

Conversation

@abhiShedge

@abhiShedge abhiShedge commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

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

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 20, 2026
@meta-codesync

meta-codesync Bot commented Aug 20, 2026

Copy link
Copy Markdown

@abhiShedge has exported this pull request. If you are a Meta employee, you can view the originating Diff in D116837219.

@meta-codesync meta-codesync Bot changed the title Derive per NUMA node DRAM latency on AMD Derive per NUMA node DRAM latency on AMD (#598) Aug 20, 2026
abhiShedge added a commit to abhiShedge/dynolog that referenced this pull request Aug 21, 2026
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
@abhiShedge
abhiShedge force-pushed the export-D116837219 branch 2 times, most recently from 817cfca to c413586 Compare August 21, 2026 18:23
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. meta-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant