fix(memory/topology): Fallback to NVML for GPU NUMA node when sysfs reports -1#119
Open
nirandaperera wants to merge 3 commits intoNVIDIA:mainfrom
Open
fix(memory/topology): Fallback to NVML for GPU NUMA node when sysfs reports -1#119nirandaperera wants to merge 3 commits intoNVIDIA:mainfrom
nirandaperera wants to merge 3 commits intoNVIDIA:mainfrom
Conversation
Signed-off-by: niranda perera <niranda.perera@gmail.com>
Signed-off-by: niranda perera <niranda.perera@gmail.com>
pentschev
requested changes
May 6, 2026
Comment on lines
+872
to
+884
| gpu.numa_node = get_numa_node_from_sys(gpu.pci_bus_id); | ||
| // Fallback: if the kernel reports -1 (typical when ACPI SRAT lacks PCIe | ||
| // affinity data), ask NVML directly. NVML walks the GPU driver's PCI bridge | ||
| // topology rather than relying on firmware tables, so it usually has the | ||
| // correct answer (same source as `nvidia-smi topo -m`). | ||
| if (gpu.numa_node == -1) { | ||
| unsigned long nodeset = 0; | ||
| if (nvml.p_nvmlDeviceGetMemoryAffinity(device, 1, &nodeset, NVML_AFFINITY_SCOPE_NODE) == | ||
| NVML_SUCCESS && | ||
| nodeset != 0) { | ||
| gpu.numa_node = std::countr_zero(nodeset); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
I'm actually considering whether we need get_numa_node_from_sys anyway, do we use it elsewhere? If not, we should probably just remove it completely and rely directly on nvmlDeviceGetMemoryAffinity for this.
Contributor
Author
There was a problem hiding this comment.
Sure, I was thinking the same TBH.
Comment on lines
+201
to
+202
| if (topology.num_gpus == 0 || topology.num_numa_nodes <= 0) { | ||
| SUCCEED("Skipped: requires at least one GPU and a NUMA-aware host"); |
Contributor
There was a problem hiding this comment.
Does a non-NUMA-aware host even exists, at least in the context that we care about and test? I think not and we should remove the second condition.
Signed-off-by: niranda perera <niranda.perera@gmail.com>
pentschev
requested changes
May 7, 2026
Contributor
pentschev
left a comment
pentschev
left a comment
There was a problem hiding this comment.
Couple of request to tidy up docs/comments, after that we should be good.
Comment on lines
+185
to
+193
| // Regression test for the bug where GPU NUMA node was reported as -1 on hosts whose ACPI | ||
| // SRAT/SLIT tables do not publish PCIe-to-NUMA affinity data. The previous implementation | ||
| // read /sys/bus/pci/devices/<pci>/numa_node, which returns -1 in that case; | ||
| // topology_discovery now resolves the NUMA node via nvmlDeviceGetMemoryAffinity, which | ||
| // walks the GPU driver's PCI bridge topology and is unaffected by firmware quirks. | ||
| // | ||
| // Invariant: when the host advertises NUMA topology and GPUs are present, every discovered | ||
| // GPU must resolve to a valid NUMA node. This test would catch a regression that | ||
| // re-introduced the -1 leak. |
Contributor
There was a problem hiding this comment.
Suggested change
| // Regression test for the bug where GPU NUMA node was reported as -1 on hosts whose ACPI | |
| // SRAT/SLIT tables do not publish PCIe-to-NUMA affinity data. The previous implementation | |
| // read /sys/bus/pci/devices/<pci>/numa_node, which returns -1 in that case; | |
| // topology_discovery now resolves the NUMA node via nvmlDeviceGetMemoryAffinity, which | |
| // walks the GPU driver's PCI bridge topology and is unaffected by firmware quirks. | |
| // | |
| // Invariant: when the host advertises NUMA topology and GPUs are present, every discovered | |
| // GPU must resolve to a valid NUMA node. This test would catch a regression that | |
| // re-introduced the -1 leak. | |
| // Invariant: when the host advertises NUMA topology and GPUs are present, every discovered | |
| // GPU must resolve to a valid NUMA node. |
Contributor
There was a problem hiding this comment.
We don't care about details of the previous implementation. Only the expected behavior, which is what being tested, matters.
Comment on lines
+337
to
+341
| * Queries NVML's `nvmlDeviceGetMemoryAffinity` with `NVML_AFFINITY_SCOPE_NODE` and | ||
| * returns the lowest-numbered NUMA node in the resulting bitmask. NVML walks the | ||
| * GPU driver's PCI bridge topology directly, so this is unaffected by ACPI | ||
| * SRAT/SLIT firmware quirks that cause `/sys/bus/pci/devices/<pci>/numa_node` to | ||
| * report -1 on otherwise NUMA-aware hosts. Same source as `nvidia-smi topo -m`. |
Contributor
There was a problem hiding this comment.
Suggested change
| * Queries NVML's `nvmlDeviceGetMemoryAffinity` with `NVML_AFFINITY_SCOPE_NODE` and | |
| * returns the lowest-numbered NUMA node in the resulting bitmask. NVML walks the | |
| * GPU driver's PCI bridge topology directly, so this is unaffected by ACPI | |
| * SRAT/SLIT firmware quirks that cause `/sys/bus/pci/devices/<pci>/numa_node` to | |
| * report -1 on otherwise NUMA-aware hosts. Same source as `nvidia-smi topo -m`. | |
| * Queries NVML's `nvmlDeviceGetMemoryAffinity` with `NVML_AFFINITY_SCOPE_NODE` and | |
| * returns the lowest-numbered NUMA node in the resulting bitmask. Same source as | |
| * `nvidia-smi topo -m`. |
Contributor
There was a problem hiding this comment.
Same here, we don't care about the previous implementations.
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
2 participants
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.
GPU
numa_nodewas reported as-1on hosts whose ACPI firmware (SRAT/SLIT tables) does not publish PCIe-to-NUMA affinity data. The kernel writes-1to/sys/bus/pci/devices/<pci>/numa_nodein that case, andtopology_discovery::discover()propagated that straight intogpu_topology_info::numa_node— even though the NVIDIA driver knows the correct NUMA node via its own PCI-bridge traversal (the same sourcenvidia-smi topo -muses).This change adds an NVML-driven fallback. When sysfs returns
-1, we now querynvmlDeviceGetMemoryAffinity(..., NVML_AFFINITY_SCOPE_NODE)and pick the lowest-numbered NUMA node from the returned bitmask.Implementation notes
nvmlDeviceGetMemoryAffinityis added to the required symbol set inNvmlLoadernodeSetSize = 1(oneunsigned long= 64 NUMA bits) — covers any realistic system;CONFIG_NODES_SHIFTdefaults to 6 on x86_64 and the fallback only runs in the rare ACPI-quirk case anyway.nvmlDeviceGetNumaNodeIdwas considered but rejected: it returns the NUMA node of the GPU itself, which is only meaningful on Grace/GB200-style coherent platforms where the GPU is its own NUMA node.