feat(comm): host-pinned GDRCopy fallback for hosts without gdrdrv#565
feat(comm): host-pinned GDRCopy fallback for hosts without gdrdrv#565n-WN wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f09607583
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return Err(CudaError::GdrCopyError("Failed to create GDR copy handle")); | ||
| // gdrdrv kernel module not loaded — fall back to host-pinned mapped | ||
| // memory rather than failing. Only control-flag latency degrades. | ||
| log::warn!( |
There was a problem hiding this comment.
Add the missing log dependency before using log::warn!
When the hw-cuda feature builds openinfer-comm-cuda-lib, this new log::warn! reference is compiled in, but openinfer-comm/crates/openinfer-comm-cuda-lib/Cargo.toml does not declare log as a dependency (only the parent openinfer-comm crate does). That makes the CUDA comm crate fail to compile with an unresolved log crate before the fallback can be used.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — this is correct and valid. log was only declared on the parent openinfer-comm crate, and the gdr module is #[cfg(feature = "hw-cuda")]-gated, so my earlier compile check (which didn't enable hw-cuda) never actually compiled this path. Added log = { workspace = true } to openinfer-comm-cuda-lib's deps and re-verified with cargo build -p openinfer-comm-cuda-lib --features hw-cuda (clean). Amended into the commit.
d46439d to
4f67775
Compare
GdrCopyContext::new() falls back to cuMemHostAlloc(DEVICEMAP) + cuMemHostGetDevicePointer when gdr_open() returns null (gdrdrv kernel module not loaded and no CAP_SYS_MODULE to insmod it). Mirrors GDRCopy — maps host-pinned memory into the GPU's address space so the a2a kernel spins on it over PCIe while the proxy pokes it locally. Higher small-message latency, identical semantics; the RDMA data plane (ibv_reg_mr/dma-buf) is untouched. No feature flag, no build change. Lets P/D a2a benchmarks run without root kernel access. Compile-verified on an H100 host.
4f67775 to
d809216
Compare
What
GdrCopyContext::new()falls back tocuMemHostAlloc(DEVICEMAP)+cuMemHostGetDevicePointerwhengdr_open()returns null — i.e. thegdrdrvkernel module isn't loaded and the process lacksCAP_SYS_MODULEtoinsmodit.Why
On hosts without the
gdrdrvmodule,GdrCopyContext::new()currently fails atgdr.rs("Failed to create GDR copy handle"), which hard-blocks the a2a worker (a2a_worker.rs:136) and any P/D benchmarking.gdrdrvis only used for the control-plane flag signalling (GdrFlag/GdrVec) — the RDMA data plane is already GDR-independent (ibv_reg_mr/dma-buf). So a host-pinned mirror is enough to keep a2a running.How
The fallback mirrors GDRCopy in reverse: instead of mapping GPU BAR into CPU space, it maps host-pinned memory into the GPU's address space. The a2a kernel spins on the device pointer (over PCIe), the proxy thread pokes the same bytes locally. Higher small-message latency, identical semantics; the RDMA data path is untouched.
gdrdrvis absent.GdrBufferbecomes an enum (Gdr/HostPinned); all methods branch.GdrCopyContext::is_host_pinned_fallback()lets callers log/telemetry the degraded mode.Testing
Compile-verified on an H100 host where
gdrdrvis not loadable. The RDMA a2a bench itself needs multi-GPU orchestration and is a follow-up; this PR unblocks running it at all on such hosts.