fix: identify GPU by PCI bus id when probing hw decompression - #176
Open
aminaramoon wants to merge 3 commits into
Open
fix: identify GPU by PCI bus id when probing hw decompression#176aminaramoon wants to merge 3 commits into
aminaramoon wants to merge 3 commits into
Conversation
query_hw_decompression took a device ordinal and resolved it with
rmm::cuda_set_device_raii. Ordinals are not a stable identity across
APIs: NVML enumerates in PCI-bus order while the CUDA runtime defaults
to CUDA_DEVICE_ORDER=FASTEST_FIRST, so gpu.id -- a position in this
discovery's own CUDA_VISIBLE_DEVICES-filtered list -- need not name the
same device to CUDA on a heterogeneous host.
The mismatch was latent because rmm::detail::hwdecompress::is_supported()
only calls cudaDriverGetVersion; it answers "is the driver >= 12.8",
never a per-device question, so the ordinal selected nothing.
Query CU_DEVICE_ATTRIBUTE_MEM_DECOMPRESS_ALGORITHM_MASK against the
device resolved by cuDeviceGetByPCIBusId instead. That is immune to both
the FASTEST_FIRST reordering and CUDA_VISIBLE_DEVICES remapping, takes
its device explicitly (no context created, current device untouched),
and reports actual silicon capability rather than a driver version.
The driver API is reached via dlopen("libcuda.so.1") + dlsym rather than
a link dependency, so the library still loads on driverless hosts --
matching the treatment of NVML -- and CUdevice/CUresult are spelled as
int to avoid pulling in <cuda.h>. The runtime API is not an option here:
this CUDA version exposes no cudaDevAttrMemDecompress* equivalent.
This removes the only RMM use in topology_discovery.cpp, so drop
rmm::rmm from the three topology targets. Side effect:
CUCASCADE_TOPOLOGY_ONLY=ON now configures and builds -- it never calls
find_package(rmm), so linking rmm::rmm had it failing at generate time.
Behavior change: hw_decompression_available now reports false on
pre-Blackwell GPUs that previously reported true on any >= 12.8 driver.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
wence-
reviewed
Jul 31, 2026
Comment on lines
+67
to
+69
| * `CUdevice` and `CUresult` are spelled as `int` to avoid pulling in `<cuda.h>`: | ||
| * `CUdevice` is a typedef for `int` and `CUresult` is an int-sized enum, so both | ||
| * match the driver ABI. |
There was a problem hiding this comment.
This is only necessary if you want to be able to compile without cuda.h. Is that required?
Contributor
Author
There was a problem hiding this comment.
I guess topology discovery could, but you do have a point as cucascade has cuda as a dependency. so technicallyt it will never happen. I can change that.
Address review feedback on NVIDIA#176. Spell the driver entry points with CUdevice/CUresult/CUdevice_attribute and use CU_DEVICE_ATTRIBUTE_MEM_DECOMPRESS_ALGORITHM_MASK directly rather than hand-rolled int signatures and a literal 136. Including <cuda.h> costs nothing here: the toolkit include path already comes in via CUDA::nvml_static, the project requires CUDA 12.9+ so the 12.8 enumerator is always present, and the header adds no link dependency -- the .so still has no DT_NEEDED on libcuda.so.1. The dlopen indirection stays, since that is what keeps the library loadable on driverless hosts; only the type spelling changes. Resolve symbols by clearing dlerror() and inspecting it afterwards. A null return from dlsym is not by itself an error, so the previous null-check was the wrong test. This also drops the memcpy: a plain reinterpret_cast compiles clean under the project's full warning set including -Wpedantic -Werror, so the workaround was unnecessary. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Remove 19 comments in topology_discovery.cpp that narrate the line
below them without adding context ("// Get GPU count" above a
GetCount call, "// Convert to lowercase" above a tolower loop, and
similar).
Comments carrying information the code cannot express are kept: the
NVML re-init SEGV explanation, the MIG parent/instance rationale, the
NVML-vs-sysfs PCI bus id format mismatch, the path-type proximity
heuristic, and the /sys state file format.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Problem
query_hw_decompressiontook a device ordinal and resolved it withrmm::cuda_set_device_raii:rmm::cuda_set_device_raii set_device{rmm::cuda_device_id{static_cast<int>(cuda_ordinal)}}; return rmm::detail::hwdecompress::is_supported();Ordinals are not a stable identity across APIs. NVML enumerates in PCI-bus order, while the CUDA runtime defaults to
CUDA_DEVICE_ORDER=FASTEST_FIRST. The value passed in wasgpu.id— a position in this discovery's ownCUDA_VISIBLE_DEVICES-filtered list — which need not name the same device to CUDA on a heterogeneous host (mixed SKUs, or a display GPU alongside compute GPUs).The mismatch was latent because
rmm::detail::hwdecompress::is_supported()only callscudaDriverGetVersion. It answers "is the driver >= 12.8" and never asks a per-device question, so the ordinal selected nothing and theset_devicewas decoration. Any real per-device query would have made the wrong ordinal load-bearing.Change
Query
CU_DEVICE_ATTRIBUTE_MEM_DECOMPRESS_ALGORITHM_MASKagainst the device resolved bycuDeviceGetByPCIBusId:This is immune to both the FASTEST_FIRST reordering and
CUDA_VISIBLE_DEVICESremapping, takes its device explicitly (no context created, calling thread's current device untouched), and reports actual silicon capability rather than a driver version. For MIG entries the bus id is the parent physical GPU's, which is the correct scope — the decompression engine is a property of the physical device.Notes on the implementation:
dlopen("libcuda.so.1")+dlsymrather than a link dependency, so the library still loads on hosts without an NVIDIA driver, matching how NVML is already treated here.CUdevice/CUresultare spelled asintto avoid pulling in<cuda.h>; both match the driver ABI.cudaDevAttrMemDecompress*equivalent, so the driver API is the only route.This removes the only RMM use in
topology_discovery.cpp(its sole source file), sormm::rmmis dropped from the three topology targets and${CMAKE_DL_LIBS}added. Side effect:CUCASCADE_TOPOLOGY_ONLY=ONnow configures and builds — it never callsfind_package(rmm), so linkingrmm::rmmhad it failing at generate time.Behavior change
hw_decompression_availablenow reportsfalseon pre-Blackwell GPUs that previously reportedtrueon any >= 12.8 driver. The old code answered a different question. The field has no in-tree readers as of this branch, but downstream consumers relying on the permissive answer should be checked.Verification
On a 2x RTX 6000 Ada host:
-Wall -Wextra -Wpedantic -Wconversionwith warnings-as-errors;CUCASCADE_TOPOLOGY_ONLY=ONbuilds clean too.cucascade_topology_discovery_tests: 47 assertions in 6 test cases, all pass.readelf -donlibcucascade_topology_discovery.so:libdl,libstdc++,libgcc_s,libc— nolibcuda, no rmm.CUDA_VISIBLE_DEVICES=1and UUID-form masking:idrenumbers to 0 butpcistays03:00.0and resolves to the correct physical device.hw_decomp=0. Confirmed a true negative rather than a swallowed error via a direct driver probe:attr136_rc=0 mask=0. Ada has no decompression engine; that is Blackwell hardware.🤖 Generated with Claude Code