[TransferEngine] Skip the CUDA pointer probe on GPU-less hosts#2955
[TransferEngine] Skip the CUDA pointer probe on GPU-less hosts#2955he-yufeng wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request optimizes memory location detection on GPU-less hosts by checking for the presence of a GPU device once, preventing repetitive and noisy error logs from cudaPointerGetAttributes. The reviewer suggested renaming cuda_device_present and the corresponding warning log to use more generic "GPU" terminology, as this code block is compiled for multiple GPU backends (e.g., MUSA, HIP, MLU).
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| static const bool cuda_device_present = [] { | ||
| int device_count = 0; | ||
| if (cudaGetDeviceCount(&device_count) != cudaSuccess || | ||
| device_count == 0) { | ||
| LOG(WARNING) << "No CUDA device detected; treating buffers as " | ||
| "host memory"; | ||
| return false; | ||
| } | ||
| return true; | ||
| }(); | ||
|
|
||
| if (cuda_device_present) { |
There was a problem hiding this comment.
Since this block is compiled for multiple GPU backends (including MUSA, HIP, MLU, MACA, etc., as seen in the #if guard on line 38), referring specifically to "CUDA" in the variable name and warning message can be confusing to users and developers on non-CUDA platforms.
Consider using more generic terms like gpu_device_present and "GPU" in the log message to make the code and diagnostics vendor-agnostic.
static const bool gpu_device_present = [] {
int device_count = 0;
if (cudaGetDeviceCount(&device_count) != cudaSuccess ||
device_count == 0) {
LOG(WARNING) << "No GPU device detected; treating buffers as "
"host memory";
return false;
}
return true;
}();
if (gpu_device_present) {getMemoryLocation() probes cudaPointerGetAttributes for every registered buffer in a CUDA-enabled build. On a GPU-less host (e.g. an RDMA-only real-client sidecar) the call fails with no device and logs an ERROR per buffer, drowning the real signal, before falling back to the host path. Detect device presence once via cudaGetDeviceCount and skip the probe entirely when no device exists, logging a single WARNING. Hosts with a GPU are unaffected. Refs kvcache-ai#2937 Signed-off-by: Yufeng He <[email protected]>
f91d626 to
46cc4f6
Compare
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| static const bool cuda_device_present = [] { | ||
| int device_count = 0; | ||
| if (cudaGetDeviceCount(&device_count) != cudaSuccess || | ||
| device_count == 0) { | ||
| LOG(WARNING) << "No CUDA device detected; treating buffers as " | ||
| "host memory"; | ||
| return false; | ||
| } | ||
| return true; | ||
| }(); |
There was a problem hiding this comment.
You can extract the lambda function to make coding style simple.
Description
getMemoryLocation()probescudaPointerGetAttributesfor every registered buffer in a CUDA-enabled build. On a GPU-less host — e.g. the RDMA-onlymooncake_clientreal-client sidecar in the deployment from #2937 — that call fails with "no CUDA-capable device is detected" and logs anERRORfor every buffer before falling back to the host/NUMA path. Registration still succeeds, but the per-bufferERRORbecomes the dominant line in the sidecar's stderr and buries the real problem (symptom 1 in #2937).This detects device presence once via
cudaGetDeviceCountand skips the probe entirely when no device exists, emitting a singleWARNINGinstead. Hosts with a GPU keep the exact previous behavior. This addresses only symptom 1 (the diagnostic noise); theclient_expiredliveness behavior (symptoms 2–3) needs the full deployment and is out of scope here.cudaGetDeviceCountis already aliased in everygpu_vendor/*.hshim (musaGetDeviceCount,mcGetDeviceCount,tangGetDeviceCount, the MLU inline, …), so the change compiles for allUSE_*GPU backends, not justUSE_CUDA.Addresses #2937.
Module
mooncake-transfer-engine)Type of Change
How Has This Been Tested?
I could not build the full stack on this host (Linux-only deps), so I verified the new control flow with a standalone harness that mirrors the patched branch with fake CUDA-alike calls and a call counter:
So a GPU-less host no longer calls
cudaPointerGetAttributesper buffer (0 calls, oneWARNING) and still classifies buffers via the host/NUMA path, while a host with a device keeps probing every buffer unchanged. Portability was checked by grepping eachgpu_vendorheader for thecudaGetDeviceCountalias. Full compile/tests rely on CI.Test results:
Checklist
./scripts/code_format.sh— clang-format was not available on this host; I hand-matched the repo.clang-format(Google, 80 col) and CI will verifypre-commit run --all-filesand all hooks pass — toolchain unavailable locally; relying on CIAI Assistance Disclosure
Claude Code helped analyze the issue, draft the change, and build the standalone verification harness above. The change is small and was reviewed line by line: the root cause matches @Icedcoco's analysis on #2937,
cudaGetDeviceCountportability was checked against eachgpu_vendorheader, and the branch behavior was confirmed with the harness.