Skip to content

[sglang] No-fabric DeepEP EP: port the NVL CUDA-IPC path to the SGLang integration#1

Draft
rchalamala wants to merge 11 commits into
mainfrom
devin/1784752573-sglang-nofabric-ep-ipc
Draft

[sglang] No-fabric DeepEP EP: port the NVL CUDA-IPC path to the SGLang integration#1
rchalamala wants to merge 11 commits into
mainfrom
devin/1784752573-sglang-nofabric-ep-ipc

Conversation

@rchalamala

@rchalamala rchalamala commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

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/cuIpcOpenMemHandle on VMM-backed allocations emit a versioned VMM2 blob (exporter pid, ptr, size, token, whole-chunk metadata, exporter rank slot + region base); the actual allocation FD travels via SCM_RIGHTS over a per-process abstract Unix socket (UID-checked, token-validated against pid reuse).
  • Whole-chunk export/import for LOAD allocations carved from preallocated VMM regions.
  • Deterministic imports: ranked exporters (FOUNDRY_RANK/RANK/LOCAL_RANK) carry their slot + region base in the blob; the importer reserves one process-wide import zone (base 0x300000000000, fallback 0x500000000000; 2 slots × 512 GiB) and maps each peer allocation at zone + 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() wraps deep_ep.Buffer.__init__: with FOUNDRY_DEEPEP_NVL_IPC=1 it sets use_fabric=False and preserves SGLang's computed num_nvl_bytes (NVL buffer goes through the CUDA-IPC/VMM path); default Foundry mode keeps use_fabric=True, num_nvl_bytes=0. Requires DeepEP 29d31c0+ (adds use_fabric); fails loudly at Buffer construction 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)

  • Standalone DeepEP matrix incl. nvl_ipc (64 MB NVL buffer, use_fabric=False): SAVE→LOAD→replay correct.
  • Single-GPU SGLang Foundry LOAD: token-identical to baseline (39 s vs 155 s cold start).
  • EP=2 SGLang (Qwen3-30B-A3B-FP8, tp=dp=ep=2, dp-attention, DeepEP low-latency, deep_gemm): SAVE token-correct; LOAD restores 20 graphs in ~2 s with no MMU fault and deterministic peer import mappings identical to SAVE.

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:

  • Same-process graph reload: correct. Fresh-process eager (no restored graphs): correct. Fresh-process restored-graph replay: corrupt.
  • All replay inputs (DecodeInputBuffers, FA3 metadata, page table, positions/seq-lens, token counts) verified byte-identical SAVE↔LOAD at the first decode step; graph replay executes (sentinel test).
  • Ruled out: NCCL capture (removed via custom all-reduce), nondeterministic peer VAs (fixed by this PR), live-vs-duplicate module handles, DeepEP LL phase counters, weight drift, LL buffer canonicalization (clean_low_latency_buffer on both sides did not fix it).
  • Remaining suspects: fresh-process device-global state consumed inside the restored graphs (DeepEP/NVSHMEM device state, DeepGEMM init) that SAVE's capture phase establishes.

Link to Devin session: https://modal.devinenterprise.com/sessions/2d49ddd0e8d84e7bbed75d325c632369
Requested by: @rchalamala

xenshinu and others added 5 commits July 22, 2026 20:36
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>
@rchalamala rchalamala self-assigned this Jul 22, 2026
@devin-ai-integration

Copy link
Copy Markdown

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

devin-ai-integration[bot]

This comment was marked as resolved.

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-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 new potential issues.

Open in Devin Review

Comment thread csrc/hook.cpp
Comment on lines +3301 to +3306
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;

@devin-ai-integration devin-ai-integration Bot Jul 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread python/foundry/integration/sglang/hooks.py
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>
devin-ai-integration[bot]

This comment was marked as resolved.

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-integration[bot]

This comment was marked as resolved.

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-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 0 new potential issues.

Open in Devin Review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants