[PR #704] Make the low-latency timeout trap self-attributing in the log #719

Open
opened 2026-10-27 21:05:33 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/deepseek-ai/DeepEP/pull/704
Author: @KeitaW
Created: 7/27/2026
Status: 🔄 Open

Base: main ← Head: pr/self-attributing-timeout-trap


📝 Commits (1)

  • 86a737c 719 self-attribution: make the DeepEP timeout trap name itself in the log

📊 Changes

1 file changed (+34 additions, -5 deletions)

View changed files

📝 csrc/kernels/legacy/internode_ll.cu (+34 -5)

📄 Description

Problem

CUDA 719 (CUDA_ERROR_LAUNCH_FAILED) is sticky: once raised on a context, every later launch on that context reports the same code. The three low-latency timeout sites in csrc/kernels/legacy/internode_ll.cu call trap() when mask_buffer_ptr == nullptr — that trap() is the origin of the 719 — but they print no line number and no reason, so the failure is attributed to whichever library happens to make the next launch-API call.

On a 32-rank EP32 cell the same-second 719 split 25 / 7 between DeepGEMM's runtime_utils.hpp:143 and this file's own launch site. Both are victims; neither is the cause. Recovering the real origin took a separate CUDA_LAUNCH_BLOCKING=1 run — the log alone could not have told anyone.

The fatal branch is not hypothetical. On a vLLM build that does not pass enable_shrink when constructing the low-latency Buffer, buffer.py:42 defaults it to False, so deep_ep.cpp:386 never allocates the mask buffer and mask_buffer_ptr is nullptr — every one of these three sites then traps rather than masks. That is the configuration this was measured on. Current vLLM main does wire it up (all2all.py:282-284 reads parallel_config.enable_fault_tolerance into enable_shrink at all2all.py:326), so on main the branch is reachable whenever fault tolerance is left off, which is the default. Either way the trap is live and says nothing about where it came from.

Change

Diagnostics only — no behaviour change. Each of the three sites (barrier, dispatch-receive, combine-receive) additionally prints:

  • the resolved source line via __LINE__
  • waited vs limit cycles
  • the mask_buffer pointer value
  • an explicit FATAL-WILL-TRAP(origin of CUDA 719) vs masked-survivable marker, selected on the same condition that decides whether trap() fires

The if (mask_buffer_ptr == nullptr) trap(); lines themselves are untouched.

The existing Warning: DeepEP timeout for ... prefix is preserved verbatim so downstream log scanners keep matching. A silently blinded scanner would turn a real timeout into a false PASS, so that property is verified rather than assumed (below).

Verification

  • nvcc -arch=sm_90, real H100. The vararg lists type-check on the device path, below-threshold iterations print nothing, and trap() still yields exactly 719 when armed — fatal semantics unchanged.
  • Log-scanner regression. Replayed the real device-printf bytes against a scanning regex: 7/7 patched lines match, 3/3 pre-patch lines match, no regressions.
  • Reachability. Confirmed against the extracted build under test that mask_buffer_ptr and enable_shrink are both reachable, i.e. the patched branch is not dead code behind a hardcoded-false flag.

Why legacy/ and not the new backend

csrc/kernels/legacy/ is currently the only DeepEP path vLLM builds against: all2all.py:241,317 construct deep_ep.Buffer (deep_ep/buffers/legacy.py), and ElasticBuffer is never referenced. So this patch targets the path in production use today; it is not an argument against the direction of travel.

To be clear about what this does not claim: the newer trees reach trap() too, via deep_ep/include/deep_ep/common/comm.cuh:46 → ptx.cuh:21-23, so this is not a problem that only exists in legacy/. The newer path is in better shape for it, though — comm::timeout_while (comm.cuh:30-49) already waits ~1 s before trapping so other threads can flush, and its timeout printfs already carry rank/thread/status detail (e.g. dispatch.cuh:140, comm.cuh:122). What legacy/ is missing is exactly that: the source line and the reason, which is what this patch adds.

Relationship to #480 and #539

#480 reports that critical timeout logs go missing. #539 adds a delay before trap() so other warps can flush their printf buffers.

This change is complementary and non-overlapping: #539 keeps messages from being lost; this one makes the message that does survive say where it came from and why it is fatal. Different files (utils.cuh/configs.cuh vs internode_ll.cu), and #539 changes trap()'s behaviour while this is diagnostics-only. Both are needed — with #539 alone a flushed message still carries no line number, and with this alone the message can still be lost before it reaches the host.


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/deepseek-ai/DeepEP/pull/704 **Author:** [@KeitaW](https://github.com/KeitaW) **Created:** 7/27/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `pr/self-attributing-timeout-trap` --- ### 📝 Commits (1) - [`86a737c`](https://github.com/deepseek-ai/DeepEP/commit/86a737c963c786d9608580d425122ebd0e88e6f5) 719 self-attribution: make the DeepEP timeout trap name itself in the log ### 📊 Changes **1 file changed** (+34 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `csrc/kernels/legacy/internode_ll.cu` (+34 -5) </details> ### 📄 Description ### Problem CUDA 719 (`CUDA_ERROR_LAUNCH_FAILED`) is sticky: once raised on a context, every later launch on that context reports the same code. The three low-latency timeout sites in `csrc/kernels/legacy/internode_ll.cu` call `trap()` when `mask_buffer_ptr == nullptr` — that `trap()` is the **origin** of the 719 — but they print no line number and no reason, so the failure is attributed to whichever library happens to make the next launch-API call. On a 32-rank EP32 cell the same-second 719 split **25 / 7** between DeepGEMM's `runtime_utils.hpp:143` and this file's own launch site. Both are victims; neither is the cause. Recovering the real origin took a separate `CUDA_LAUNCH_BLOCKING=1` run — the log alone could not have told anyone. The fatal branch is not hypothetical. On a vLLM build that does not pass `enable_shrink` when constructing the low-latency `Buffer`, `buffer.py:42` defaults it to `False`, so `deep_ep.cpp:386` never allocates the mask buffer and `mask_buffer_ptr` is `nullptr` — every one of these three sites then traps rather than masks. That is the configuration this was measured on. Current vLLM main does wire it up (`all2all.py:282-284` reads `parallel_config.enable_fault_tolerance` into `enable_shrink` at `all2all.py:326`), so on main the branch is reachable whenever fault tolerance is left off, which is the default. Either way the trap is live and says nothing about where it came from. ### Change Diagnostics only — no behaviour change. Each of the three sites (barrier, dispatch-receive, combine-receive) additionally prints: - the resolved source line via `__LINE__` - waited vs limit cycles - the `mask_buffer` pointer value - an explicit `FATAL-WILL-TRAP(origin of CUDA 719)` vs `masked-survivable` marker, selected on **the same condition that decides whether `trap()` fires** The `if (mask_buffer_ptr == nullptr) trap();` lines themselves are untouched. The existing `Warning: DeepEP timeout for ...` prefix is preserved verbatim so downstream log scanners keep matching. A silently blinded scanner would turn a real timeout into a false PASS, so that property is verified rather than assumed (below). ### Verification - **`nvcc -arch=sm_90`, real H100.** The vararg lists type-check on the device path, below-threshold iterations print nothing, and `trap()` still yields exactly 719 when armed — fatal semantics unchanged. - **Log-scanner regression.** Replayed the real device-`printf` bytes against a scanning regex: 7/7 patched lines match, 3/3 pre-patch lines match, no regressions. - **Reachability.** Confirmed against the extracted build under test that `mask_buffer_ptr` and `enable_shrink` are both reachable, i.e. the patched branch is not dead code behind a hardcoded-false flag. ### Why `legacy/` and not the new backend `csrc/kernels/legacy/` is currently the only DeepEP path vLLM builds against: `all2all.py:241,317` construct `deep_ep.Buffer` (`deep_ep/buffers/legacy.py`), and `ElasticBuffer` is never referenced. So this patch targets the path in production use today; it is not an argument against the direction of travel. To be clear about what this does *not* claim: the newer trees reach `trap()` too, via `deep_ep/include/deep_ep/common/comm.cuh:46` → `ptx.cuh:21-23`, so this is not a problem that only exists in `legacy/`. The newer path is in better shape for it, though — `comm::timeout_while` (`comm.cuh:30-49`) already waits ~1 s before trapping so other threads can flush, and its timeout `printf`s already carry rank/thread/status detail (e.g. `dispatch.cuh:140`, `comm.cuh:122`). What `legacy/` is missing is exactly that: the source line and the reason, which is what this patch adds. ### Relationship to #480 and #539 #480 reports that critical timeout logs go missing. #539 adds a delay before `trap()` so other warps can flush their `printf` buffers. This change is **complementary and non-overlapping**: #539 keeps messages from being *lost*; this one makes the message that does survive say *where it came from and why it is fatal*. Different files (`utils.cuh`/`configs.cuh` vs `internode_ll.cu`), and #539 changes `trap()`'s behaviour while this is diagnostics-only. Both are needed — with #539 alone a flushed message still carries no line number, and with this alone the message can still be lost before it reaches the host. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
Sign in to join this conversation.
No labels
pull-request
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
deepseek-ai/DeepEP#719
No description provided.