Skip to content

patches: keep a promoted draft block a multiple of the layer's kernel granularity - #142

Merged
mhenrichsen merged 2 commits into
syv-ai:mainfrom
TyroneNel:fix/sw-block-promote-granularity
Sep 22, 2026
Merged

mhenrichsen merged 2 commits into
syv-ai:mainfrom
TyroneNel:fix/sw-block-promote-granularity

Conversation

@TyroneNel

@TyroneNel TyroneNel commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

What

hybrid-sw-block-promote promotes a draft sliding-window layer's block to the smallest divisor of the primary block that still covers that layer's page. This change adds one condition to the search: the divisor must also be a whole multiple of the layer's own spec.block_size — 16 for the SW drafter. The no-divisor fallback is unchanged.

Why

A divisor can satisfy the scheduler's LCM invariant and still break the layer's own kernel granularity. vLLM states the rule itself: a framework-level block size must be a multiple of the backend's kernel requirement — Backend.supports_block_size (v1/attention/backend.py, the MultipleOf branch: "the framework-level block size only needs to be a multiple of the kernel's requirement"). Upstream's unify_kv_cache_spec_page_size cannot violate it, because it only ever scales by whole ratios (new_block_size = layer_spec.block_size * ratio). Replacing that ratio with an arbitrary divisor of the primary block can.

Concrete case, derived from the code, no hardware needed — primary block 1840, covering block 96:

  • 1840 % 96 != 0, so the promotion runs;
  • divisors of 1840 at or above 96 start at 115;
  • 115 % 16 == 3, so the promoted block is not a multiple of the drafter's 16-token kernel granularity.

The same search can also land on 460 or 920 of 1840 (460 % 16 == 12, 920 % 16 == 8).

Verification

bash patches/check_vllm_series.sh <pristine vLLM v0.28.0 checkout>:

== pass 1: the whole series, GNU patch, patches/series order
   38 patches applied with exact context, 0 of them at an offset, 0 with fuzz
== pass 2: the ordered DFlash patches, git apply --check
patch integrity: OK

Both hunk headers in the touched file are regenerated to match the added lines (@@ -1033,6 +1033,135 @@ → +1033,143, @@ -1060,6 +1189,12 @@ → +1197,12), so the file parses and applies cleanly.

Limits

No hardware failure has been observed for this geometry; the argument is invariant-based — the promotion could emit a block size that supports_block_size would reject. The fallback is untouched: when no divisor qualifies, the config is left alone and the layer's page is padded at block 16 (expensive but boots).

Review follow-up: what spec.block_size stands for

The condition reads the layer's current framework block, which is the backend's kernel granularity only because nothing has promoted this layer yet — it is not get_supported_kernel_block_sizes(). The patch now says so, since the two diverge for a layer whose block was already promoted upstream of this call, and this condition would then be the weaker of the two.

Hunk headers regenerated for the five comment lines: -1033,6 +1033,143 becomes +1033,148, and the second hunk's start becomes 1060 + (148-6) = 1202. Counted against the emitted body, and the git-apply job is green on the rebuilt branch.

The boot on the reference box (that the shipped CTX=fast / CTX=long / CTX=huge geometries come out unchanged) is still yours to run — nothing here changes that.

… granularity

The promotion picks the smallest divisor of the primary block that still
covers the draft layer's page. A divisor can satisfy the scheduler's LCM
invariant and still break the layer's own kernel granularity: vLLM requires a
framework-level block size to be a multiple of the backend's kernel
requirement (Backend.supports_block_size, the MultipleOf branch), and
upstream's unify_kv_cache_spec_page_size cannot violate that because it scales
by whole ratios only.

Derived from the code, no hardware needed: primary block 1840 with a covering
block of 96 runs the promotion, the smallest divisor of 1840 at or above 96 is
115, and 115 % 16 == 3 -- not a multiple of the SW drafter's 16-token kernel
granularity. 460 and 920 of 1840 fail the same way.

Adds the constraint to the divisor search and regenerates the two affected
hunk headers. The no-divisor fallback is unchanged: log and leave the config
alone rather than risk the scheduler.

Verified: bash patches/check_vllm_series.sh against a pristine v0.28.0
checkout -- 38 patches applied with exact context, 0 offsets, 0 fuzz; the five
contractual DFlash patches pass git apply --check; patch integrity OK.
@mhenrichsen

mhenrichsen commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Reproduced your verification independently, on a fresh git clone --depth 1 --branch v0.28.0 of vLLM (nothing of ours in the tree):

== pass 1: the whole series, GNU patch, patches/series order
   38 patches applied with exact context, 0 of them at an offset, 0 with fuzz
== pass 2: the ordered DFlash patches, git apply --check
patch integrity: OK

Both regenerated hunk headers check out by hand as well: hunk 1 is -1033,6 +1033,143 against 6 context / 143 emitted lines, hunk 2 is -1060,6 +1197,12 and 1060 + (143-6) = 1197.

The invariant is quoted correctly too — vllm/v1/attention/backend.py in the pristine 0.28.0 tree, supports_block_size, lines 184-190:

if isinstance(supported_size, MultipleOf):
    supported_size = supported_size.base
# With hybrid_blocks feature, the framework-level block size
# only needs to be a multiple of the kernel's requirement,
# even if the kernel requires a fixed block_size.
if block_size % supported_size == 0:
    return True

and your worked case holds: 1840 = 2^4 * 5 * 23, so its divisors from 96 up are 115, 184, 230, 368, 460, 920, 1840, of which only 368 and 1840 are multiples of 16 (184, 230, 460 and 920 are not). Today the search returns 115 and the added condition moves it to 368: still a divisor, now also a legal block size, at the cost of a larger page.

Two notes rather than objections:

  1. The condition uses spec.block_size as a stand-in for the backend's kernel requirement. That is right for the SW drafter at 16, but it is the layer's current framework block, not get_supported_kernel_block_sizes(). Worth a sentence in the patch comment saying so, since the two could diverge for a layer whose block was already promoted upstream.
  2. Nothing here changes a geometry the repo currently ships — a non-multiple-of-16 promotion would have been rejected at boot, so the configs in the README cannot be landing on one. I still want one boot on the reference box to confirm the shipped CTX=fast / CTX=long / CTX=huge geometries come out unchanged before this merges; the GPU is on a training job right now, so that is a day or two out, not a review objection.

Related history, if you want the context: #63 is where the divide-the-primary-block rule came from (int4 k=15 would not boot; k=7 divided by luck), so this is the second half of the same invariant.

The added condition reads the layer's current framework block, which is the
backend's kernel granularity only because nothing has promoted this layer yet
-- it is not get_supported_kernel_block_sizes(). Write that down in the patch,
since the two diverge for a layer whose block was already promoted upstream of
this call. Hunk headers regenerated for the five comment lines (1033,143 ->
1033,148; second hunk start 1060 + (148-6) = 1202).
@TyroneNel

Copy link
Copy Markdown
Contributor Author

Added the sentence — and thank you for reproducing the series on a pristine 0.28.0 tree and checking the hunk arithmetic by hand; that is more than I had any right to expect.

The patch comment now says that spec.block_size is the layer's current framework block, not get_supported_kernel_block_sizes(): it is the kernel granularity here only because nothing has promoted this layer yet, and for a layer whose block was already promoted upstream of this call the two diverge, at which point this condition is the weaker of the two.

Hunk headers regenerated for the five comment lines: -1033,6 +1033,143 → +1033,148, second hunk start 1060 + (148-6) = 1202. Counted against the emitted body rather than trusted, and the git-apply job is green on the rebuilt branch.

Your point 2 is still yours: nothing here changes a shipped geometry, and the boot that confirms CTX=fast / CTX=long / CTX=huge come out unchanged is the thing this is waiting on. No rush from my side.

Thanks for the #63 pointer — that is the other half of the invariant and I had not connected them.

@mhenrichsen

Copy link
Copy Markdown
Contributor

Booted on the reference box. Every shipped geometry comes out identical to the token, which is what this was waiting on.

Same box, same tree, same checkpoint; the only change between arms was swapping the installed hybrid-sw-block-promote.patch for this branch's (reverse-apply the old, apply yours at --fuzz=0, confirmed with _check_applied.py and the new condition on line 1132). Profile defaults, SPEC=dflash2 PREFIX_CACHE=1:

profile main this PR
CTX=fast (k=7) block 448, pad 3.23%, 68,605 tok block 448, pad 3.23%, 68,605 tok
CTX=long (int8 KV) block 864, pad 1.09%, 136,429 tok block 864, pad 1.09%, 136,429 tok
CTX=huge (KVarN) block 2176, pad 2.82%, 268,169 tok block 2176, pad 2.82%, 268,169 tok
CTX=fast DFLASH_TOKENS=15 block 480, pad 1.27%, 57,669 tok block 480, pad 1.27%, 57,669 tok

The last row is the one I cared about most. DFLASH_TOKENS=15 is where the divisor search actually engages — it is the #63 case, where the promoted drafter block has to divide the primary rather than merely cover it — and it is also what production runs. So the tightened condition picks the same divisor the old one did on every configuration this repo ships, and only changes the answer on geometries like your 1840 example, where the old answer was wrong.

All three main pools also reproduce the numbers the launcher's own comments cite (136,429 at CTX=long, 268,169 at CTX=huge), so the baseline arm is the documented one and not a drifted local tree.

Merging. Thank you for arguing this from the invariant rather than waiting for a crash — it is exactly the class of bug that would have surfaced on someone else's card, at a geometry nobody here runs, as an illegal memory access with no pointer back to this function.

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.

2 participants