fix(vulkan): the 27B load held the model TWICE on a unified box (#203) - #204
Merged
Conversation
FOLLOWING_AGENTS_PROTOCOL
THE BUG. Loading Qwen3.6-27B bf16 (50.89 GiB) on a Vulkan GB10 could take
the machine down rather than fail -- NVRM NV_ERR_NO_MEMORY out of
_memdescAllocInternal, twice in one day. MEASURED cause, same binary,
VT_ADOPT_DEVICE_BYTES A/B on GB10:
OFF VmRSS 100.759 GiB at 50.755 GiB of Vulkan allocation, MemAvailable
13.85 / MemFree 1.13 of 119.6 GiB, and still allocating (863 of 894
buffers) when the harness killed it.
ON completes at VmHWM 53.413 GiB, MemAvailable never below 47.3 GiB.
The gap is a FLAT ~50.0 GiB across every high-water line -- one whole extra
copy of the model. ResidentWeight uploaded each weight and kept `bytes`
too. Vulkan allocates every buffer HOST_VISIBLE|HOST_COHERENT and
persistently mapped (its Copy/Memset are already a plain host
memcpy/memset over that pointer), so on unified memory both copies come
out of the same RAM. platforms/vulkan.cpp had REASONED that "there is
exactly one copy of the bytes"; this makes that true.
WHAT WAS RULED OUT, with numbers. Requested bytes == driver-committed
bytes EXACTLY in every run (50.756 GiB over 894 buffers at 27B), so there
is no per-allocation rounding, no allocator excess, and no staging or
dequant scratch to recover. The windowed source-page release DOES fire:
process RSS during load holds one copy, not the mmap as well. Page cache
tracks the copied bytes 1:1 and is not dropped, but it is reclaimable --
MemAvailable stayed healthy -- so it is not the OOM cause.
THE FIX is an ADOPTION, not a free: `bytes` is re-pointed AT the device
allocation through the existing OwnedBytes borrow, keyed alive by d_dev's
own control block, so every `.bytes` reader -- the f32 upcast, the
portable CPU reference tier, View/Numel -- reads the SAME bytes from the
surviving copy. Nothing is dropped, so unlike ReleaseHost this needs no
"is the device path committed" proof.
Backend::DeviceMemoryIsHostAddressable() is the gate and defaults false,
so every discrete-GPU path is byte-identical. It is deliberately narrower
than UnifiedMemory(): CUDA on GB10 is unified yet a cudaMalloc pointer is
still not host-dereferenceable.
Also lands the accounting that made the attribution possible
(VT_VULKAN_ALLOC_STATS, counters always maintained so a test can assert
on them) and three mutation-checked mechanism tests.
GATES on GB10, Vulkan build (VLLM_CPP_VULKAN=ON, CUDA absent so
CurrentPlatform resolves kVULKAN -- the engine selected device type 3):
test_opt_paged_engine 6/6 prompts token-exact (96/96 tokens), 0 declines;
test_backend_cross_device 11/11 (132); test_vulkan_backend 35/35
(2650/2650); test_qwen36_weights adoption cases 3/3 (20), red under all
three mutations (no-op body, dropped host-addressable guard, dropped
keep-alive -- the last a real use-after-free).
Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: Claude-Code:claude-opus-5 [Claude Code]
The new intake rule requires an open issue before a row is claimed, linked from the roadmap, the row's spec and the PR. No open issue covered the Vulkan double-copy at load -- #83 is memory BUDGETING, a different thing -- so #203 was opened for it and is linked here. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: Claude-Code:claude-opus-5 [Claude Code]
mudler
added a commit
that referenced
this pull request
Aug 9, 2026
…message Main CI has been red on EVERY merged PR. Two independent defects, both landed by #178's push (run 31332846716); `cuda-fat-build` in that run was cancelled by concurrency, not failing. THE GATE REDDENED MAIN FOR OBEYING IT. `check-role-discipline.py` judges every commit in the `before..after` range on its own message. A PR landed with a REAL merge commit pushes the merge AND the branch commits under it: the merge names the PR, the branch commits were never required to, so each one read as a direct push. `6603356a` (#178), `e73cbbae` (#204) and `1a02ab4f` (#196) all failed this way, in both `documentation-checkpoint` and `agent-record`. Arrival is now judged ONCE, on the commit that lands the change: `merged_pr_content` exempts what a row/* PR merge brings in. Squash-merges are untouched -- their one commit carries "(#N)" and passes on its own message. NOT a weakening, and gated as such: only the SIDE parents count, so `--not parents[0]` keeps a commit pushed straight to main from being laundered by merging a PR on top, and a merge naming no row and no PR exempts nothing. Four unit checks build real git history for those cases, plus the exact `3bbee96e..0cf3dbb` range CI ran, pinned with `has_reached_main` forced TRUE -- from a `row/*` worktree everything reports as pending PR disposition, and the test would have passed against the defect it exists to catch. Suite 47/47, and 5/5 red without the fix. A STALE MESSAGE IN A TEST. `6603356a` taught `LoadMergedBf16RawNK` to accept F8_E4M3 shards and rewrote its rejection to name the supported dtypes; `test_qwen27_dense_forward.cpp:229` still asserted the old "expected BF16", so `build-test-cpu` and both sanitizer legs failed on it. The expectation now reads the message the loader raises, and the FP8 merge path that arrived WITHOUT a test in this file gets one: a mixed BF16+FP8 merged parameter, expectations hand- computed from E4M3 bytes and the scale (never re-derived through the same dequant helper the loader calls), plus the per-channel-scale rejection. 7/7, 333 assertions. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: Claude:claude-opus-5 [ClaudeCode]
mudler
added a commit
that referenced
this pull request
Aug 9, 2026
…message (#210) Main CI has been red on EVERY merged PR. Two independent defects, both landed by #178's push (run 31332846716); `cuda-fat-build` in that run was cancelled by concurrency, not failing. THE GATE REDDENED MAIN FOR OBEYING IT. `check-role-discipline.py` judges every commit in the `before..after` range on its own message. A PR landed with a REAL merge commit pushes the merge AND the branch commits under it: the merge names the PR, the branch commits were never required to, so each one read as a direct push. `6603356a` (#178), `e73cbbae` (#204) and `1a02ab4f` (#196) all failed this way, in both `documentation-checkpoint` and `agent-record`. Arrival is now judged ONCE, on the commit that lands the change: `merged_pr_content` exempts what a row/* PR merge brings in. Squash-merges are untouched -- their one commit carries "(#N)" and passes on its own message. NOT a weakening, and gated as such: only the SIDE parents count, so `--not parents[0]` keeps a commit pushed straight to main from being laundered by merging a PR on top, and a merge naming no row and no PR exempts nothing. Four unit checks build real git history for those cases, plus the exact `3bbee96e..0cf3dbb` range CI ran, pinned with `has_reached_main` forced TRUE -- from a `row/*` worktree everything reports as pending PR disposition, and the test would have passed against the defect it exists to catch. Suite 47/47, and 5/5 red without the fix. A STALE MESSAGE IN A TEST. `6603356a` taught `LoadMergedBf16RawNK` to accept F8_E4M3 shards and rewrote its rejection to name the supported dtypes; `test_qwen27_dense_forward.cpp:229` still asserted the old "expected BF16", so `build-test-cpu` and both sanitizer legs failed on it. The expectation now reads the message the loader raises, and the FP8 merge path that arrived WITHOUT a test in this file gets one: a mixed BF16+FP8 merged parameter, expectations hand- computed from E4M3 bytes and the scale (never re-derived through the same dequant helper the loader calls), plus the per-channel-scale rejection. 7/7, 333 assertions. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: Claude:claude-opus-5 [ClaudeCode] Co-authored-by: Ettore Di Giacinto <mudler@localai.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #203.
On a unified-memory box the Vulkan backend held two full copies of the
weights. For Qwen3.6-27B that is an extra 50 GiB, and it hard-rebooted this
GB10 twice in one session with
NVRM ... NV_ERR_NO_MEMORY @ mem_desc.c:1359.Measured
Qwen3.6-27B bf16 (50.89 GiB), GB10, one binary,
VT_ADOPT_DEVICE_BYTESA/B:The gap is a flat 50.003 GiB across the last four high-water lines — one
whole extra copy of the model. Qwen3-4B: VmHWM 16.392 → 9.607 GiB. The unfixed
27B never completed, so 100.759 GiB is a lower bound.
Cause
ResidentWeightuploaded each weight and keptOwnedTensor.bytesas well.Vulkan allocates every buffer
HOST_VISIBLE | HOST_COHERENT | DEVICE_LOCALandpersistently mapped, so on unified memory both copies are the same RAM.
src/vllm/platforms/vulkan.cpphad reasoned "there is exactly one copy of thebytes";
dense_attn_block.h:190made a second one.The investigation refuted my own first diagnosis
I briefed this row with "the Vulkan loader lacks the CUDA path's shard release".
That was wrong, and the row was told not to build on it. Windowed release is
not the defect and it does fire: during load the RSS holds one copy, not the mmap
as well. The other candidates were measured and cleared too:
requested == committedexactly, every run — 50.756GiB over 894 buffers. Nothing to win from suballocation, and
maxMemoryAllocationCountis nowhere near 894.bump; the excess is a constant equal to the model.
reclaimable. (Worth recording:
MADV_DONTNEEDon a private file mapping doesnot evict page cache — that needs
POSIX_FADV_DONTNEED.)A first watchdog keyed on
MemFreekilled a healthy load at 11.46 GiB free whileMemAvailablewas 60.65 — recorded as a negative result.Fix
AdoptDeviceBytesAsHostre-pointsbytesat the device allocation via theexisting
OwnedBytesborrow, keyed alive byd_dev's control block. An adoption,not a release: every
.bytesreader sees the same bytes, so unlikeReleaseHostit needs no "device path committed" proof. Gated on a new
Backend::DeviceMemoryIsHostAddressable(), default false, deliberatelynarrower than
UnifiedMemory()— CUDA on GB10 is unified yetcudaMallocis nothost-dereferenceable. Discrete and non-Vulkan paths are byte-identical.
Gates, operator-verified on GB10 with the real driver
All four arms (
VT_ADOPT_DEVICE_BYTESxVT_VULKAN_INFLIGHT), re-run by merather than taken on report:
test_vulkan_backend2650/2650 assertions in all fourtest_opt_paged_engine6/6 prompts token-exact (96/96), 0 declines, in allfour, on device type 3
test_backend_cross_device11/11 (132)On llvmpipe from a clean build:
test_vulkan_backend35/35 (2107),test_backend_cross_device11/11,test_opt_paged_engine6/6 token-exact ondevice type 3, and the three new mechanism tests 7/7 (45 assertions).
gen-vulkan-spirv.py --checkclean.The row's three mechanism tests each go red under mutation — no-op body (3
assertions), dropped guard (3), dropped keep-alive (2, a real use-after-free
reading 128 where 167 was written).
A trap this uncovered, worth knowing repo-wide
VLLM_CPP_DEVICEis read nowhere in the tree. The engine selects viaCurrentPlatform(). The campaign's recorded gate commandVLLM_CPP_DEVICE=vulkan test_opt_paged_enginehas been selecting Vulkan onlybecause those builds had no CUDA compiler; with CUDA on PATH the identical
command reports
device type 1and passes 6/6 on CUDA. Every Vulkan gate inthis PR was therefore checked for
device type 3explicitly, and so were thebarrier gates in #198 (confirmed type 3 in all four arms). The env var gives
false confidence and the docs should stop implying it selects anything.
Left open, named
Peak is now the load phase: the host build reaches ~51 GiB before the first
upload. Copying from the mmap straight into the device buffer would cut that too.
Not tested
No CUDA or Metal run — argued byte-identical from the default-false gate, not
measured. Only Qwen3-4B and Qwen3.6-27B.
test_qwen36_weightshas onepre-existing failure on a CUDA-less build (
in_proj_qkv_fp8behind#ifdef VT_CUTLASS_FP8) that is unrelated and touches no Backend.FOLLOWING_AGENTS_PROTOCOL