Skip to content

[Bugfix] Make GLM-5.3 kpool metadata graph-safe without prefix caching - #7

Merged
ZJY0516 merged 3 commits into
ZJY0516:glm-releasefrom
andyluo7:fix/glm53-kpool-cudagraph
Sep 2, 2026
Merged

ZJY0516 merged 3 commits into
ZJY0516:glm-releasefrom
andyluo7:fix/glm53-kpool-cudagraph

Conversation

@andyluo7

@andyluo7 andyluo7 commented Aug 27, 2026

Copy link
Copy Markdown

Purpose

This is a focused follow-up to vllm-project#53906 and the ROCm work
tracked in vllm-project#53943.

GLM-5.3-Flash uses MambaHybridModelState together with a one-block circular
kpool tail. With prefix caching disabled, two shared metadata issues prevent
reliable graph execution on both CUDA and ROCm:

  1. MambaHybridModelState.prepare_attn() does not forward the input positions,
    so the kpool-tail metadata builder cannot construct the circular slot
    mapping.
  2. The corrected mapping is currently returned from a temporary clone. Full
    graph replay retains that tensor address after the temporary tensor is
    released, which can produce an illegal memory access.

This PR forwards positions and gives KpoolTailMetadataBuilder persistent,
builder-owned output storage. It does not change attention kernels or platform
dispatch. The implementation is backend-neutral because both CUDA and ROCm use
the same kpool-tail metadata builder.

This does not duplicate another open PR. Searches for issue 53943, GLM-5.3
graph/kpool/positions, and kpool cudagraph fixes found no existing submission.

This change was prepared with OpenAI Codex assistance. The submitted diff and
validation evidence were reviewed by the human submitter before publication.

Test Plan

.venv/bin/python -m pytest \
  tests/v1/worker/test_mamba_hybrid_model_state.py \
  tests/v1/attention/test_kpool_tail_slot_mapping.py \
  -q

pre-commit run --files \
  vllm/v1/worker/gpu/model_states/mamba_hybrid.py \
  vllm/v1/attention/backends/mla/indexer.py \
  tests/v1/worker/test_mamba_hybrid_model_state.py \
  tests/v1/attention/test_kpool_tail_slot_mapping.py

pre-commit run mypy-3.12 --hook-stage manual --files \
  vllm/v1/worker/gpu/model_states/mamba_hybrid.py \
  vllm/v1/attention/backends/mla/indexer.py \
  tests/v1/worker/test_mamba_hybrid_model_state.py \
  tests/v1/attention/test_kpool_tail_slot_mapping.py

Hardware validation used 8x AMD Instinct MI355X (gfx950), TP8, AITER,
Triton MoE, CUDAGraphMode.FULL_AND_PIECEWISE, disabled prefix caching, and a
random exact-1024-input/exact-1024-output workload with ignore-EOS enabled.
The regression tests exercise the shared metadata behavior, but CUDA graph-mode
serving was not independently rerun as part of this validation.

Test Result

  • Focused pytest on MI355X: 14 passed, 3 skipped.
  • All staged-file pre-commit hooks passed, including Ruff and mypy.
  • Mypy Python 3.12 manual hook passed.
  • C1: 10/10 requests, 10,240 input and 10,240 output tokens, 146.526 total
    tok/s, 13.558 ms mean TPOT.
  • C32: 320/320 requests, 327,680 input and 327,680 output tokens, 3,254.841
    total tok/s, 18.929 ms mean TPOT.
  • All responses were non-empty and all per-request errors were empty.
  • Post-run health was HTTP 200. No measured HIP faults, illegal accesses,
    OOMs, engine deaths, HTTP 500s, tracebacks, retries, or JIT activity occurred
    during either measured window.
  • All eight GPUs passed a post-run allocation-and-matmul recovery check.

The benchmark used base commit
142062f13d16bed254b5d97cc3d371fbd4f7790a plus exactly the two commits in
this PR.


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, including links to the related upstream PR and
    issue.
  • The test commands are provided.
  • Focused test and end-to-end benchmark results are provided.
  • No documentation update is required for this metadata fix.

andyluo7 and others added 2 commits August 27, 2026 12:40
Co-authored-by: OpenAI Codex <codex@openai.com>
Signed-off-by: andyluo7 <andy.luo@amd.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Signed-off-by: andyluo7 <andy.luo@amd.com>
@andyluo7

Copy link
Copy Markdown
Author

/ci run

@stefanskiasan

Copy link
Copy Markdown

Verified on 8× MI350X (gfx950): this fixes the CUDA-graph fault, and it is worth a lot more than a bugfix — graph-mode decode is 3.3× the eager throughput.

You noted "CUDA graph-mode serving was not independently rerun". That is the exact configuration that was failing on our box (reported in vllm-project#53943), so here is that run.

Setup: ZJY0516/vllm@glm-release (142062f) with only the two production files from this PR applied — vllm/v1/attention/backends/mla/indexer.py and vllm/v1/worker/gpu/model_states/mamba_hybrid.py at 26cbb6d50. AITER sparse-MLA files from ROCm/aiter#4919 mounted as before. TP8, --max-model-len 8192, --kv-cache-dtype bfloat16, --max-num-seqs 64, --gpu-memory-utilization 0.85, VLLM_ROCM_USE_AITER=1, VLLM_ROCM_USE_AITER_MOE=0, --reasoning-parser glm45 --tool-call-parser glm47. No --enforce-eager. 8× MI350X (gfx950), ROCm 7.2.3, Triton 3.7.1, zai-org/GLM-5.3-Flash FP8 checkpoint.

I removed a local kpool-warmup experiment of my own before this run, so the result is attributable to your two files alone.

Fault: gone. Capturing CUDA graphs (FULL): 100%, server up, and the load that previously killed all workers now completes cleanly. Before this PR the same box, same config, same load: server started, captured graphs, began generating (Avg generation throughput: 281.9 tokens/s), then died mid-decode with Memory access fault by GPU node-N on every rank. Your diagnosis matches what we saw exactly — the fault fired after generation was already flowing, which is what a retained-then-released tensor address in graph replay would look like.

Throughput (8 fixed short English prompts, temperature=0.0, max_tokens=1024, generated completion_tokens / wall_clock):

N eager (previous baseline) with CUDA graphs (this PR) speedup
1 12 tok/s 60 tok/s 5.0×
8 82 tok/s 323 tok/s 3.9×
32 327 tok/s 1,150 tok/s 3.5×
64 659 tok/s 2,203 tok/s 3.3×
128 670 tok/s 2,217 tok/s 3.3×

All requests completed in every run, zero faults, zero empty generations. Median latency at N=64 drops from 87.3 s to 26.1 s. Saturation is still --max-num-seqs 64 (N=64 → N=128 is flat at ~2.2 k tok/s), so the ceiling is a scheduler bound, not this change.

Correctness spot checks after the load runs, both finish_reason: stop:

"Was ist die Hauptstadt von Frankreich?"  -> "Die Hauptstadt von Frankreich ist Paris."
"Was ist 17 mal 23? Nur die Zahl."        -> "391"

Two things worth noting for whoever picks this up:

  1. The AMD recipes currently published for this model — sgl-project/sglang#36608 and vllm-project/recipes#857 — both specify eager / --disable-cuda-graph for the FP8 checkpoint. If this lands, that guidance is worth revisiting: on our node it is leaving a 3.3× on the table.
  2. This does not change the other two constraints. VLLM_ROCM_USE_AITER_MOE=0 is still required on the FP8 checkpoint ([Bug] fused MoE GPU-faults on GLM-5.3-Flash 288-expert/top-8 shape (gfx950, no tuned config) ROCm/aiter#5049), and fp8 KV still falls through to the faulting asm decode kernel, so BF16 KV remains necessary.

Happy to re-run anything here, including GSM8K under graphs if an accuracy check alongside the perf number would help the review.

cgtarmenta added a commit to Research-Warrant/vllm that referenced this pull request Sep 1, 2026
Our own first inference died with CUDA_ERROR_ILLEGAL_ADDRESS (700) at
DeepGEMM's handle.hpp:154, inside fp8_fp4_mqa_logits, reached from
sparse_attn_indexer_kpool.py:523 by way of glm5next/nvidia/attention.py:392.
Engine core dead, container restarting, on a fifteen-token prompt.

It is a known defect in PR vllm-project#53906, reported on SM90 and confirmed on our exact
configuration -- SM100 (B200), TP=4 -- and the reporter states these two
commits resolve it there. Root cause per the PR: MambaHybridModelState's
prepare_attn() does not forward input positions, so the kpool-tail metadata
builder cannot construct the circular slot mapping; and the corrected mapping
is returned from a temporary clone whose address full graph replay retains
after the temporary is freed. The second is the illegal access.

Pure Python -- four files, two of them tests, no CUDA -- so the image rebuild
reuses every compiled layer.

Note a discrepancy worth keeping: the B200 reporter describes the trigger as a
KV cache exceeding index_topk=2048 tokens, with shorter prompts serving
correctly. Ours crashed on fifteen tokens. The PR's own explanation is
prompt-length independent, so read the threshold as that reporter's
observation rather than the condition.

Claude-Session: https://claude.ai/code/session_019Z67JT37vJqhsn7yMhJNv2
@ZJY0516
ZJY0516 merged commit 8f8cc41 into ZJY0516:glm-release Sep 2, 2026
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.

3 participants