Skip to content

[Bugfix][Attention] Size FlashInfer paged-KV buffers for local attention - #50022

Open
thegoldenflow wants to merge 3 commits into
vllm-project:mainfrom
thegoldenflow:fix/flashinfer-local-attn-buffers
Open

thegoldenflow wants to merge 3 commits into
vllm-project:mainfrom
thegoldenflow:fix/flashinfer-local-attn-buffers

Conversation

@thegoldenflow

@thegoldenflow thegoldenflow commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Purpose

Fixes #49980.

Under chunked local attention, make_local_attention_virtual_batches turns N requests into M > N virtual batches and passes M to the builder as num_reqs. FlashInferMetadataBuilder sized paged_kv_indptr, paged_kv_last_page_len and paged_kv_indices from max_num_seqs, so they overflow whenever M > max_num_seqs. The reporter (max_num_seqs=1) hit ValueError: provided out is the wrong size for the accumulation from the np.cumsum in _compute_flashinfer_kv_metadata.

This PR sizes those buffers in __init__ from an upper bound on the virtual batch count, min(2 * max_num_seqs + cdiv(max_num_batched_tokens, c), max_num_batched_tokens) (new helper max_local_attention_virtual_batches).

  • Detected with getattr(spec, "attention_chunk_size", None). The builder gets a ChunkedLocalAttentionSpec when the hybrid KV cache manager is on (the CUDA default). When it is off (e.g. EAGLE, --disable-hybrid-kv-cache-manager) it gets a FullAttentionSpec(attention_chunk_size=...), which an isinstance check would miss. The test covers both.
  • Sized up front, not grown on overflow. When a whole batch runs on trtllm-gen (SM100), build() skips _compute_flashinfer_kv_metadata and runs a torch.cumsum into an out-of-range slice of paged_kv_indptr.gpu. The only signal is a warning, and the kernel gets a cum_seq_lens_kv that is too short. The issue's grow-on-overflow check lives in _compute_flashinfer_kv_metadata, so it would never see this path. Rebinding the buffers is also unsafe: CUDA-graph decode wrappers alias them, and fast_decode_plan does not copy into them.
  • Backstop. _ensure_paged_kv_capacity runs at the top of build() and again once the page count is known. If the bound is ever wrong, it grows the buffers, or raises when enable_cuda_graph is set.
  • Page count only grows (max, not min). With the hybrid manager off, all Llama-4 layers merge into one KV cache group whose FullAttentionSpec keeps attention_chunk_size. The global-attention layers read that spec but attend over the whole sequence, so capping pages at the chunk size would under-size them (2250 → 512 pages in the reporter's config).
  • XQA draft mask (third commit). [Attention] Add FlashInfer XQA decode support on SM12x #49718, merged after this PR was opened, added _decode_mask_cache, also sized from max_num_seqs. With XQA speculative decoding, a verify window split evenly by a chunk boundary can produce more uniform decodes than that. XQA indexes the mask per request without a shape check, so it reads past the end. The cache now uses the same bound. Reaching this needs Llama-4 with FlashInfer on SM90/SM12x and speculative decoding (any method) with an odd k >= 3.
Why the bound holds, and what it allocates

With c = attention_chunk_size, N = max_num_seqs and T = max_num_batched_tokens, each request contributes 1 + cdiv(q_i - f_i, c) virtual batches, where f_i = min(c - ((s_i - q_i) mod c), q_i) >= 1. Summing gives V <= 2N + cdiv(T, c). The 2N term is needed: q_i = 2 with f_i = 1 gives V = 2N. Every virtual batch holds at least one query token, so V <= T. Every virtual batch sees at most c KV tokens, so pages <= V * cdiv(c, block_size). A brute-force check over 2.7M configurations found no violations. Zero-length padded requests only occur in FULL CUDA-graph mode, which chunked-local rules out (AttentionCGSupport.NEVER).

config paged_kv_indptr paged_kv_indices
reporter's (max_model_len=36000, T=36000, N=1, c=8192, block 16) 2 → 8 2250 → 3584
max_model_len=131072, T=8192, N=256, c=8192, block 16 257 → 514 unchanged
spec without attention_chunk_size unchanged unchanged

Question for reviewers: FullAttentionSpec.merge keeps attention_chunk_size, but its consistency assert only checks fields(AttentionSpec), which excludes it. Is that intended? If not, the max() above can go. I left it as out of scope.

Test Plan

pytest -q tests/v1/attention/test_chunked_local_attention.py             # CPU only
pytest -q tests/v1/attention/test_flashinfer_chunked_local_attention.py  # CUDA + flashinfer, no Llama-4 weights

To see the failures, revert vllm/v1/attention/backends/flashinfer.py to origin/main and re-run the second file. create_vllm_config fetches config.json from the gated meta-llama/Meta-Llama-3-8B, so an HF token is needed.

Test Result

AGENTS.md

@mergify mergify Bot added nvidia v1 bug Something isn't working labels Jul 27, 2026
@thegoldenflow

Copy link
Copy Markdown
Contributor Author

GPU verification is done. Ran the FlashInfer chunked-local test on an RTX 4090: 4 passed with this patch. Reverting only the two source files this PR touches reproduces #49980 with the predicted "provided out is the wrong size for the accumulation", and restoring them returns to 4 passed. Neighbouring flashinfer tests show no regressions, and full pre-commit passes on Linux. Both the GPU test and Linting sections of the body now carry the real output, so this is no longer a draft.

@thegoldenflow
thegoldenflow marked this pull request as ready for review July 28, 2026 18:32

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

@janbernloehr

Copy link
Copy Markdown

I confirm that cherry-picking this patch into the current upstream vLLM 0.26.1rc1.dev528+gf8d03e774 passes our previously-fatal 32k-prefix Llama-4 Scout scenario on DGX H100 (ISL avg 34,000.80 tok, 108.23 output tok/s).

@mergify

mergify Bot commented Aug 12, 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, @wsyjh8.

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 Aug 12, 2026
FlashInferMetadataBuilder preallocates paged_kv_indptr,
paged_kv_last_page_len and paged_kv_indices from
scheduler_config.max_num_seqs. Under chunked local attention,
make_local_attention_virtual_batches replaces the batch with one virtual
batch per local attention block and reports that count as num_reqs, which
is decoupled from max_num_seqs. A prefill longer than attention_chunk_size
therefore overflows the buffers, raising "provided out is the wrong size
for the accumulation" from the cumsum in _compute_flashinfer_kv_metadata,
and silently truncating the shared indptr view on the trtllm-gen prefill
path, which never reaches that cumsum.

Size the buffers from an upper bound on the virtual batch count instead.
The builder already receives attention_chunk_size on its kv_cache_spec, on
ChunkedLocalAttentionSpec and on the FullAttentionSpec it is promoted to
when the hybrid KV cache manager is disabled, so no new plumbing is
needed.

The page count only ever grows: when the hybrid manager is disabled, all
layers promote to FullAttentionSpec and merge into a single KV cache group
whose spec keeps attention_chunk_size (FullAttentionSpec.merge asserts
only over fields(AttentionSpec), which excludes it). The global attention
layers form their own attention group but share that spec, and they attend
over the whole sequence, so capping pages per request at the chunk size
would under-size their buffer.

Growing on overflow is not a general option here: _get_decode_wrapper
hands these buffers to CUDA-graph decode wrappers as fixed-address
buffers and fast_decode_plan does not copy into them, so rebinding them
would leave those wrappers reading stale storage. Keep a guarded backstop
that raises instead of reallocating when decode CUDA graphs are enabled.

Closes vllm-project#49980

Signed-off-by: Jason Yao <wsyjh8@gmail.com>
Signed-off-by: Jason Yao <wsyjh8@gmail.com>
_get_decode_mask caches the uniform XQA speculative-decode mask with
max_num_seqs rows and returns its first num_decodes rows. Chunked local
attention can split a verify window evenly across a chunk boundary, so
the uniform decode count can exceed max_num_seqs; the slice then comes
back short and XQA, which indexes the mask per request without a shape
check, reads past its end.

Size the cached mask from max_buffer_reqs like the paged-KV buffers,
drop cached masks when those buffers grow, and remove the now-unused
max_num_reqs attribute.

Signed-off-by: Jason Yao <wsyjh8@gmail.com>
@thegoldenflow
thegoldenflow force-pushed the fix/flashinfer-local-attn-buffers branch from d4dd5cb to 05cc6b7 Compare September 18, 2026 05:00
@mergify mergify Bot removed the needs-rebase label Sep 18, 2026
@thegoldenflow

Copy link
Copy Markdown
Contributor Author

@janbernloehr Thank you for verifying this on DGX H100 with your Llama-4 Scout workload, much appreciated! I've since rebased onto current main and referenced your result in the PR description.

This branch has not been deployed

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

Labels

bug Something isn't working nvidia v1

Projects

Status: No status

2 participants