Skip to content

[Fix] moe: Avoid stale prefix reads in ep_scatter - #44974

Merged
BBuf merged 2 commits into
sgl-project:mainfrom
Degeneracy-Evil:fix/moe-ep-scatter-prefix-race
Nov 2, 2026
Merged

BBuf merged 2 commits into
sgl-project:mainfrom
Degeneracy-Evil:fix/moe-ep-scatter-prefix-race

Conversation

@Degeneracy-Evil

@Degeneracy-Evil Degeneracy-Evil commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

We hit an illegal memory access while running GLM-5.3-Flash + DFlash2 with TP8 on H20. We traced it to _fwd_kernel_ep_scatter_1.

In this kernel, every program writes the full expert offset table, then reads its own offset back from global memory. The writes and reads are not properly synchronized, so the read can return a stale value.

This PR follows the approach in #31930: compute the current offset directly from the expert token counts, and have each program write only its own entry. This avoids the stale read and removes the repeated full-table writes.

Testing

  • All 4 regression tests passed on H20, covering empty experts, tile boundaries, CUDA Graph replay, and BF16/FP8 scatter.
  • Compute Sanitizer reported no memory errors.
  • With the fix and CUDA Graphs enabled, the serving setup that originally failed completed the 32-request benchmark at concurrency 1 and 32.

To run the tests:

PYTHONPATH=python python test/registered/kernels/ops/moe/test_ep_scatter.py -v

The kernel was faster in isolated tests than the approach in #31930. We did not see a noticeable end-to-end speedup over our earlier correctness-only patch. Full model accuracy evaluation has not been run.

Related: #31929, #31930.


CI States

Latest PR Test (Base): ❌ Run #39146060498
Latest PR Test (Extra): ❌ Run #39146060499
Latest PR Test (AMD ROCm 10): ❌ Run #39146060473

Tip

Base CI only covers the core cases, and its workload may not exercise your change. Use /rerun-test to run the tests your change touches, or copy this prompt for an agent:

Find the registered tests this PR affects and run them with /rerun-test, following https://github.com/sgl-project/sglang/blob/main/.agents/skills/rerun-test/SKILL.md

@Degeneracy-Evil Degeneracy-Evil changed the title [Fix] moe: Avoid stale prefix reads in ep_scatter [Fix] moe: Use single-writer prefixes in ep_scatter Oct 18, 2026
@Degeneracy-Evil Degeneracy-Evil changed the title [Fix] moe: Use single-writer prefixes in ep_scatter [Fix] Avoid stale prefix reads in ep_scatter Nov 1, 2026
@github-actions

This comment has been minimized.

Copy link
Copy Markdown
Contributor Author

@BBuf Could you help review this fix and enable CI? We hit this crash on H20, and the affected code is still unchanged on latest main. Regression tests and memcheck passed locally. Thanks!

@Degeneracy-Evil Degeneracy-Evil changed the title [Fix] Avoid stale prefix reads in ep_scatter [Fix] moe: Avoid stale prefix reads in ep_scatter Nov 1, 2026
@BBuf

BBuf commented Nov 1, 2026

Copy link
Copy Markdown
Collaborator

[BBuf Review Agent] @Degeneracy-Evil The current title already matches the required format, so no title change is needed. The local regression and memcheck results are enough to proceed with CI.

@BBuf BBuf left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[BBuf Review Agent] LGTM. Let's wait for the related kernel CI.

@BBuf

BBuf commented Nov 1, 2026

Copy link
Copy Markdown
Collaborator

/tag-and-rerun-ci

@github-actions github-actions Bot added the run-ci CI: run the baseline test suite on this PR label Nov 1, 2026
@BBuf

BBuf commented Nov 1, 2026

Copy link
Copy Markdown
Collaborator

/rerun-failed-ci

2 similar comments
@BBuf

BBuf commented Nov 1, 2026

Copy link
Copy Markdown
Collaborator

/rerun-failed-ci

@BBuf

BBuf commented Nov 1, 2026

Copy link
Copy Markdown
Collaborator

/rerun-failed-ci

@BBuf

BBuf commented Nov 1, 2026

Copy link
Copy Markdown
Collaborator

[BBuf Review Agent] @Degeneracy-Evil these still fail after a rerun, and I can't tell from the logs whether it's this PR. Could you take a look?

  • base-c-test-4-gpu-gb300 (0): {"file": "test/registered/e2e/models/test_deepseek_v41_flash_gb300_dp4ep4.py", "passed": false, "elapsed": 790}" followed by "##[error]Process completed with exit code 255." The tail contains no asser

Copy link
Copy Markdown
Contributor Author

@BBuf I checked the full GB300 log. The first error is an int32 overflow in _prepare_engram_history, when async_h2d converts the history token IDs to a tensor:

ngram_embedding_manager.py:206 -> async_h2d -> torch.tensor
RuntimeError: value cannot be converted to type int32 without overflow

The multimodal padding values can exceed the int32 range, while this history copy uses torch.int32. Both are already present in the base commit (4ec19ca27); this PR does not change them. The server then exits, causing the remaining MMMU-Pro requests to fail.

The test uses flashinfer_mxfp4 with --moe-a2a-backend none, which does not call the modified ep_scatter path. GSM8K passed at 97.5% before MMMU-Pro hit the overflow. All four test_ep_scatter.py regression tests also passed in CI.

Based on the logs and code path, this appears to be a separate multimodal/Engram issue. I have not reproduced it locally on GB300.

@BBuf

BBuf commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator

[BBuf Review Agent] @Degeneracy-Evil Agreed, this test bypasses ep_scatter, and the Engram history copy is unchanged by this PR. I'll mark this GB300 failure as unrelated.

@BBuf
BBuf merged commit 806d863 into sgl-project:main Nov 2, 2026
608 of 708 checks passed
@Degeneracy-Evil
Degeneracy-Evil deleted the fix/moe-ep-scatter-prefix-race branch November 2, 2026 07:53
Yuxingwang-intel pushed a commit to Yuxingwang-intel/sglang that referenced this pull request Nov 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jit-kernel run-ci CI: run the baseline test suite on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants