Skip to content

[Bugfix] Keep the GLM-5.3 kpool tail and Qwen4 QSA ring out of the null block - #59528

Merged
njhill merged 5 commits into
vllm-project:mainfrom
ivanium:fix/glm53-kpool-tail-skips-null-block
Oct 8, 2026
Merged

njhill merged 5 commits into
vllm-project:mainfrom
ivanium:fix/glm53-kpool-tail-skips-null-block

Conversation

@ivanium

@ivanium ivanium commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Block id 0 is the null block. It is shared by every KV cache group and is assumed to stay zero. Since #35431, the runners mark rows that own no state with 0 in the block table, not PAD_SLOT_ID: dummy runs, CUDA graph capture and padding rows all carry 0. Two ring caches that address their pages through column 0 of the block table did not treat 0 as "no ring", so dummy runs wrote their rows into block 0:

  • GLM-5.3-Flash kpool tail: _kpool_tail_slot_mapping_kernel maps every token to block_table[req][0] * kpool + pos % kpool. Its torch fallback does the same.
  • Qwen4 QSA circular ring: circular_qsa_slot_mapping and _build_qsa_metadata_kernel map tokens to block_table[req, 0] and kept every block >= 0. The compressed QSA path already keeps dummy slots inert (test_qsa_compressed_metadata_keeps_dummy_slots_inert); the ring path did not.

Every scheduled request owns its ring block, so a request on the null block has none. Its tokens now map to PAD, as the generic slot mapping does. The ring writers already skip PAD slots.

This is the same pattern as #58560, which fixes the DeepSeek-V4.1 compressor ring.

Changes

  • vllm/v1/attention/backends/mla/indexer.py: the kpool tail slot mapping (Triton and torch) emits PAD for requests on the null block.
  • vllm/models/qwen4_exp/common/qsa_cache.py: the QSA circular ring slot mapping (torch and Triton) requires block > 0.
  • Tests: the existing QSA circular metadata test, which runs the Triton builder, also checks a dummy batch (all-zero block table) and expects PAD. QSA tests that used block 0 as a real ring block now start real blocks at 1. This includes _make_block_table in the AMD pre-indexer test, which could randomly hand out block 0. For the kpool tail, the existing CPU padding test now ends with a padding request on the null block, and the existing Triton-vs-CPU test adds a mixed batch and an all-null dummy batch. Both check that the null-block request maps to PAD.

Not a duplicate

Test Plan

pytest tests/models/qwen4_exp/test_qsa_reference.py tests/models/qwen4_exp/test_qsa_pre_indexer_amd.py tests/v1/attention/test_kpool_tail_slot_mapping.py tests/v1/worker/test_gpu_kpool_tail_slot_mapping.py
pre-commit run --files <changed files>

Test Result

  • Rebased on main d6fe5dca68. 102 passed, 16 skipped on GB200. The 16 skips are test_qsa_pre_indexer_amd.py, which requires ROCm.
  • Control: with the QSA change reverted, the dummy-batch check fails: dummy tokens map to real slots in block 0.
  • Control: with the kpool tail change reverted, the CPU padding test and the two null-block Triton cases fail.
  • pre-commit passes.
  • No model eval. Only tokens of requests on the null block change, and those are dummy or padding rows whose outputs are discarded. Real requests never own block 0 (BlockPool pops it as the null block at init), so their mappings and outputs are unchanged.

AI assistance (Claude Code) was used for this PR. The submitter reviewed the changes and ran the tests above.

🤖 Generated with Claude Code

@mergify mergify Bot added glm bug Something isn't working labels Oct 1, 2026
@ivanium
ivanium marked this pull request as ready for review October 1, 2026 01:59
@ivanium
ivanium requested a review from pavanimajety as a code owner October 1, 2026 01:59

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

@ivanium
ivanium force-pushed the fix/glm53-kpool-tail-skips-null-block branch from 3c3bac9 to 4ac9ca0 Compare October 1, 2026 02:07
@ivanium ivanium changed the title [Bugfix][GLM-5.3] Keep the kpool tail out of the null block [Bugfix] Keep ring and linear-attention state writers out of the null block Oct 1, 2026
@mergify mergify Bot added the qwen Related to Qwen models label Oct 1, 2026
@ivanium
ivanium force-pushed the fix/glm53-kpool-tail-skips-null-block branch 2 times, most recently from 8e811e4 to 752ff37 Compare October 1, 2026 03:27
@ivanium ivanium changed the title [Bugfix] Keep ring and linear-attention state writers out of the null block [Bugfix] Keep the GLM-5.3 kpool tail and Qwen4 QSA ring out of the null block Oct 1, 2026
@ivanium
ivanium force-pushed the fix/glm53-kpool-tail-skips-null-block branch 2 times, most recently from 62f0e10 to 7b29aa4 Compare October 1, 2026 03:33
@mergify

mergify Bot commented Oct 1, 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, @ivanium.

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

@mergify mergify Bot added the needs-rebase label Oct 1, 2026

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

LGTM @ivanium
nit: tests for diffs in vllm/v1/attention/backends/mla/indexer.py also needed

ivanium and others added 3 commits October 7, 2026 23:32
The tail slot kernel maps every token to its request's tail block from column
0 of the block table. Dummy runs, CUDA graph capture and padding requests get
the null block there, so their tail rows were written into block 0, which all
KV cache groups share. Map tokens of a request without a tail block to PAD, as
the generic slot mapping does for them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yifan Qiao <yifanqiao@inferact.ai>
The QSA circular ring maps a request's tokens to block_table[req, 0] and kept
every block >= 0, but the runners mark rows that own no ring with the null
block (0), not PAD. Dummy runs and CUDA graph capture therefore wrote ring rows
into block 0, which all KV cache groups share. The compressed QSA path already
keeps such dummy slots inert; do the same for the ring by requiring a block > 0.
Tests that used block 0 as a real ring block now start real blocks at 1.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yifan Qiao <yifanqiao@inferact.ai>
The CPU padding test now ends with a padding request on the null block, and
the Triton-vs-CPU test runs a mixed batch and an all-null dummy batch. Both
check that tokens of a request on block 0 map to PAD.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yifan Qiao <yifanqiao@inferact.ai>
@ivanium
ivanium force-pushed the fix/glm53-kpool-tail-skips-null-block branch from 7b29aa4 to b96ce88 Compare October 7, 2026 23:41
@mergify mergify Bot removed the needs-rebase label Oct 7, 2026
@ivanium

ivanium commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

/ci run

@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown

❌ This PR is 1 commit behind upstream main. Your branch must contain every commit currently on upstream main. No new CI build was started. Merge or rebase onto the latest main, then rerun /ci run. To test this branch at your own risk, use /ci run --allow-stale.

@njhill

njhill commented Oct 8, 2026

Copy link
Copy Markdown
Member

/ci run

@njhill
njhill enabled auto-merge (squash) October 8, 2026 19:00
@github-actions github-actions Bot added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 8, 2026
@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #93683 for commit c25289d5cf19.

@njhill
njhill merged commit 240785b into vllm-project:main Oct 8, 2026
202 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working glm qwen Related to Qwen models ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants