Repository navigation
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces support for layerwise KV pool functionality in Qwen3.5 hybrid models, which combine GDN and full attention layers. The changes ensure proper synchronization, prevent memory leaks in the block pool, and update the Mamba postprocess kernel to maintain compatibility with recent vLLM versions. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:
If CI fails, you can run linting and testing checks locally according Contributing and Testing. Tip 💡 Consider Linking a Related Issue or RFCYour PR title contains the [Feature] tag, indicating a bug fix or new feature. Linking a related issue or RFC in the PR description is strongly encouraged — it gives reviewers helpful context and speeds up the review. You can use any of these keywords:
🙏 Thanks for helping us keep the project well-organized! |
There was a problem hiding this comment.
Code Review
Suggested PR Title:
[Ops][Feature] Support layerwise KV transfer and DS conv layout in Mamba postprocessingSuggested PR Summary:
### What this PR does / why we need it?
This PR introduces several enhancements and fixes to the Ascend backend:
1. Adds metadata checks in KV layer connector utilities to prevent redundant operations.
2. Integrates layerwise KV transfer wait and compute start recording into the GDN attention forward pass.
3. Updates the fused Mamba postprocessing Triton kernel to support index mapping, precomputed token counts, and a dimension-first layout for DS conv states, while hoisting pointer casts out of loops to avoid Triton compilation issues.
4. Adds an early return in `touch_sending_mamba_blocks` when layerwise transfer is enabled.
However, a critical issue was identified in `pool_scheduler.py`: returning early when `use_layerwise` is True prevents sending mamba blocks from being pinned. This can cause the block manager to free them immediately when the request finishes, leading to a race condition and data corruption if the background sending thread is still reading them.
### Does this PR introduce _any_ user-facing change?
No user-facing API changes are introduced. These are internal performance and compatibility improvements for the Ascend backend.
### How was this patch tested?
No specific tests were added in this PR. It is recommended to verify these changes with existing integration tests for the Ascend backend and add synchronization tests for the layerwise transfer.| if self.use_layerwise: | ||
| return |
There was a problem hiding this comment.
Returning early here when self.use_layerwise is True prevents the sending mamba blocks from being touched (i.e., pinned/referenced). Since request_finished and request_finished_all_groups also return False immediately when use_layerwise is True, these blocks will be freed immediately by the block manager when the request finishes. If the layerwise sending thread (KVCacheStoreLayerSendingThread) is still asynchronously reading from these blocks in the background, they can be reassigned to a new request and overwritten, leading to a critical race condition and data corruption. Consider implementing a proper synchronization or event-based completion mechanism for layerwise sending to safely defer freeing these blocks until the transfer is fully complete.
…n models - Add wait_for_kv_layer_from_connector and record_attention_compute_start to GDN forward (ops/gdn.py) - Add has_connector_metadata guard to wait/save utils (attention/utils.py) - Skip touch_sending_mamba_blocks for layerwise to prevent block pool leak (pool_scheduler.py) Signed-off-by: tyy0829 <tyy0829@users.noreply.github.com>
- _alloc_gvas_for_save: only cache and track gva>0 keys, skip failed batch_alloc - _prepare_load_gvas: only call batch_add_lease for keys with valid size (>0) - _prepare_load_gvas: report size<=0 and lease failure blocks to _invalid_block_ids - _prepare_load_gvas: skip invalid_block_ids for multi-group (hybrid) models - build_shared: filter out gva<=0 blocks from block_ids/block_gvas before batch_copy (cherry picked from commit a80bd1c of fix-gva-invalid-v0.23, PR vllm-project#12643) Signed-off-by: tyy0829 <87685049+tyy0829@users.noreply.github.com>
The store is already initialized eagerly during MemcacheBackend construction in _init_backend. Calling init_store() again in register_kv_caches is redundant; on the lazy path it re-triggers _setup_store()/SmemBmCreate2, which synchronously allocates the HBM/DRAM memory pool at startup and causes service-launch hangs on old HDK where memory allocation blocks. (cherry picked from commit 139a166 of fix-gva-invalid-v0.23, PR vllm-project#12643) Signed-off-by: tyy0829 <87685049+tyy0829@users.noreply.github.com>
…tion models (#12711) ## Description This PR adds layerwise KV pool support for Qwen3.5 (and Qwen3-Next) hybrid models that combine GDN (Gated Delta Net) linear attention layers with standard full attention layers. ## Changes 1. **GDN forward (`ops/gdn.py`)**: Add `wait_for_kv_layer_from_connector(self.prefix)` at the start of `forward()` and `record_attention_compute_start()` before the custom op call. GDN layers do not go through the `@maybe_transfer_kv_layer` decorator, so they must explicitly call these functions to participate in layerwise KV transfer. 2. **Attention utils (`attention/utils.py`)**: Add `connector.has_connector_metadata()` guard to `wait_for_kv_layer_from_connector` and `maybe_save_kv_layer_to_connector`, matching the guard in the `@maybe_transfer_kv_layer` decorator. Prevents spurious `current_layer` counter increments during non-save steps (e.g. profile run). 3. **Pool scheduler (`pool_scheduler.py`)**: Skip `touch_sending_mamba_blocks` when `use_layerwise=True`. The layerwise send thread (`KVCacheStoreLayerSendingThread`) does not use the `completed_events` mechanism, so touched mamba blocks would never be freed, leaking the block pool. 4. **Postprocess kernel (`ops/triton/mamba/postprocess.py`)**: Update `postprocess_mamba_fused_kernel` signature to match vLLM v0.25.0, adding `state_dim_row_count_ptr`, `state_dim_row_stride_ptr`, `idx_mapping_ptr`, `CONV_STATE_DIM_FIRST`, `HAS_IDX_MAPPING`, and `PRECOMPUTED_NEW_COMPUTED` parameters. Preserves the triton-ascend pointer-type-cast optimization. ## Testing Verified on Atlas 800T A2 (Ascend 910B3, 8 NPU) with Qwen3.5-9B: - Service starts successfully with `use_layerwise=true` and MemCache backend - KV cache hit confirmed: 4 groups all hit 512 tokens, External prefix cache hit rate 22.9% - 5 concurrent requests all succeed with cache hits - MTP speculative decoding (`num_speculative_tokens=3, method=qwen3_5_mtp`) works correctly --- (Replaces #12526, which was locked-closed after a force-push to the head branch; this PR is based on the clean single-commit feature.) - vLLM main: vllm-project/vllm@ba07e4a --------- Signed-off-by: tyy0829 <tyy0829@users.noreply.github.com> Signed-off-by: tyy0829 <1455207791@qq.com> Co-authored-by: tyy0829 <tyy0829@users.noreply.github.com>
…tion models (vllm-project#12711) ## Description This PR adds layerwise KV pool support for Qwen3.5 (and Qwen3-Next) hybrid models that combine GDN (Gated Delta Net) linear attention layers with standard full attention layers. ## Changes 1. **GDN forward (`ops/gdn.py`)**: Add `wait_for_kv_layer_from_connector(self.prefix)` at the start of `forward()` and `record_attention_compute_start()` before the custom op call. GDN layers do not go through the `@maybe_transfer_kv_layer` decorator, so they must explicitly call these functions to participate in layerwise KV transfer. 2. **Attention utils (`attention/utils.py`)**: Add `connector.has_connector_metadata()` guard to `wait_for_kv_layer_from_connector` and `maybe_save_kv_layer_to_connector`, matching the guard in the `@maybe_transfer_kv_layer` decorator. Prevents spurious `current_layer` counter increments during non-save steps (e.g. profile run). 3. **Pool scheduler (`pool_scheduler.py`)**: Skip `touch_sending_mamba_blocks` when `use_layerwise=True`. The layerwise send thread (`KVCacheStoreLayerSendingThread`) does not use the `completed_events` mechanism, so touched mamba blocks would never be freed, leaking the block pool. 4. **Postprocess kernel (`ops/triton/mamba/postprocess.py`)**: Update `postprocess_mamba_fused_kernel` signature to match vLLM v0.25.0, adding `state_dim_row_count_ptr`, `state_dim_row_stride_ptr`, `idx_mapping_ptr`, `CONV_STATE_DIM_FIRST`, `HAS_IDX_MAPPING`, and `PRECOMPUTED_NEW_COMPUTED` parameters. Preserves the triton-ascend pointer-type-cast optimization. ## Testing Verified on Atlas 800T A2 (Ascend 910B3, 8 NPU) with Qwen3.5-9B: - Service starts successfully with `use_layerwise=true` and MemCache backend - KV cache hit confirmed: 4 groups all hit 512 tokens, External prefix cache hit rate 22.9% - 5 concurrent requests all succeed with cache hits - MTP speculative decoding (`num_speculative_tokens=3, method=qwen3_5_mtp`) works correctly --- (Replaces vllm-project#12526, which was locked-closed after a force-push to the head branch; this PR is based on the clean single-commit feature.) - vLLM main: vllm-project/vllm@ba07e4a --------- Signed-off-by: tyy0829 <tyy0829@users.noreply.github.com> Signed-off-by: tyy0829 <1455207791@qq.com> Co-authored-by: tyy0829 <tyy0829@users.noreply.github.com>
…tion models (vllm-project#12711) ## Description This PR adds layerwise KV pool support for Qwen3.5 (and Qwen3-Next) hybrid models that combine GDN (Gated Delta Net) linear attention layers with standard full attention layers. ## Changes 1. **GDN forward (`ops/gdn.py`)**: Add `wait_for_kv_layer_from_connector(self.prefix)` at the start of `forward()` and `record_attention_compute_start()` before the custom op call. GDN layers do not go through the `@maybe_transfer_kv_layer` decorator, so they must explicitly call these functions to participate in layerwise KV transfer. 2. **Attention utils (`attention/utils.py`)**: Add `connector.has_connector_metadata()` guard to `wait_for_kv_layer_from_connector` and `maybe_save_kv_layer_to_connector`, matching the guard in the `@maybe_transfer_kv_layer` decorator. Prevents spurious `current_layer` counter increments during non-save steps (e.g. profile run). 3. **Pool scheduler (`pool_scheduler.py`)**: Skip `touch_sending_mamba_blocks` when `use_layerwise=True`. The layerwise send thread (`KVCacheStoreLayerSendingThread`) does not use the `completed_events` mechanism, so touched mamba blocks would never be freed, leaking the block pool. 4. **Postprocess kernel (`ops/triton/mamba/postprocess.py`)**: Update `postprocess_mamba_fused_kernel` signature to match vLLM v0.25.0, adding `state_dim_row_count_ptr`, `state_dim_row_stride_ptr`, `idx_mapping_ptr`, `CONV_STATE_DIM_FIRST`, `HAS_IDX_MAPPING`, and `PRECOMPUTED_NEW_COMPUTED` parameters. Preserves the triton-ascend pointer-type-cast optimization. ## Testing Verified on Atlas 800T A2 (Ascend 910B3, 8 NPU) with Qwen3.5-9B: - Service starts successfully with `use_layerwise=true` and MemCache backend - KV cache hit confirmed: 4 groups all hit 512 tokens, External prefix cache hit rate 22.9% - 5 concurrent requests all succeed with cache hits - MTP speculative decoding (`num_speculative_tokens=3, method=qwen3_5_mtp`) works correctly --- (Replaces vllm-project#12526, which was locked-closed after a force-push to the head branch; this PR is based on the clean single-commit feature.) - vLLM main: vllm-project/vllm@ba07e4a --------- Signed-off-by: tyy0829 <tyy0829@users.noreply.github.com> Signed-off-by: tyy0829 <1455207791@qq.com> Co-authored-by: tyy0829 <tyy0829@users.noreply.github.com>
…tion models (vllm-project#12711) ## Description This PR adds layerwise KV pool support for Qwen3.5 (and Qwen3-Next) hybrid models that combine GDN (Gated Delta Net) linear attention layers with standard full attention layers. ## Changes 1. **GDN forward (`ops/gdn.py`)**: Add `wait_for_kv_layer_from_connector(self.prefix)` at the start of `forward()` and `record_attention_compute_start()` before the custom op call. GDN layers do not go through the `@maybe_transfer_kv_layer` decorator, so they must explicitly call these functions to participate in layerwise KV transfer. 2. **Attention utils (`attention/utils.py`)**: Add `connector.has_connector_metadata()` guard to `wait_for_kv_layer_from_connector` and `maybe_save_kv_layer_to_connector`, matching the guard in the `@maybe_transfer_kv_layer` decorator. Prevents spurious `current_layer` counter increments during non-save steps (e.g. profile run). 3. **Pool scheduler (`pool_scheduler.py`)**: Skip `touch_sending_mamba_blocks` when `use_layerwise=True`. The layerwise send thread (`KVCacheStoreLayerSendingThread`) does not use the `completed_events` mechanism, so touched mamba blocks would never be freed, leaking the block pool. 4. **Postprocess kernel (`ops/triton/mamba/postprocess.py`)**: Update `postprocess_mamba_fused_kernel` signature to match vLLM v0.25.0, adding `state_dim_row_count_ptr`, `state_dim_row_stride_ptr`, `idx_mapping_ptr`, `CONV_STATE_DIM_FIRST`, `HAS_IDX_MAPPING`, and `PRECOMPUTED_NEW_COMPUTED` parameters. Preserves the triton-ascend pointer-type-cast optimization. ## Testing Verified on Atlas 800T A2 (Ascend 910B3, 8 NPU) with Qwen3.5-9B: - Service starts successfully with `use_layerwise=true` and MemCache backend - KV cache hit confirmed: 4 groups all hit 512 tokens, External prefix cache hit rate 22.9% - 5 concurrent requests all succeed with cache hits - MTP speculative decoding (`num_speculative_tokens=3, method=qwen3_5_mtp`) works correctly --- (Replaces vllm-project#12526, which was locked-closed after a force-push to the head branch; this PR is based on the clean single-commit feature.) - vLLM main: vllm-project/vllm@ba07e4a --------- Signed-off-by: tyy0829 <tyy0829@users.noreply.github.com> Signed-off-by: tyy0829 <1455207791@qq.com> Co-authored-by: tyy0829 <tyy0829@users.noreply.github.com>
Description
This PR adds layerwise KV pool support for Qwen3.5 (and Qwen3-Next) hybrid models that combine GDN (Gated Delta Net) linear attention layers with standard full attention layers.
Changes
GDN forward (
ops/gdn.py): Addwait_for_kv_layer_from_connector(self.prefix)at the start offorward()andrecord_attention_compute_start()before the custom op call. GDN layers do not go through the@maybe_transfer_kv_layerdecorator, so they must explicitly call these functions to participate in layerwise KV transfer.Attention utils (
attention/utils.py): Addconnector.has_connector_metadata()guard towait_for_kv_layer_from_connectorandmaybe_save_kv_layer_to_connector, matching the guard in the@maybe_transfer_kv_layerdecorator. Prevents spuriouscurrent_layercounter increments during non-save steps (e.g. profile run).Pool scheduler (
pool_scheduler.py): Skiptouch_sending_mamba_blockswhenuse_layerwise=True. The layerwise send thread (KVCacheStoreLayerSendingThread) does not use thecompleted_eventsmechanism, so touched mamba blocks would never be freed, leaking the block pool.Postprocess kernel (
ops/triton/mamba/postprocess.py): Updatepostprocess_mamba_fused_kernelsignature to match vLLM v0.25.0, addingstate_dim_row_count_ptr,state_dim_row_stride_ptr,idx_mapping_ptr,CONV_STATE_DIM_FIRST,HAS_IDX_MAPPING, andPRECOMPUTED_NEW_COMPUTEDparameters. Preserves the triton-ascend pointer-type-cast optimization.Testing
Verified on Atlas 800T A2 (Ascend 910B3, 8 NPU) with Qwen3.5-9B:
Service starts successfully with
use_layerwise=trueand MemCache backendKV cache hit confirmed: 4 groups all hit 512 tokens, External prefix cache hit rate 22.9%
5 concurrent requests all succeed with cache hits
MTP speculative decoding (
num_speculative_tokens=3, method=qwen3_5_mtp) works correctlyvLLM version: v0.25.1
vLLM main: vllm-project/vllm@54503ec