[sglang] No-fabric DeepEP EP: port the NVL CUDA-IPC path to the SGLang integration#1
[sglang] No-fabric DeepEP EP: port the NVL CUDA-IPC path to the SGLang integration#1rchalamala wants to merge 11 commits into
Conversation
Signed-off-by: xenshinu <liuxs@umich.edu>
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
| if (it != vmm_ipc_chunk_mappings.end()) { | ||
| it->second.refcount++; | ||
| CUdeviceptr mapped = it->second.local_base + (original_ptr - (CUdeviceptr)chunk_base); | ||
| vmm_ipc_chunk_subptrs[mapped] = {exporter_pid, chunk_base}; | ||
| *pdptr = mapped; | ||
| return CUDA_SUCCESS; |
There was a problem hiding this comment.
🟡 Peer memory buffer stays mapped forever if the same peer address is shared more than once
When the same interior peer pointer is imported more than once (vmm_ipc_chunk_subptrs[mapped] at csrc/hook.cpp:3304), the shared-buffer use counter is bumped each time but only one lookup entry is kept, so releasing it later frees the mapping one time too few and the peer buffer is never torn down.
Impact: In the rare case where a peer buffer at the same address is opened twice, that GPU memory mapping leaks for the lifetime of the process.
Refcount vs. single-key tracking asymmetry in chunk import/close
Both the fast path (csrc/hook.cpp:3301-3306) and the slow path (csrc/hook.cpp:3461-3466) do refcount++ on every cuIpcOpenMemHandle for a chunk carve, but record the reverse mapping as vmm_ipc_chunk_subptrs[interior] = key. If the identical interior VA (same exporter_pid, chunk_base, and original_ptr) is opened N times, refcount becomes N while vmm_ipc_chunk_subptrs holds a single entry. On cuIpcCloseMemHandle (csrc/hook.cpp:3509-3514) the first close erases the subptr entry and decrements refcount to N-1; subsequent closes with the same dptr no longer find it in vmm_ipc_chunk_subptrs, fall through to global_alloc_metadata (absent) and finally real_func, so refcount never reaches 0 and the whole-chunk mapping (local_base, imported handle, reservation) is never released. Reachability is low because DeepEP typically opens each peer buffer exactly once, hence non-severe.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Confirmed the asymmetry exists as described: refcount increments per open while vmm_ipc_chunk_subptrs keeps one entry per interior VA, so N>1 opens of the same interior pointer would leave the chunk mapping alive after N closes. Two mitigating points: (1) this code is the upstream ep-ipc commit (fc5843f) cherry-picked verbatim — keeping it byte-identical to the branch that was validated on vLLM makes it easier to track upstream; (2) DeepEP opens each peer buffer exactly once per rank, so the duplicate-open case isn't reachable in the current usage and the worst case is a bounded per-process mapping leak, not corruption. Leaving as-is in this PR to stay aligned with upstream ep-ipc; happy to add a per-interior open count in a follow-up if we want to diverge.
There was a problem hiding this comment.
Confirmed the asymmetry exists: refcount increments per open while vmm_ipc_chunk_subptrs keeps one entry per interior VA, so duplicate opens of the same interior pointer would leak the chunk mapping. In practice DeepEP and custom-allreduce open each peer buffer exactly once per process, so this isn't reachable today; the clean fix is tracking a per-interior open count. Leaving as-is for this PR unless reviewers want it addressed here — happy to add the per-interior count if so.
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Summary
Ports Foundry's no-fabric DeepEP path to the SGLang integration so EP works on GPUs with CUDA P2P/NVLink but no IMEX/fabric handles (e.g. Modal H200), and makes cross-process VMM-IPC peer imports deterministic so pointers baked into captured graphs stay valid on LOAD.
Core no-fabric transport (
csrc/hook.cpp)cuIpcGetMemHandle/cuIpcOpenMemHandleon VMM-backed allocations emit a versionedVMM2blob (exporter pid, ptr, size, token, whole-chunk metadata, exporter rank slot + region base); the actual allocation FD travels viaSCM_RIGHTSover a per-process abstract Unix socket (UID-checked, token-validated against pid reuse).FOUNDRY_RANK/RANK/LOCAL_RANK) carry their slot + region base in the blob; the importer reserves one process-wide import zone (base0x300000000000, fallback0x500000000000; 2 slots × 512 GiB) and maps each peer allocation atzone + slot*stride + (ptr - exporter_region_base), so SAVE and LOAD processes see identical peer VAs. Legacy/unranked handles keep best-effort placement.SGLang DeepEP policy (
python/foundry/integration/sglang/hooks.py)_patch_deepep()wrapsdeep_ep.Buffer.__init__: withFOUNDRY_DEEPEP_NVL_IPC=1it setsuse_fabric=Falseand preserves SGLang's computednum_nvl_bytes(NVL buffer goes through the CUDA-IPC/VMM path); default Foundry mode keepsuse_fabric=True,num_nvl_bytes=0. Requires DeepEP29d31c0+ (addsuse_fabric); fails loudly atBufferconstruction on older DeepEP.Recipes/docs: experimental 2×H200 EP recipe (
recipe/experimental/serve_qwen3-30ba3bfp8_sglang_ipc_ep.sh+ save/load TOMLs) and SGLang integration docs.Validation (Modal
rahul-dev, H200:2, no fabric/IMEX)nvl_ipc(64 MB NVL buffer,use_fabric=False): SAVE→LOAD→replay correct.Known issue (open, under investigation)
EP=2 LOAD in a fresh process is numerically wrong after the first token (
[12095, 0, 0, ...]vs baseline[12095, 13, 576, ...]). Extensive bisection so far:clean_low_latency_bufferon both sides did not fix it).Link to Devin session: https://modal.devinenterprise.com/sessions/2d49ddd0e8d84e7bbed75d325c632369
Requested by: @rchalamala