Skip to content

[Bugfix] Bound accepted-token state lookups in GDN/KDA spec decode - #50021

Open
amittell wants to merge 6 commits into
vllm-project:mainfrom
amittell:bugfix/gdn-mtp-spec-decode-index-bounds
Open

[Bugfix] Bound accepted-token state lookups in GDN/KDA spec decode#50021
amittell wants to merge 6 commits into
vllm-project:mainfrom
amittell:bugfix/gdn-mtp-spec-decode-index-bounds

Conversation

@amittell

@amittell amittell commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

On a hybrid GDN model (Qwen3.5 / Qwen3.6) with MTP speculative decoding and prefix caching (--mamba-cache-mode align), the engine dies with CUDA error: unspecified launch failure within 7-10 requests of agent-shaped traffic. The GPU faults, not the runtime: the signature is an SM address exception (Xid 13, ESR 0x404000) or an MMU fault (Xid 31), depending on whether the wild address happens to be mapped.

Validation scope (added 2026-08-19). Everything below was developed and validated with
--mamba-cache-mode align and --enforce-eager. That matters: align is the only branch in
which gpu_model_runner still calls num_accepted_tokens_event.synchronize() (the wait is
skipped when use_async_scheduling and mamba_cache_mode != "align"), and eager mode removes
CUDA-graph stream structure. So this PR addresses accepted-count-derived indices that are
correctly synchronized but out of range. It does not address the separate cross-stream
race reported by @noonghunna under async scheduling with the default (non-align) mamba cache
mode, where the accepted-count value itself arrives unsynchronized — in-kernel bounds cannot
repair a value that arrives wrong. See the discussion in this thread; that path needs stream
ordering, not bounds. Note mamba_cache_mode resolves per-model: with prefix caching and no
explicit flag it is "all" (sync skipped) when model_config.supports_mamba_prefix_caching,
else "align" (sync kept).

This is the crash half of the problems reported around hybrid-Mamba + MTP. It is distinct from the prefix-cache corruption in #43559 (the coordinator-level EAGLE cache-peek gating for Mamba), which is already handled on current main. This PR does not touch that path and does not claim to fix #43559; it fixes a separate GPU fault in two kernels downstream.

Root cause: unchecked accepted-count-derived indices

Speculative decoding produces a per-request accepted-token count. Multiple GPU state consumers turn that count into an array index without bounding it, so a count that is stale, zero, or too large indexes outside its tensor and yields a wild address the GPU then dereferences.

Site 1 fused_recurrent_gated_delta_rule_fwd_kernel (fused_recurrent.py):

i_t = tl.load(num_accepted_tokens + i_n).to(tl.int64) - 1        # unbounded
state_idx = tl.load(ssm_state_indices + i_n * stride_indices_seq + i_t)
if state_idx <= 0: return                                        # positive garbage passes
p_h0 = h0 + state_idx * stride_init_state_token                  # dereferenced

i_t (= count - 1) is unbounded against a stride_indices_seq-column tensor. A zero accepted count gives i_t == -1, a read before this request's row (before the tensor for i_n == 0); a stale or too-large count reads past the row. The state_idx <= 0 guard only rejects non-positive values, so an out-of-range read that returns a garbage positive int flows into the address math and faults the SM.

Site 2 _copy_mamba_state_block (mamba_utils.py): the block-table columns derive from the same count and index the per-request block-table row unbounded; the loaded block id becomes state_base_addr + block_id * state_block_stride, which is then read and written.

The two sites fire under different loads. A light decode load exercises only Site 1; a heavy cache-transition load (long prefills, A->B->A prefix reuse) also drives Site 2. Both must be bounded.

The fix

Both loads are masked to the valid range, so an out-of-range index falls into the existing invalid-state path instead of producing an address. No stream-ordering change, no device sync, no measurable throughput cost.

Test plan

RTX 5090 (sm_120), Qwen3.6-27B NVFP4, TP=1, --enforce-eager --max-model-len 16384, MTP 3 (qwen3_5_mtp), prefix caching on, align mode.

build crash (Xid) corruption (A->B->A probe)
stock dies at 7-10 requests, Xid every run 0/N reproduced (already fixed on main)
Site 1 fix alone survives light load; still crashes on the first heavy probe (Xid 31) 0/N reproduced
both fixes 68 heavy probes + a 39-minute soak, 0 new Xid 0/68 reproduced

The A->B->A probe issues an agent-shaped sequence (long prefill, prefix reuse, 20 tool schemas) and reports whether a poisoned prefix reappears; 68/68 returned a clean verdict, which also confirms the out-of-range early-return does not drop a needed state copy. Warm throughput and MTP acceptance length (3.98-4.00 of 4) are unchanged.

Note: a separate, still-open livelock (#49203)

Independently of this crash, the same stack can occasionally hang: engine alive, /v1/models answering, but the in-flight request stuck at 0% GPU util with no Xid. It is rare and timing-variable (seen once, then not across the 68 probes here) and matches open issue #49203. This PR does not address it and does not claim to; the 68/68 clean-verdict result rules out this change as a cause.

Follow-up bounds audit

A follow-up audit expanded the same fail-closed rule to the remaining consumers in this path:

  • Both FLA wrappers now zero rejected output deterministically instead of returning with new_empty storage visible downstream.
  • _causal_conv1d_update_kernel now bounds the accepted-count offset before state address math; invalid active rows produce zero output and leave state unchanged.
  • GPU regressions cover too-small/too-large accepted counts, NULL block IDs, source/destination columns crossing the block-table row, and temporal-bias overflow.

Validation after this follow-up: full pre-commit passed; on an RTX 5090, 186 FLA/causal-conv kernel tests and all 21 fused Mamba postprocess tests passed (207 total).

Kimi K3 KDA expansion

The same accepted-count-derived state selection pattern also existed in Kimi K3 KDA fused recurrent decode, in both the NVIDIA and AMD vendored kernels. This PR now masks that initial state-index load to the request row, zeroes all invalid-count output tokens, preserves state on invalid counts, and releases NVIDIA PDL dependents before the new empty/invalid early returns.

Additional validation on an RTX 5090:

  • Patched KDA invalid-count test: 4/4 passed across NVIDIA and AMD implementations (num_accepted 0 and 4).
  • Existing KDA spec-decode correctness plus the new invalid-count target: 12/12 passed.
  • Negative control at previous PR head e7f66b199 with only the new test added: 4/4 failed on old KDA for NVIDIA and AMD, proving the regression is non-vacuous.

AI assistance disclosure

OpenAI Codex assisted with the follow-up bounds audit, implementation, and regression-test generation. The submitter owns the conclusions and final review.

Mamba2 selective-state expansion

The Mamba2 selective-state-update kernel also derives its initial state-slot lookup from num_accepted_tokens - 1. It already clamps the lower side, but had no upper row bound. This update preserves that zero-count behavior and makes an oversized count fail closed: it writes zero output and returns before reading or writing state.

RTX 5090 red/green regression: a count one past a three-column state row made the prior source select an adjacent row and emit nonzero output; the new kernel passes by emitting zero output and preserving the complete state tensor. Pre-commit passes for both changed files.

Copilot AI review requested due to automatic review settings July 27, 2026 17:29
@amittell
amittell requested a review from njhill as a code owner July 27, 2026 17:29

@claude claude 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.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@mergify mergify Bot added the bug Something isn't working label Jul 27, 2026

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dd1fd0389c

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vllm/v1/worker/mamba_utils.py Outdated
Comment thread vllm/third_party/flash_linear_attention/ops/fused_recurrent.py
Comment thread vllm/third_party/flash_linear_attention/ops/fused_recurrent.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the hybrid GDN + MTP speculative decode path against CUDA illegal memory accesses by bounding two indices derived from num_accepted_tokens, ensuring out-of-range acceptance counts don’t turn into out-of-bounds reads and wild pointer dereferences in downstream Triton kernels.

Changes:

  • Mask the ssm_state_indices load in fused_recurrent_gated_delta_rule_fwd_kernel so invalid num_accepted_tokens values fall into the existing “invalid state” early-return path.
  • Mask block-table column loads in _copy_mamba_state_block to prevent out-of-range accepted-token-derived columns from reading outside a request’s block-table row.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
vllm/v1/worker/mamba_utils.py Adds masked block-table column loads in _copy_mamba_state_block to prevent out-of-range accepted-token-derived indexing from producing illegal addresses during state copies.
vllm/third_party/flash_linear_attention/ops/fused_recurrent.py Masks the initial-state index lookup derived from num_accepted_tokens - 1 to avoid out-of-bounds reads and subsequent invalid state dereferences.
Comments suppressed due to low confidence (3)

vllm/v1/worker/mamba_utils.py:102

  • Same sentinel issue as above: block ID 0 (NULL_BLOCK_ID) should be treated as invalid. Otherwise, an in-range but unallocated src_col can cause reads from the reserved padding block 0.
        src_block_id = tl.load(
            block_table_base + src_col, mask=src_col_ok, other=-1
        ).to(tl.int64)
        if src_block_id < 0:
            return

vllm/v1/worker/mamba_utils.py:129

  • Same sentinel issue as above: block ID 0 (NULL_BLOCK_ID) should be treated as invalid to avoid copying from the reserved padding block.
        src_block_id = tl.load(
            block_table_base + src_col, mask=src_col_ok, other=-1
        ).to(tl.int64)
        if src_block_id < 0:
            return

vllm/v1/worker/mamba_utils.py:150

  • Same sentinel issue as above in the temporal-state path: block ID 0 (NULL_BLOCK_ID) should be treated as invalid. Otherwise an in-range but unallocated tmp_col can copy from the reserved padding block.
    actual_src_block_id = tl.load(
        block_table_base + tmp_col, mask=tmp_col_ok, other=-1
    ).to(tl.int64)
    if actual_src_block_id < 0:
        return

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread vllm/v1/worker/mamba_utils.py Outdated
Comment thread vllm/v1/worker/mamba_utils.py Outdated
@amittell

Copy link
Copy Markdown
Contributor Author

Thanks for the review — pushed 09edaaa addressing the substantive findings.

Bound the sigmoid-gating kernel (Codex P1). Correct and important: the Qwen3.5/Qwen3.6 GDN decode path calls fused_sigmoid_gating_delta_rule_update (qwen_gdn_linear_attn.py:1377,1404), not fused_recurrent_gated_delta_rule (that one is the OLMo path). fused_sigmoid_gating.py has the identical unbounded ssm_state_indices load, so the original PR left Qwen's actual kernel unpatched. Applied the same row-bounded mask there.

Reject NULL_BLOCK_ID in the copy (Codex P1 + Copilot). Right on both counts — NULL_BLOCK_ID is 0, not negative. Changed all four block-id guards in _copy_mamba_state_block from < 0 to <= 0, so a stale-but-in-range column landing on an unallocated (0) slot is rejected instead of copying block 0. This also matches the FLA kernels' own state_idx <= 0 convention. Fixed the comment that wrongly called 0 "negative".

Initialize outputs when rejecting (Codex P1). The if state_idx <= 0: return early-exit is pre-existing upstream behavior for the legitimate NULL-state case — the mask only routes out-of-range indices into that same existing path instead of faulting, so the output-write semantics are unchanged from main (the caller already tolerates unwritten positions for NULL states). I left this as-is rather than add output-zeroing, which would change behavior beyond the bug; happy to revisit if a maintainer prefers explicit init.

The masking change was validated end-to-end on an RTX PRO 6000 (SM120, driver 610) with Qwen3.6-27B-NVFP4, MTP-3, prefix caching, fp8 KV, full PIECEWISE cudagraph: 0 GPU faults across a long-prompt sweep to 32k tokens, 0 cache poisoning, 149-158 t/s warm.

Copy link
Copy Markdown

Independent GPU red/green confirmation

I independently exercised the exact current-source bounds changes on RTX 5090/SM120 with Qwen3.6 hybrid GDN + MTP n=3.

Five deterministic GPU regressions failed before the bounded-index changes and passed with all three files mounted:

  • accepted-token row bound in fused recurrent;
  • the corresponding fused sigmoid-gating/GDN bound;
  • Mamba source block-table row bound;
  • destination row bound;
  • temporal-bias source-row bound.

This is independent of the FlashInfer/XQA MTP routing issue in #49010: fixing the attention route restored long-context recall, but it does not remove this wild-address/adjacent-state class. I therefore consider this PR required for a production-safe hybrid-GDN MTP stack, not a workaround for the attention bug.

@amittell

amittell commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

The contributor-side checks and requested GPU evidence are complete. The remaining pre-run-check failure is the repository admission gate: it requires a maintainer-applied verified, ready, or ready-run-all-tests label (the workflow reports my merged-PR count as 1). Could a maintainer please apply the appropriate gate label so the full test workflow can run?

justtestingthingsx pushed a commit to meandmyboiclaude/vllm that referenced this pull request Aug 7, 2026
justtestingthingsx pushed a commit to meandmyboiclaude/vllm that referenced this pull request Aug 7, 2026
…bounded block-table load in the SD-conv branch)
@mergify

mergify Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @amittell.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

…ment stride

The 71171a880 bound compared init_token_idx against stride(1) of the
state-indices tensor -- 1 for a contiguous [batch, T] layout -- so every
accepted count > 1 was wrongly failed closed (zero output, no state
update), and a non-contiguous layout with stride(1) > stride(0) would have
reopened the out-of-bounds lookup. The invalid-count regression could not
catch this: an oversized count fails closed under both the wrong and the
correct bound.

Bound against stride(0) (elements per batch row; == T when contiguous),
matching fused_recurrent.py's i_t < stride_indices_seq pattern, and add
the discriminating valid-side regression: num_accepted=2 in a 3-column row
must produce real output and a state update.

RTX 5090 evidence: the new test fails on 71171a880 exactly as predicted
(all-zero output) and passes here; the invalid-count test passes on both
(demonstrating it was non-discriminating); the full
tests/kernels/mamba/test_mamba_ssm.py file passes 288/288 (18 skipped).

Found by the depthfirst review bot on the PR diff.

Assisted-by: Claude (Anthropic)
Signed-off-by: Alex Mittell <mittell@me.com>
@amittell
amittell force-pushed the bugfix/gdn-mtp-spec-decode-index-bounds branch from a303cf1 to 9a198c0 Compare August 17, 2026 13:04
@mergify mergify Bot removed the needs-rebase label Aug 17, 2026
ch2lab added a commit to ch2lab/vllm that referenced this pull request Aug 18, 2026
…DA state indices

Replaces the local vllm-project#48475-style clamp with the upstream bounds fix
(PR vllm-project#50021, open): masked row-bounded loads for ssm_state_indices and
block-table columns, fail-closed zero output for invalid counts, so
stale/too-large accepted counts can no longer dereference wild state
addresses (Xid 13/31). Clamps kept as defense in depth in FLA kernels.
ch2lab added a commit to ch2lab/vllm that referenced this pull request Aug 18, 2026
…tale zero-accept rows

Builder-level fix (open PR vllm-project#51508): rows whose async-scheduling step was
discarded get their whole spec_state_indices row set to NULL_BLOCK_ID,
so FLA/conv kernels skip them entirely (no initial-state read, no
final-state write) instead of advancing state for a dead request. Kernel
clamps retained as defense in depth (already applied via vllm-project#50021 port).
ch2lab added a commit to ch2lab/vllm that referenced this pull request Aug 18, 2026
Drop the local vllm-project#48475-style clamps in fused_recurrent/fused_sigmoid_gating
so a zero num_accepted_tokens falls through the row-masked load into the
invalid-state path (zeroed output, state untouched) per PR vllm-project#50021, instead
of silently reading slot 0. Adapt the PR vllm-project#51508 defense test that assumed
clamp equivalence, and point the GDN builder fixture at the local
Qwen3.5-0.8B checkpoint for offline runs.
@noonghunna

Copy link
Copy Markdown

We maintain a serving stack for hybrid-GDN Qwen models on consumer GPUs and have multiple field reports of this crash class. We built a fast reproducer and ran a controlled matrix against v0.27.1, including this PR's current head. Three results that seem important:

1. This PR's head does not stop the crash. We applied the five runtime-file changes at 9a198c0 onto vllm/vllm-openai:v0.27.1 (three files are byte-identical to the PR base outside its hunks and were used verbatim; the fused_recurrent.py/mamba_utils.py hunks were ported onto the v0.27.1 copies; all five verified loaded). The engine died twice under the reproducer with the same signature — Xid 31 MMU faults of type VIRT_WRITE, at the identical virtual address both times — including once from a 100% recent-acceptance state, so the zero/stale accepted-count precondition the bounds target is not the (only) faulting path.

2. The crash is gated on async scheduling — it looks like a cross-stream race, not an index bug. Matched single-variable A/B, identical client and config (MTP n=4, --enable-prefix-caching, fp8 KV, TP=2 on 2× RTX 3090, driver 610.57.04):

async scheduling outcome (5-10 min agent-shaped soak, growing multi-turn conversation)
on (default) dies at 6.4k-12.9k generated tokens — 5/5 runs (illegal access surfacing in gdn_attn.py build() / synchronize_input_prep event frames, always on a cache-hit resume step with spec tokens scheduled)
--no-async-scheduling survives to 33k+ generated, full arc incl. a 35k-ctx full-history prefill
on + CUDA_LAUNCH_BLOCKING=1 survives full arc

Drafter-off and prefix-caching-off arms also never fault. Our reading: step N+1's metadata build consumes num_accepted_tokens (device tensor written by step N's rejection sampler) via the boolean gather at gdn_attn.py:326, and h2d's spec_sequence_masks at :207, while overlapped with step N's execution — a stale/garbage value there is dereferenced downstream as a state-block address. In-kernel bounds can't repair a value that arrives wrong.

3. Separately, we found a second, distinct fault while isolating this one — crossing sequence position 32,768 with the MTP drafter active permanently kills draft acceptance engine-wide (reproduces with async scheduling disabled, so it is not this PR's bug). Filed with full evidence and reproducer as #52873.

Reproducer below (stdlib-only; point at an OpenAI-compatible endpoint; min_tokens: 400 standardizes exposure — note min_p-style params are rejected under spec decode but min_tokens is accepted). Happy to run candidate fixes or instrumented builds — this turns "hours of agent traffic" into a ~10-minute red/green, and the position-32768 half into a deterministic boundary test.

Reproducer
#!/usr/bin/env python3
"""Fast reproducer: CUDA illegal memory access (Xid 31) on Qwen3-Next-family
hybrid GDN + MTP spec decode + prefix caching, vLLM v0.27.1.

Shape: a single growing multi-turn conversation (agent-style). Every turn is a
prefix-cache-hit resume with speculative tokens scheduled. On 2x RTX 3090 (TP=2,
fp8 KV, MTP n=4) the engine dies with `gdn_attn.py build -> illegal memory access`
within 6k-13k generated tokens (~5-10 min), reproduced 4/4 runs. With the drafter
off (SPEC_N=0) the same traffic runs indefinitely.

Usage: python3 gdn-mtp-apc-repro.py [base_url] [model]
"""
import json, sys, time, urllib.request

BASE = sys.argv[1] if len(sys.argv) > 1 else "http://localhost:8095"
MODEL = sys.argv[2] if len(sys.argv) > 2 else "qwen3.8-27b"
SYS = ("You are a senior systems engineer pair-programming with the user on a large "
       "C++ and Python codebase for GPU inference. Be concrete and show code. ") * 40

TOPICS = [
    "Refactor this CUDA kernel launcher to support streams. Write the full code.",
    "Design a block-paged KV cache allocator; write the C++ header with comments.",
    "Write a Python asyncio scheduler for batched LLM requests with timeouts.",
    "Explain then implement speculative-decoding token verification in PyTorch.",
    "Write unit tests for a ring-buffer class; include edge cases and comments.",
    "Port this concept to Triton: fused RMSNorm + residual add. Full kernel please.",
    "Debug: our TP=2 all-reduce hangs on PCIe. List hypotheses, then a bisect plan.",
    "Write a bash script that soaks an OpenAI endpoint and logs TPS per request.",
]

def chat(messages, max_tokens=1000):
    req = urllib.request.Request(BASE + "/v1/chat/completions",
        data=json.dumps({"model": MODEL, "messages": messages, "max_tokens": max_tokens,
                         "temperature": 0.7, "top_p": 0.95,
                         "chat_template_kwargs": {"enable_thinking": False}}).encode(),
        headers={"Content-Type": "application/json"})
    with urllib.request.urlopen(req, timeout=600) as r:
        out = json.loads(r.read().decode())
    ch = out["choices"][0]["message"]["content"] or ""
    return ch, out.get("usage", {}).get("completion_tokens", 0)

msgs = [{"role": "system", "content": SYS}]
total, turns = 0, 0
while total < 220_000:
    msgs.append({"role": "user", "content": f"[turn {turns}] {TOPICS[turns % len(TOPICS)]} "
                 f"Focus on a module named engine_{turns} with a {5 + turns % 7}-stage pipeline."})
    try:
        content, comp = chat(msgs)
    except Exception as e:
        print(f"FAILURE after {total} generated tokens: {e}")
        print("check: docker logs <container> | grep -m5 'illegal memory' ; dmesg | grep Xid")
        sys.exit(2)
    if len(content.strip()) < 20:
        msgs.pop(); turns += 1; continue   # keep the history well-formed
    msgs.append({"role": "assistant", "content": content})
    total += comp; turns += 1
    if turns % 10 == 0:
        print(f"generated={total} turns={turns}", flush=True)
print("no failure up to 220k generated tokens")

@amittell

Copy link
Copy Markdown
Contributor Author

Thank you — the matched single-variable A/B plus the CUDA_LAUNCH_BLOCKING=1 arm is what turns "probably a race" into "a race", and I think you are right on all three points. Two things I can add: the exact gate that produces it, and why this PR's head was never going to move your needle.

The accepted-count sync is conditional, and prefix caching turns it off

vllm/v1/worker/gpu_model_runner.py (v0.27.1, ~L2107):

# Sync num_accepted_tokens from CPU (set by
# _update_states_after_model_execute for hybrid models).
# Skipped under async scheduling (non-align): the CPU copy races with
# the in-flight D2H copy and with input-batch row moves.
needs_cpu_accepted_counts = self.num_accepted_tokens_event is not None and not (
    self.use_async_scheduling and self.cache_config.mamba_cache_mode != "align"
)
if needs_cpu_accepted_counts:
    assert self.num_accepted_tokens_event is not None
    self.num_accepted_tokens_event.synchronize()

num_accepted_tokens_event is recorded immediately after num_accepted_tokens.gpu is written (L1586 → L1607 / L1613), and the synchronize() above is the only consumer of it. It is skipped whenever use_async_scheduling and mamba_cache_mode != "align". CacheConfig.mamba_cache_mode defaults to "none" and resolves to "all" — not "align" — once prefix caching is enabled, so async + APC + MTP (your config) lands squarely in the skip branch: the event is recorded and never waited on.

gdn_attn.py build() then consumes that tensor anyway — the boolean gather you flagged at L326, and a non_blocking=True copy into the persistent self.num_accepted_tokens buffer at L460. That is exactly the shape you inferred from the outside, and the comment in that gate is upstream's own: this code already knows there is a race in the area and resolves it by dropping the CPU-side path, not by making the GPU-side read stream-safe. Which is why, as you put it, in-kernel bounds cannot repair a value that arrives wrong. Agreed — and that is a fair criticism of bounds-as-a-fix for your failure.

Falsifiable in one flag: --mamba-cache-mode align

If that gate is the mechanism, align should survive your reproducer with async scheduling left on, because it is the one branch that keeps the synchronize(). That distinguishes "async scheduling is unsafe for hybrid GDN" from "this specific sync gate is unsafe", and it is a single flag on your existing rig.

Our data at that config point, using your reproducer verbatim:

config result
Qwen3.8-27B hybrid GDN, MTP-3, APC, --mamba-cache-mode align, async scheduling auto-on, TP=1, v0.26.0 16,918 generated tokens, 0 faults, engine healthy — straight through the 6.4k–12.9k window where you died 5/5
Same recipe in production (NVFP4, RTX 6000 Blackwell, TP=1, align, async on) no illegal memory access in the unit's journal history; no Xid in dmesg on that host

Caveat I will flag myself: both of our arms are TP=1 on Blackwell, so this is consistent with the gate being the variable, not proof of it — TP=2 rank synchronization is its own timing regime, and your 2×3090 result may well have a second contributing factor. I have a non-align arm (mamba_cache_mode="all", everything else identical, same node class) staging now and will post it either way; if it faults where the align arm did not, the gate is confirmed independently of arch and TP.

Why this PR's head did not help you

The PR was developed and validated entirely in align mode with --enforce-eager — see the repro line and the test plan in the description. Both sidestep the async race: align keeps the sync, and eager mode removes the CUDA-graph stream structure. So it targets a different defect — accepted-count-derived indices that are correctly synchronized but still out of range (zero count → i_t == -1 reads before the row; stale or oversized count reads past it), which is deterministic and reproduces with async scheduling off. Its negative control (test_invalid_block_table_lookup_does_not_copy_state) still fails 4/4 against pristine main for us, so the defect it bounds is live independent of yours.

So I read your result as "there are at least two bugs here", not "the bounds are wrong". That said, your run shows how easily the PR's scope reads as broader than it is, so I will make the description state the align + eager validation scope explicitly rather than leaving align as a parenthetical, and reference this comment for the async/non-align path.

What a fix for your half probably looks like

Not more bounds: the GPU-side consumer needs to be ordered against the producer. Either the metadata build waits on num_accepted_tokens_event unconditionally (the current gate makes the wait conditional on a cache mode that has nothing to do with whether the GPU tensor is safe to read), or the L326/L460 reads move onto the stream that wrote them so the ordering is implicit rather than host-mediated. Happy to test a candidate patch of either shape.

On #52873

The position-32,768 acceptance collapse reproducing with async scheduling disabled does read as independent, and a hard boundary at exactly 2^15 is a strong hint on its own. We have hybrid-GDN capacity across Blackwell (RTX 6000, GB10, 5090) and can run that as a deterministic boundary sweep if a second data point on different silicon would help.

@amittell

Copy link
Copy Markdown
Contributor Author

Correction to my previous comment, and it sharpens the predicate rather than weakening it.

I wrote that mamba_cache_mode "resolves to "all" — not "align" — once prefix caching is enabled". That is wrong as stated. The actual resolution (vllm/model_executor/models/config.py, identical in v0.26.0 and v0.27.1) is per-model:

if cache_config.enable_prefix_caching:
    if cache_config.mamba_cache_mode == "none":
        cache_config.mamba_cache_mode = (
            "all" if model_config.supports_mamba_prefix_caching else "align"
        )

So with prefix caching on and no explicit flag, a model that supports mamba prefix caching gets "all" (→ use_async_scheduling and mamba_cache_mode != "align" is true → num_accepted_tokens_event.synchronize() is skipped), and one that does not gets "align" (→ the sync is kept).

How I caught it: I claimed our fleet was a clean data point at the same config as yours. It isn't. Our Qwen3.8-27B logs

Mamba cache mode is set to 'align' for Qwen3_5ForConditionalGeneration by default
when prefix caching is enabled

i.e. it auto-selects align and lands in the sync-kept branch even though I never passed the flag in that arm. So my "16,918 tokens, 0 faults" run was an align run either way, and both of my arms were the same arm. That number therefore says nothing about non-align; please discount it as an A/B. It remains a data point that async + MTP + APC is stable in the align branch on Blackwell/TP=1, which is all I should have claimed.

The upshot for your report is a tighter exposure predicate than I gave:

async scheduling on and supports_mamba_prefix_caching for the model and MTP/Eagle-class spec decode (so the event exists) and no explicit --mamba-cache-mode align

which fits Qwen3-Next-family + APC landing in "all", and fits --no-async-scheduling and CUDA_LAUNCH_BLOCKING=1 both making it disappear. The mechanism I described — the event being recorded and never waited on, while gdn_attn.py reads that tensor at L326 and non_blocking=True copies it at L460 — is unchanged; only my account of who lands in the skip branch was sloppy.

--mamba-cache-mode align therefore remains the one-flag test/workaround worth trying on your rig, and now with a clear reason it should work: it forces the branch that keeps the synchronize, without giving up async scheduling. The cost is that align caches mamba state only at step boundaries, so on a model that supports "all" you would be trading some prefix-cache reuse for the sync — worth knowing before anyone ships it as a fix rather than a diagnostic.

I have the genuine non-align arm running now (same node, same model, everything identical, but --mamba-cache-mode all forced so it enters the skip branch), and will report it whichever way it falls. If it faults where the align arm did not, that isolates the gate on hardware and a TP setting quite different from yours.

@amittell

Copy link
Copy Markdown
Contributor Author

Non-align arm, as promised — and it did not reproduce, which is evidence against the gate being sufficient on its own. Reporting it because I said I would either way.

Single variable against my earlier align run: same node, same model, same flags, but --mamba-cache-mode all forced so the engine genuinely enters the skip branch. Resolved args confirm it ('mamba_cache_mode': 'all', enable_prefix_caching: True, speculative_config {'method': 'qwen3_5_mtp', 'num_speculative_tokens': 3}), and the engine logged Asynchronous scheduling is enabled., so needs_cpu_accepted_counts is False and num_accepted_tokens_event.synchronize() is never called.

arm resolved mode result
A align (auto-selected) 16,918 generated, 0 faults
B all (forced) → sync skipped 22,034 generated over 108 turns / 1,250 s, 0 faults, engine healthy

So on Qwen3.8-27B, TP=1, single Blackwell (GB10), v0.26.0, MTP-3, APC, your reproducer verbatim: the missing synchronize is not sufficient to produce the fault. That is a real strike against my "this gate is the mechanism" framing, and you should weight it accordingly.

What I think survives, and what does not:

  • Survives: the unsynchronized read itself is not in doubt — the event is recorded and, in this branch, never waited on, while gdn_attn.py consumes that tensor at L326 and non_blocking=True copies it at L460. That is a genuine ordering hazard in the code regardless of whether it fires here.
  • Does not survive: my implication that entering the skip branch is what turns it into your Xid. Something else is required, and the most structurally different variable left is TP=2. Rank synchronization inserts collectives into exactly the window where step N's sampler write and step N+1's metadata build overlap, which changes both the timing and which stream the write retires on. The other unexamined deltas are v0.27.1 vs v0.26.0, Ampere vs Blackwell, MTP n=4 vs n=3, and fp8 KV vs default.

Given that, the --mamba-cache-mode align test on your rig is now more informative than before, not less — it discriminates cleanly:

  • if align fixes it on 2×3090/TP=2, the gate is load-bearing and my negative result just means TP=1/Blackwell doesn't lose the race;
  • if align does not fix it, the gate is a red herring and the fault is somewhere else in the overlap — worth knowing before anyone spends time on stream ordering there.

Honest limits on our side: we cannot currently match your rig. Our Blackwell pair is both GPUs in production, and the GB10s are one GPU per node, so a TP=2 arm would need multi-node Ray rather than a same-box tensor-parallel split — not a like-for-like substitute for two 3090s on one host. If you can run the one-flag align arm, that is the cheapest discriminator available to either of us. If it would help, I can also rerun arm B under v0.27.1 to remove the version delta, since that is the one variable I can change cheaply.

@noonghunna

Copy link
Copy Markdown

This is the missing piece, and it resolves your arm B cleanly: our config runs align, not the skip branch. Every one of our boots logs

Mamba cache mode is set to 'align' for Qwen3_5ForConditionalGeneration by default when prefix caching is enabled

Qwen3.8-27B reports supports_mamba_prefix_caching = False, so it auto-selects align and lands in the sync-kept branch — needs_cpu_accepted_counts is True and num_accepted_tokens_event.synchronize() does run for us. So we were never in your arm-B path, and your "missing sync is not sufficient" result and our crash are not in tension: they're different branches.

Which means, for our failure, align is not a fix — we crash in align, with the synchronize present. The one-flag test you proposed is effectively already run: we're in align by default and we die 5/5. So the predicate isn't "the sync is skipped"; for us it's "the sync runs too late."

Where "too late" is

In align, the synchronize lives inside _prepare_inputs (~L2116), and _prepare_inputs's own block-table copies to GPU (L2136 num_accepted_tokens, L2267 num_decode_draft_tokens) come after it — so those are already ordered. The thing that is not ordered is _update_states(scheduler_output), which runs earlier in the same synchronize_input_prep block, before that synchronize. Under async scheduling it mutates the persistent batch / block tables while the previous step's postprocess_mamba_align_gpu is still in flight reading them — a write-after-read on the state buffers, upstream of the L326 gather you already flagged.

The fix we shipped, and what it buys

Two lines: hoist the wait to the top of synchronize_input_prep, before _update_states:

    @contextmanager
    def synchronize_input_prep(self):
        if self.prepare_inputs_event is None:
            yield
            return
        self.prepare_inputs_event.synchronize()
        # order this step's _update_states/_prepare_inputs after the previous
        # step's spec-decode postprocess, which records this event
        if self.num_accepted_tokens_event is not None:
            self.num_accepted_tokens_event.synchronize()
        try:
            yield
        finally:
            self.prepare_inputs_event.record()

This is your "wait unconditionally" shape, with the extra point that in align the wait already exists — it's the ordering relative to _update_states that's wrong, not the presence of the wait. It relocates a synchronize align already performed a few lines later, so it's TPS-neutral by construction (no net GPU stall), and it's a no-op without spec decode (event is None) and without async scheduling (the method early-returns).

Result on the rig you can't reproduce on (2×3090, TP=2, Ampere, v0.27.1, MTP n=4, fp8 KV, APC, async on): 3/3 boots clean to 33–36k generated tokens each, straight through the 6.4–13k window where we died 5/5 unpatched, across the ctx-32k crossing and a compaction; bench 79.1 narr / 108.5 code / 1756 prefill tok/s, i.e. ≥ baseline. We've shipped it as a downstream install-script overlay on our stack.

Honest limits

We have not isolated TP=2 as a necessary co-factor — the early-sync fix closes the window regardless of why the window is wide enough to fire, so "align-late-sync race" and "TP=2 timing widens it" are not separated by our data. Your arm B says the skip branch alone doesn't fault on TP=1/Blackwell; our result says the align branch does fault on TP=2/Ampere and the early hoist fixes it. Both can be true. Update — we tried the TP=1 arm to isolate the collective, and it's memory-blocked on our hardware. The 18.2 GiB AutoRound-INT4 weights plus fixed cudagraph/GDN-state scratch don't fit one 24 GB Ampere card: OOM at an identical 2.37 GiB shortfall across --max-model-len 132K / 100K / 75K, and unchanged with --max-num-batched-tokens cut 8192→2048 — so it's neither context- nor profiling-batch-driven, it's a fixed allocation. The only lever that fits is --enforce-eager, which removes the CUDA-graph stream structure and therefore suppresses the race (same as CUDA_LAUNCH_BLOCKING=1), so any TP=1 boot we can produce gives an uninterpretable no-crash. We can't generate a clean TP=1-with-cudagraphs point here, so your arm B (TP=1 + cudagraphs + Blackwell, no fault) stands as the reference TP=1 data point — which reads as evidence that TP=2 (or Ampere) is the co-factor rather than the align late-sync alone. Still glad to test any candidate patch of this shape you'd prefer for the PR.

On #52873

Yes please to the Blackwell boundary sweep — a second-silicon data point is exactly what that one needs. Note the boundary is not a fixed position: three async-on boots here collapsed at ctx ~21,025 while an async-off boot collapsed at ~32,570, reproducible within a config but shifting with it (details on #52873). So a sweep that brackets a fixed 2^15 may miss it; varying the config (async on/off, TP, depth) and watching where the per-window acceptance flips to 0 is the more informative shape.

Copy link
Copy Markdown

Follow-up stress validation with the accepted-token bounds applied

Following my earlier red/green confirmation, I kept the relevant #50021 hunks in the final MTP-3 integration stack and ran a materially larger async/concurrent workload. This is additional soak evidence, not a new isolated A/B.

Environment: RTX 5090, Qwen3.8-27B NVFP4, NVFP4 KV, GDN/MTP-3, async scheduling, up to eight sequences.

  • Two 900-request structured hammers: zero HTTP/FSM/grammar/server errors; approximately 84% MTP acceptance
  • Raw c8 decode: 744.1 tok/s
  • Forced eight-active replay: 857.4 tok/s, 71.9% acceptance, zero errors
  • Real parent + seven-child soak: 58 requests, 1.46M prompt tokens, 33.2K generated tokens, 71.1% acceptance
  • KV peaked at 81%; no accepted-token state OOB, CUDA fault, corruption signature, or preemption was observed

The longer soak adds coverage for mixed prompt lengths, async structured output, prefix caching, and divergent agent histories on top of the original targeted red/green result.

Disclosure: AI-assisted analysis and comment posting; the runs and measurements were produced and verified by me on the hardware described.

justtestingthingsx pushed a commit to meandmyboiclaude/vllm that referenced this pull request Aug 22, 2026
…re-pads an already-padded layer to the shared page (boot AssertionError class); commensurate per-token-bytes zero guard; AR speculator draft-prefill dispatches on num_tokens_padded (restores upstream vllm-project#47352, reverted by carried vllm-project#48244 pick); gemma4_dspark + laguna_dflash fused-KV dtype derived from norm weight + bias cast (fp16 drafters); unpadded() propagates mm_req_doc_ranges; V1 runner zeroes padded-row num_prompt_tokens_cpu (stale-length dummy-row class); fused_recurrent + fused_sigmoid_gating INPLACE_FINAL_STATE load masked to the row (SM-fault class, mirrors vllm-project#50021's sibling bound); profile_cudagraph_memory empty-sample guard; TQ spec head_size_v; packed-codec (kvarn_/turboquant_) branch in _validate_cache_dtype logging; max_page_block_lcm includes padded-to-max layers (PR vllm-project#52804 intent)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@amittell

Copy link
Copy Markdown
Contributor Author

Thanks - that closes the loop on arm B cleanly: we were never in the same branch, and "the sync runs too late" fits the code as far as I can verify it. Before upstreaming your hoist I ran the duplicate-work check, and there is an open PR already aimed at this hazard: #51599 (closes #51571, njhill reviewing). It takes the other route: postprocess_mamba_align_gpu's only host write is the accepted-counts D2H into input_batch.num_accepted_tokens_cpu_tensor (mamba_utils.py:1439), and #51599 retargets that copy at a runner-owned buffer condense() never touches, remapping into the input batch after the event sync. The write-write interleaving your hoist orders away cannot occur because the two writers no longer share memory, and no host stall is added.

Evidence from our side:

So I am not opening a hoist PR while #51599 is open - same defect, and the buffer-isolation shape is the better mechanism. The deciding experiment is yours if you are willing: your ~10-minute reproducer against #51599's head on the 2x3090 rig. Crash gone -> #51599 is the upstream fix and I will link your field data on that PR. Crash still there -> the hoist covers something isolation does not, and I will upstream it with you credited for the diagnosis, the fix, and the evidence.

Dropping the planned Blackwell sweep for #52873 since you closed it as checkpoint-specific. @seanyourhighness thanks for the extended async soak on the bounds half.

Determinized D2H repro (RTX 5090)
"""Determinized demonstration of the align-mode accepted-count hazard.

Main's shape: postprocess_mamba_align_gpu D2H-copies accepted counts into
input_batch.num_accepted_tokens_cpu_tensor (pinned, condense()-managed).
Under async scheduling, _update_states/condense() compacts rows of that
same buffer BEFORE num_accepted_tokens_event is synchronized, so host
writes and the in-flight DMA interleave on one buffer. torch.cuda._sleep
delays the DMA so each ordering is observable deterministically instead of
depending on bus timing.

Scenario: prev step had 4 requests [counts 4,3,2,1]; request at row 0
finished; condense moves the last row (3) into row 0 and the batch shrinks
to 3. A new request is admitted at row 3 with init count 1.
"""
import torch

assert torch.cuda.is_available()
counts_gpu = torch.tensor([4, 3, 2, 1], dtype=torch.int32, device="cuda")
SLEEP_CYCLES = 400_000_000


def condense_and_admit(buf):
    buf[0] = buf[3]   # condense: move last row into the freed slot
    buf[3] = 1        # _update_states: init a newly admitted request's row


def run(isolated: bool):
    batch_cpu = torch.tensor([4, 3, 2, 1], dtype=torch.int32).pin_memory()
    runner_cpu = torch.zeros(4, dtype=torch.int32).pin_memory()
    ev = torch.cuda.Event()
    torch.cuda._sleep(SLEEP_CYCLES)               # step N still on the GPU
    target = runner_cpu if isolated else batch_cpu
    target.copy_(counts_gpu, non_blocking=True)   # postprocess's D2H
    ev.record()
    condense_and_admit(batch_cpu)                 # host, BEFORE the sync
    ev.synchronize()                              # _prepare_inputs' wait
    return batch_cpu.tolist(), runner_cpu.tolist()


main_batch, _ = run(isolated=False)
iso_batch, iso_runner = run(isolated=True)
want_batch = [1, 3, 2, 1]  # row0 = surviving req's count, row3 = new req init

print(f"host wrote after condense+admit: {want_batch}")
print(f"main shape   -> input_batch after sync: {main_batch}"
      f"   {'HOST WRITES DESTROYED BY LATE DMA' if main_batch != want_batch else 'ok'}")
print(f"#51599 shape -> input_batch after sync: {iso_batch}"
      f"   runner buf: {iso_runner}"
      f"   {'ok - both coherent' if iso_batch == want_batch and iso_runner == [4, 3, 2, 1] else 'CORRUPTED'}")

Output:

host wrote after condense+admit: [1, 3, 2, 1]
main shape   -> input_batch after sync: [4, 3, 2, 1]   HOST WRITES DESTROYED BY LATE DMA
#51599 shape -> input_batch after sync: [1, 3, 2, 1]   runner buf: [4, 3, 2, 1]   ok - both coherent

@noonghunna

Copy link
Copy Markdown

Ran your deciding experiment on the 2×3090 (TP=2, Ampere sm_86, PCIe-only, vLLM v0.27.1, fp8 KV, mamba_cache_mode=align, prefix caching + async scheduling on, MTP n=4). Verdict: #51599 alone does not stop this crash on our rig — our sync-hoist does. Three arms, same config, same reproducer (your growing-conversation script + min_tokens=400 to keep generation sustained):

Arm Result
stock v0.27.1 (no patches) crash @ 10,873 genillegal memory access
+ #51599 only crash @ 8,446 gen — same site
+ hoist only (num_accepted_tokens_event.synchronize() at top of synchronize_input_prep) clean to 42,769 gen / 80 turns, 0 Xid (ended on a context-length 400, not a crash)

The crash site is not the buffer #51599 isolates. Both the stock and #51599 arms die here:

execute_model → _prepare_inputs → _calc_spec_decode_metadata
  → cu_num_draft_tokens = async_tensor_h2d(cu_num_draft_tokens, device=…)   # gpu_model_runner.py
  → (surfaces at) synchronize_input_prep

#51599 retargets the num_accepted_tokens D2H to a runner-owned buffer, which removes that write/write interleave — but the fault is a different input-prep H2D (cu_num_draft_tokens) racing the in-flight prev-step postprocess. Isolating one buffer doesn't order the others. The hoist fixes it because it gates the entire synchronize_input_prep (every downstream H2D, cu_num_draft_tokens included) behind the accepted-counts event, rather than isolating one buffer at a time. So on our rig #51599 is insufficient as-is for this crash — the buffer-isolation shape would need to extend to the other async input-prep copies (cu_num_draft_tokens and siblings) too, whereas the single event-ordering covers them all at once.

One caveat on the checkpoint, since it matters for interpreting this. We can no longer reproduce this crash on the checkpoint we actually ship. Our fast tier moved from an early Intel-AutoRound INT4 export ("Avuja") to a re-export ("Frozenlock") — same quant method/bits/group-size, the one difference being that the earlier export excluded mtp.layers from block_name_to_quantize (left the MTP head unquantized), which is what caused the separate acceptance-collapse bug we filed as #52873. We dropped that export for the acceptance collapse, not for this crash. On the current shipped export the crash no longer fires within a 57k-gen soak, so to get a reliable crashing baseline to test #51599 against, I had to run the retired export. Two things worth flagging from that:

Happy to share the full per-arm logs, or to re-run against a revised #51599 that also isolates the cu_num_draft_tokens (and sibling) input-prep copies if you'd rather keep the buffer-isolation shape than adopt the event-ordering.

@amittell

Copy link
Copy Markdown
Contributor Author

That settles it, and it goes against the call I made. #51599 does not cover your crash, so the hoist is upstreamed as #53613 with you as co-author, credited for the diagnosis, the fix and the field evidence. Thanks for running three arms instead of two - the #51599-only arm is the one that makes this decidable.

One correction, because a reviewer will make it otherwise. An illegal memory access is asynchronous and sticky: the frame it surfaces in is the first CUDA call that checks the error, not the kernel that raised it. async_tensor_h2d(cu_num_draft_tokens, ...) is a cudaMemcpyAsync from a freshly pinned host buffer into a freshly allocated device tensor - it has no index it can get wrong, so it is reporting someone else's fault rather than committing one. I left the site attribution out of the PR for that reason. It costs the three arms nothing.

What the code does support is your earlier framing, and that is what the PR says:

  • prepare_inputs_event is recorded in the finally of synchronize_input_prep (gpu_model_runner.py:3949), so it marks the end of input prep, before the forward. Waiting on it next step orders nothing against the spec-decode postprocess, which runs after the model.
  • the only wait that does is num_accepted_tokens_event.synchronize() at gpu_model_runner.py:2173, inside _prepare_inputs. In execute_model, _update_states is line 4322 and _prepare_inputs is 4365, both inside the same synchronize_input_prep block, so _update_states gets there first with nothing in between.
  • postprocess_mamba_align_gpu (mamba_utils.py:1416-1441) reads the persistent per-group block tables bound at initialize_from_forward_context, the staged state-index / scheduled / computed / draft GPU buffers, then D2Hs the accepted counts into the pinned input-batch tensor. _update_states rewrites all of it.

synchronize_input_prep is byte-identical between v0.27.1 (3865-3877) and main (3937-3949), and the record and wait sites sit in the same relative positions, so your v0.27.1 result carries to main without reinterpretation. That was the first thing I checked.

On the revised-#51599 offer: I would not go that way, and you should not have to. Those input-prep copies are H2D out of buffers allocated per call, so there is no shared residency to move - the defect is ordering, not destination. #51599 should still land on its own merits: the host-side condense corruption it fixes (#51571) is real and independent, and my determinized _sleep repro on the 5090 shows it destroying host writes on main's shape. Complementary, not competing, and I have said so on the PR.

Two asks. Attach your per-arm logs to the new PR so the field evidence sits with the change rather than three issues away. And when you get a window, run the branch head - it is the same two lines you shipped plus a comment and four regression tests, but I would rather cite a run of the exact tree than a reconstruction of it.

One number from our rig you will want, because it reframes the perf argument for both of us. I timed both accepted-count waits directly. On main the existing wait at 2173 blocks the host 170.1 ms per step; with the hoist the top-of-context wait is 171.8 ms and the 2173 one drops to 2.4 us. So the hoist costs +1.7 ms per step, about 1 percent - and the wait it moves was never cheap to begin with. Our end-to-end benchmark could not see this: six arms, and the sign flipped between the 200-prompt and 60-prompt sets, so the noise is bigger than the effect. Your TPS-neutral claim holds at the resolution either of us can measure end to end; the direct number is the one I would defend in review.

The retired-export caveat is in the PR as you stated it. It bears on how easy the race is to hit, not on whether it is one; an unordered write-after-read does not become ordered because a checkpoint stopped exposing it.

@amittell

Copy link
Copy Markdown
Contributor Author

Version applicability, since this has been open across two releases and it decides how anyone backports it.

Blob shas per ref (git rev-parse <ref>:<path>, first 8), for the seven source files this PR touches:

file v0.26.0 v0.27.1 v0.28.0 main
mamba/ops/causal_conv1d.py f7c237ca 8335c849 8335c849 8335c849
mamba/ops/mamba_ssm.py d348defc af454678 af454678 af454678
flash_linear_attention/ops/fused_sigmoid_gating.py 7e0c7e05 7e0c7e05 7e0c7e05 7e0c7e05
kimi_k3/nvidia/ops/third_party/kda/fused_recurrent.py absent b4d35d85 b4d35d85 b4d35d85
kimi_k3/amd/ops/third_party/kda/fused_recurrent.py absent 2f512df6 db519fb6 db519fb6
flash_linear_attention/ops/fused_recurrent.py 920efa44 c8004cb5 e1d0d965 e1d0d965
v1/worker/mamba_utils.py 7f611a2e fd2075bf 9b89af08 5dfa6f47

Four of the seven are identical at v0.27.1, v0.28.0 and main, so the unbounded state lookups are present unchanged in both shipped releases and on main and the fix applies to all three without reinterpretation. fused_sigmoid_gating.py has not moved since v0.26.0 at all, and the kimi_k3 tree does not exist at v0.26.0.

Three do move: the AMD KDA kernel changed between v0.27.1 and v0.28.0, and fused_recurrent.py and mamba_utils.py differ at every ref. Anyone backporting this should apply it as a patch per file rather than replacing whole files, which is the trap on mamba_utils.py in particular.

Ready for review whenever a maintainer has bandwidth. Every label on this PR so far is from mergify[bot].

@amittell

Copy link
Copy Markdown
Contributor Author

Model evaluation

GSM8K, 1319 questions, 5-shot, temperature 0, seed 42, max_tokens=256, against /v1/completions
using the in-tree harness tests/evals/gsm8k/gsm8k_eval.py.

AI assistance was used for this evaluation. Every number below is copied from the run's JSON
output or the server log; nothing is estimated.

Why this configuration

This PR only changes GDN/KDA/Mamba spec-decode kernels, so a GSM8K run without speculative
decoding would not touch a single changed line. The eval therefore runs a hybrid GDN model with
MTP speculative decoding and mamba cache mode align active, which is the path through
vllm/v1/worker/mamba_utils.py, vllm/third_party/flash_linear_attention/ops/fused_recurrent.py,
.../fused_sigmoid_gating.py and vllm/model_executor/layers/mamba/ops/causal_conv1d.py.

Both were verified from the server log of every run, not assumed:

INFO [config.py:605] Mamba cache mode is set to 'align' for Qwen3_5ForConditionalGeneration by default when prefix caching is enabled
speculative_config=SpeculativeConfig(method='mtp', model='/home/alexm/models/Qwen3.8-27B-NVFP4', num_spec_tokens=4)
INFO [metrics.py:120] SpecDecoding metrics: Mean acceptance length: 4.23, ... Avg Draft acceptance rate: 81.3%

83 SpecDecoding metrics windows were emitted per run, and ~147k draft tokens were accepted per
run, so the speculative path was continuously live throughout each measurement.

Hardware and software

  • Host: NVIDIA GB10 (Grace-Blackwell), aarch64, 1 GPU, driver 580.173.02, Ubuntu 24.04.4, Linux 6.17.0-1026-nvidia
  • vLLM 0.26.1rc1.dev861+g9a198c0f8 (editable, VLLM_USE_PRECOMPILED=1), Python 3.12.3
  • torch 2.13.0+cu132, triton 3.7.1, transformers 5.15.1, flashinfer-python 0.6.17, flashinfer-cubin 0.6.17
  • Model: Qwen3.8-27B-NVFP4 (Qwen3_5ForConditionalGeneration) with the in-repo model_mtp.safetensors, served from local NVMe

Serve command (identical for every run):

vllm serve /home/alexm/models/Qwen3.8-27B-NVFP4 \
  --served-model-name qwen38 \
  --speculative-config '{"method": "qwen3_5_mtp", "num_speculative_tokens": 4}' \
  --enable-prefix-caching \
  --kv-cache-dtype fp8 \
  --async-scheduling \
  --gpu-memory-utilization 0.6 \
  --max-model-len 8192 \
  --port 8000

--max-model-len 8192 was added to the requested flag set to bound the KV budget; everything else
is as specified. Reported KV cache size was 290,133 tokens.

Deviation worth recording: flashinfer had to be moved from the branch's pinned
flashinfer-python==0.6.16.post3 to 0.6.17 plus flashinfer-cubin==0.6.17. Without it the
engine died at startup on this GB10 with RuntimeError: FlashInfer backend is not available,
because has_flashinfer() requires either the cubin package or nvcc on PATH, and the
FlashInfer XQA decode path is selected on sm121. The same versions were used for both arms.

Method

Both arms ran on the same install, toggling only the patch with git apply / git apply -R
of the PR diff (git diff $(git merge-base pr50021 main) pr50021, 693 lines, md5
9ec58411bb4105e0f5240af81cf59f71). The change is pure Python plus Triton, so no rebuild is
needed between arms. The md5 of every changed source file was printed before each boot to prove
the toggle actually took effect, for example vllm/v1/worker/mamba_utils.py is
b7d5c17488a9360316efe7a53e4fedde patched and a1e9501b8721c6938b5b8316a55ebb1d stock.

The server was restarted for every single run, so no run inherits another run's prefix cache. The
first boot on the fresh install was discarded as compile-cache warmup. Arms were interleaved
(patched, stock, patched, stock) so that any machine drift is shared.

Results - 1319 questions, client concurrency 32

run accuracy correct invalid latency (s) output tok/s mean acceptance length draft acceptance
patched run 1 0.840030 1108/1319 0.68% 828.83 233.03 4.232 81.31%
patched run 2 0.840030 1108/1319 0.76% 834.18 233.31 4.256 81.47%
stock run 1 0.843063 1112/1319 0.91% 833.26 232.37 4.235 81.33%
stock run 2 0.846854 1117/1319 1.14% 831.23 232.82 4.238 81.23%

Per-arm spread:

  • patched: mean 0.840030, spread 0.000000 (both runs returned exactly 1108/1319)
  • stock: mean 0.844958, spread 0.003791 (1112 and 1117 of 1319)
  • output tok/s: patched mean 233.17, spread 0.29; stock mean 232.59, spread 0.45

Do the arms differ beyond noise? No.

Pooling the two runs per arm, patched is 2216/2638 = 0.840030 and stock is 2229/2638 = 0.844958.
The patched arm is 0.493 percentage points lower, a difference of 13 questions out of 2638.

  • pooled standard error of the difference: 1.003 pp
  • z = 0.4913, two-sided p = 0.6232
  • 95% CI on the difference: -1.47 pp to +2.46 pp, which straddles zero

For scale, the binomial standard error of a single 1319-question run at p = 0.84 is 1.009 pp,
which is twice the observed difference. The stock arm's own run-to-run spread was 0.38 pp while
the patched arm's was 0.00 pp, so the two arms' ranges overlap on that measure as well.

Throughput is likewise indistinguishable: patched is +0.25% on output tok/s, against a per-arm
spread of 0.29 and 0.45 tok/s. Speculative acceptance is unchanged - mean acceptance length
4.232 / 4.256 patched vs 4.235 / 4.238 stock, draft acceptance 81.2% to 81.5% across all four runs.

Conclusion: no accuracy regression, no throughput regression, no change in speculative
acceptance.
The bound is what a 2638-question comparison can support, namely that any accuracy
effect is smaller than about 2 percentage points in either direction.

Separate finding, present in BOTH arms, not caused or fixed by this PR

While selecting the eval configuration I hit a hard output-quality cliff as client concurrency
rises, on this model and flag set. It affects stock and patched identically, so it is reported
here as an observation rather than as an evaluation result.

Sweep of 200 questions on one server boot per arm:

client concurrency patched accuracy patched invalid stock accuracy stock invalid
8 0.820 0.0% 0.830 0.0%
32 0.855 0.0% 0.855 0.0%
64 0.595 33.0% 0.610 30.0%

At concurrency 64 the "invalid" responses are not malformed answers - they are requests that
return HTTP 200 with exactly 1 completion token and an empty string, i.e. the model emits an
end token immediately. There were 66 such requests of 200 patched and 60 of 200 stock. No client
exception was raised and no server error was logged.

A full 1319-question matrix at concurrency 128 reproduces it at scale, again equally in both arms:

run accuracy invalid latency (s) output tok/s mean acceptance length
patched run 1 0.572403 34.12% (450) 656.33 185.86 4.231
patched run 2 0.563306 34.34% (453) 652.76 186.90 4.236
stock run 1 0.575436 34.27% (452) 649.30 184.41 4.189
stock run 2 0.563306 34.95% (461) 651.72 186.81 4.242

patched mean 0.567854 (spread 0.009098), stock mean 0.569371 (spread 0.012130), difference
+0.15 pp - again inside the run-to-run spread.

The empty-output set is not deterministic. Comparing the captured raw completions of two
1319-question runs, 451 and 453 requests came back empty but only 259 indices overlap, and only
551 of 1319 completions were byte-identical between the runs. So the failure follows batch
composition and scheduling, not the prompt. During these runs the engine reported GPU KV cache
usage of 78% to 94% with 78 to 86 requests waiting, so the plausible suspect is the
preempt-and-resume path for mamba state under speculative decoding rather than anything this PR
touches. I have not root-caused it and am not claiming it is a bug in main; I am recording it
because it is reproducible and it is the reason the headline eval was run at concurrency 32.

What I could not do

  • No comparison against a non-speculative baseline, and no second model. Only Qwen3.8-27B-NVFP4
    with MTP was evaluated.
  • The concurrency-64 empty-output cliff was measured but not root-caused, and no upstream issue
    was filed for it.
  • Runs are two per arm per configuration. That is enough to show the arms overlap but it does not
    resolve differences below roughly 2 percentage points.

Reproduce

git diff $(git merge-base pr50021 main) pr50021 > /tmp/pr50021.patch   # 693 lines
VLLM_USE_PRECOMPILED=1 uv pip install -e . --torch-backend=auto
uv pip install --extra-index-url https://flashinfer.ai/whl/ flashinfer-python==0.6.17 flashinfer-cubin==0.6.17
# boot the server with the flags above, then per arm:
.venv/bin/python -c "
import sys; sys.path.insert(0, 'tests/evals/gsm8k')
from gsm8k_eval import evaluate_gsm8k
print(evaluate_gsm8k(num_questions=1319, num_shots=5, max_tokens=256,
                     temperature=0.0, seed=42, max_concurrency=32,
                     request_timeout_seconds=7200))"
# toggle arms with: git apply -R /tmp/pr50021.patch   (and git checkout -f pr50021 to restore)

max_concurrency and request_timeout_seconds are passed by calling evaluate_gsm8k() directly
because the script's main() does not expose request_timeout_seconds.

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

Labels

bug Something isn't working k3 kimi nvidia v1

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

[Bug]: Accuracy drops ~20% when --enable-prefix-caching is used together with MTP speculative decoding (Qwen3.6 35B-A3B)

4 participants