Skip to content

[https://nvbugs/6796296][fix] Pad DSv4 NVFP4 compressed FP8 scratch by one FMHA KV tile - #20119

Merged
peaceh-nv merged 2 commits into
NVIDIA:mainfrom
peaceh-nv:dev-peaceh-unwaive-dsv4-flash-nvfp4-kv
Oct 23, 2026
Merged

peaceh-nv merged 2 commits into
NVIDIA:mainfrom
peaceh-nv:dev-peaceh-unwaive-dsv4-flash-nvfp4-kv

Conversation

@peaceh-nv

@peaceh-nv peaceh-nv commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

Description
@coderabbitai summary

Description

Problem. TestDeepSeekV4Flash::test_nvfp4_kv (DGX_B300-4_GPUs-PyTorch-1) fails intermittently with CUDA 700 (illegal memory access) / KVCM2 poisoned. It usually fails during generation CUDA graph warmup or at the start of GSM8K serving, in about 15% of local runs. It was waived in #19393.

Root cause. The trtllm-gen DSv4 dynamic sparse MLA FMHA (fmhaSm100fKernel_QkvE4m3...DenseDynamicTokenSparse...) reads its aux KV pool up to one KV tile (128 rows x 512 B = 64 KB) past the rows named in the sparse indices, for KV tiles >= 1 (the compressed part). Both the static generation variant and the persistent variant used for context do this. The aux
pool is aux_kv_cache_pool_ptr, whose TMA descriptor tmaK_ is built with INT_MAX rows.

  • With FP8 KV the aux pool is the whole compressed KV pool, so these reads stay in bounds.
  • With NVFP4 KV, DeepseekV4TrtllmAttention.sparse_attn_predict dequantizes the selected compressed tokens into a per-call FP8 scratch sized to the batch (e.g. 32 rows = 16 KB for a ratio-128 context call) and passes it as the aux pool. When the caching allocator places that scratch at the end of a mapped segment, the over-read hits unmapped VA, which gives an MMU
    fault (Xid 31) and CUDA 700. Placement depends on allocator history, which is why the failure is intermittent. Syncing before the FMHA, disabling PDL, single-stream execution and fresh workers did not prevent it.

Evidence.

  • GPU coredumps of local crashes: every faulting CTA's TMA warp stops at the first tile-1 aux gather. The scratch ends at, or 20-64 KB before, the end of mapped memory. The kernel params and the tile-1 page offsets in shared memory (rows 0-1) are in range.
  • Diagnostic run: each scratch was copied to the end of its own VMM mapping, with unmapped VA after it. The FMHA then faults deterministically on all 4 ranks at the first 512-token context warmup. The fault is always at row 128 of a 32-row ratio-128 scratch (Xid 31 address = scratch base + 0x10000), although the indices for that call only reference rows 0-3.

Fix. Allocate the NVFP4 compressed FP8 scratch with a zeroed tail of one FMHA KV tile (128 rows), in both the generation and the context gather paths (_alloc_nvfp4_compress_fp8_scratch). The gather ops still see exactly the logical scratch shape. The cost is 64 KB per scratch and one small memset per layer call. The kernel-side over-read will be reported to
the trtllm-gen FMHA owners. It could in principle also reach past the end of the FP8 compressed pool; this PR only covers the NVFP4 scratch.

Also unwaives accuracy/test_llm_api_pytorch.py::TestDeepSeekV4Flash::test_nvfp4_kv.

Test Coverage

  • accuracy/test_llm_api_pytorch.py::TestDeepSeekV4Flash::test_nvfp4_kv (unwaived; DGX_B300-4_GPUs-PyTorch-1).
  • Local 4x DGX B300 runs in the CI container, using the post-merge wheel plus this patch, with a warm autotuner cache as in CI:
    • With the fix: 20/20 passed (GSM8K 94.73-95.68; reference 95.11, threshold 91.91). Without it, about 15% of runs crashed.
    • With the fix and the diagnostic above: 20/20 passed (GSM8K 94.62-95.72). Without the fix this setup crashed on every rank at the first warmup.

PR Checklist

Please review the following before submitting your PR:

  • PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.

  • PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.

  • Test cases are provided for new code paths (see test instructions)

  • If PR introduces API changes, an appropriate PR label is added - either api-compatible or api-breaking. For api-breaking, include BREAKING in the PR title.

  • Any new dependencies have been scanned for license and vulnerabilities

  • CODEOWNERS updated if ownership changes

  • Documentation updated as needed

  • Update tava architecture diagram if there is a significant design change in PR.

  • The reviewers assigned automatically/manually are appropriate for the PR.

  • Please check this after reviewing the above items as appropriate for this PR.

@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between 89e4ebe and 29dffe7.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: NVIDIA/TensorRT-LLM/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 7e8aa824-c7d6-4931-a3a9-ec61ba315be4
📥 Commits

Reviewing files that changed from the base of the PR and between a4df3e8 and 89e4ebe.

📒 Files selected for processing: 2
  • tensorrt_llm/_torch/attention/backends/sparse/deepseek_v4/backend.py
  • tests/integration/test_lists/waives.txt
💤 Files with no reviewable changes: 1
  • tests/integration/test_lists/waives.txt

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Walkthrough

The DeepSeek V4 sparse attention backend now uses padded FP8 scratch storage for NVFP4 generation and context gathers. The related DeepSeek V4 Flash accuracy-test waiver was removed.

Changes

DeepSeek V4 NVFP4 scratch handling

Layer / File(s) Summary
Pad NVFP4 scratch storage
tensorrt_llm/_torch/attention/backends/sparse/deepseek_v4/backend.py
Added a shared allocator that adds and zeroes a 128-row FP8 tail. Generation and context gather paths now use the padded allocation and pass requested rows through reshaped views.
Remove the test waiver
tests/integration/test_lists/waives.txt
Removed the TestDeepSeekV4Flash::test_nvfp4_kv skip waiver and its NVBug 6796296 reference.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 89e4e

No actionable issue is established in the scratch-padding change or test-waiver removal. Merge readiness remains subject to normal CI checks.

Pre-merge checks: Passed 5
Check name Status Explanation
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly identifies the tracked bug and the main code change: padding DSv4 NVFP4 compressed FP8 scratch by one FMHA KV tile. It is specific and related to the pull request changes.
Description check Passed The pull request description is complete and relevant. It explains the intermittent failure, root cause, fix, scope, test coverage, and checklist status. The description does not include the required …
Full details: Description check

Explanation

The pull request description is complete and relevant. It explains the intermittent failure, root cause, fix, scope, test coverage, and checklist status. The description does not include the required ticket/type title format, but this is a minor omission because the body satisfies the required sections.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@xxi-nv xxi-nv added the ci: post-merge approved Approved by TRT-LLM CI approvers for broad post-merge CI requests label Oct 12, 2026
@github-actions

Copy link
Copy Markdown

Automatically added "ci: full pre-merge approved" because this PR has satisfied the required GitHub review approvals. Unresolved review conversations and other required checks remain independent merge requirements.

@peaceh-nv

Copy link
Copy Markdown
Collaborator Author

/bot run --stage-list "DGX_B300-4_GPUs-PyTorch-1"

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #77192 [ run ] triggered by Bot. Commit: a4df3e8 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #77192 [ run ] completed with state SUCCESS. Commit: a4df3e8
/LLM/main/L0_MergeRequest_PR pipeline #63635 (Partly Tested) completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

@peaceh-nv

Copy link
Copy Markdown
Collaborator Author

/bot run --stage-list "DGX_B300-4_GPUs-PyTorch-1"

@yuxianq

yuxianq commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

@heyuhhh, could you help to review this PR as trt-llm-torch-attention-devs? Thanks~

Assignment explanation
Componentsbackends/sparse (100%)
Tagsalgorithm:sparse (100%); specific-algorithm:deepseek-v4 (100%); representation:mla (100%); phase:decode (50%); phase:prefill (50%); features:quantization (100%)
PR effort5 points — The runtime change fixes scratch allocation despite the unwaive-focused title. Review requires validating the kernel read bound, compatibility of the new two-dimensional allocation with gather views and downstream consumers, and padding initialization across both execution paths.
ImportanceImportant — The allocator adds 128 zeroed rows to accommodate stated sparse MLA FMHA reads beyond indexed KV rows. This changes a sensitive GPU memory-safety contract in both generation and context/mixed execution.
Affinity (0–1)0.378 component/tag match; 0.284 after PIC preference
Outstanding points at selection9 → 9 (+0)
Points already reserved on this PR5
Effective weight0.5
Completed review points (preceding 168 hours)6
Combined points after assignment15
Demand-derived target15.00 points (soft share)
Additional fair share before assignment0.00 points
Share explanationWithin demand-derived share.
Selection ruleaffinity with 0.3-share guard
Assignment scope (heyuhhh)sparse_attention

Affinity and workload are snapshots at selection, not live counters. Existing reservations on this PR are counted once.

@yuxianq
yuxianq removed the request for review from lfr-0531 October 21, 2026 05:38
@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #79540 [ run ] triggered by Bot. Commit: 89e4ebe Link to invocation

@peaceh-nv peaceh-nv changed the title [https://nvbugs/6796296][test] Unwaive TestDeepSeekV4Flash::test_nvfp4_kv [https://nvbugs/6796296][fix] Pad DSv4 NVFP4 compressed FP8 scratch by one FMHA KV tile Oct 21, 2026
@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #79540 [ run ] completed with state SUCCESS. Commit: 89e4ebe
/LLM/main/L0_MergeRequest_PR pipeline #65498 (Partly Tested) completed with status: 'SUCCESS'

CI Report

Link to invocation

@peaceh-nv

Copy link
Copy Markdown
Collaborator Author

@peaceh-nv

Copy link
Copy Markdown
Collaborator Author

/bot run

@peaceh-nv

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #79605 [ run ] triggered by Bot. Commit: 89e4ebe Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #79606 [ run ] triggered by Bot. Commit: 89e4ebe Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #79605 [ run ] completed with state ABORTED. Commit: 89e4ebe

Link to invocation

@peaceh-nv
peaceh-nv enabled auto-merge (squash) October 21, 2026 10:44
@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #79606 [ run ] completed with state SUCCESS. Commit: 89e4ebe
/LLM/main/L0_MergeRequest_PR pipeline #65545 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

…ch by one FMHA KV tile

The trtllm-gen DSv4 dynamic sparse MLA FMHA reads its aux KV pool up to
one KV tile (128 rows) past the rows named in the sparse indices. With
FP8 KV the aux pool is the whole compressed KV pool, so those reads stay
in bounds. With NVFP4 KV the aux pool is a per-call FP8 scratch sized to
the batch, so the FMHA faults (CUDA 700, KVCM2 poisoned) whenever the
caching allocator places that scratch at the end of a mapped segment.

Placing every scratch at the end of its own VMM mapping makes the fault
deterministic: the first 512-token context warmup faults on every rank,
always at scratch row 128 of a 32-row ratio-128 scratch.

Allocate the scratch with a zeroed tail of one KV tile in both the
generation and context gather paths.

Signed-off-by: peaceh <103117813+peaceh-nv@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…4_kv

Signed-off-by: peaceh <103117813+peaceh-nv@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@peaceh-nv
peaceh-nv force-pushed the dev-peaceh-unwaive-dsv4-flash-nvfp4-kv branch from 89e4ebe to 29dffe7 Compare October 22, 2026 07:11
@peaceh-nv

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #79842 [ run ] triggered by Bot. Commit: 29dffe7 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #79842 [ run ] completed with state SUCCESS. Commit: 29dffe7
/LLM/main/L0_MergeRequest_PR pipeline #65725 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

@peaceh-nv

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #80006 [ run ] triggered by Bot. Commit: 29dffe7 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #80006 [ run ] completed with state SUCCESS. Commit: 29dffe7
/LLM/main/L0_MergeRequest_PR pipeline #65848 completed with status: 'SUCCESS'
Pipeline passed with automatic retried tests. Check the rerun report for details.

CI Report

Link to invocation

@peaceh-nv
peaceh-nv merged commit 36d8aef into NVIDIA:main Oct 23, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci: full pre-merge approved ci: post-merge approved Approved by TRT-LLM CI approvers for broad post-merge CI requests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants