[bong-water-water-bong]

The in-process NPU route (1bit serve -m <q4nx model dir> --device npu, app/unified.cpp) took no
cross-process claim on the array, while 1bit forward-serve has held the exclusive flock on
/tmp/1bit-npu-device.lock for the life of the process since engine#612. That route is the shipped
front door: Lemonade's onebit backend runs exactly 1bit serve -m <model dir> --port N
(lemonade …/backends/onebit/onebit_server.cpp), and the payload's own logs say where it lands
(1bit unified: on the private route dx: dense Qwen3 on the dx kernels).

So two such processes — a backend the watchdog replaced before the old one exited, a second client, a
benchmark — each opened a hardware context on the array and dispatched at the same instant. That shape is
#1084's measured wedge: two serves of the c24 payload, both npu runtime: invalid call (code 614): state 8, run 0 of 28, one TDR with DPU PC 0xffffffff, and hwctx_num = 2 across the round
(hrx-study/evidence/fabric/npu-longctx/two-client-20261105-034608). Neither serve's log carries a
device-lock line, because on this route there was no lock to take. It is also the same engine-side lever
pi-hrx-fabric asked about at 04:20 ("the engine cannot see it"): the engine can, and the route simply never
took it.

The demo's own shape is this one: switching between the picture page and a chat makes Lemonade unload one NPU
backend and load another, and the replacement can start before the old one has exited.

The fix

app/unified.cpp takes the same exclusive claim, before it opens the model (opening a context is the other
half of what the lock protects), and holds the fd for the life of the process. A second backend now waits at
the lock instead of dispatching on a live context, and takes the device when this one exits — which the
exit-1-on-a-failed-runlist path makes it do, so Lemonade's reload gets the array. NPU_NO_DEVICE_LOCK=1
still skips it (the measurement escape hatch).

--npu-streams is refused by name on this route and on the fused dx route (also in process): one
context, exclusive lock, so N streams there could only queue, and a silently ignored flag is what
docs/npu.md's own rule forbids. docs/npu.md now says which route takes which mode, and its two-stream
example is forward-serve, where streams actually work — the old example was a .q4nx model directory,
i.e. this same in-process route, where the flag never streamed anything.

Measured (strixhalo, two npu-hold -l npufeat -t 15 tickets, dx route, shipped c25 kernel set)

Model ~/models/npu/Qwen3-0.6B. The "lock held elsewhere" arms use a plain flock holder — a file lock, not a
second device user — so at most one real context is ever live. Each row is a serve that was either killed by a
12 s timeout or by the arm itself, so nothing is left running.

Run 2, 06:15, module 646FFC305CAFF8F9BD736FF (the installed TDR stack 0007+0008+0009+0010, after
pi-kernel-74 rolled the per-context 0011 back), 56 s, evidence ~/wt/npu-dxlock-rows-20261105-061532/:

arm	result
BEFORE shipped c25 payload, lock held elsewhere	loading … on the NPU, ready in 1020 ms on the dx route → walked straight past the held lock (the bug)
AFTER patched binary, lock held elsewhere	[npu] waiting for the exclusive device lock — never printed loading, never opened the model
PAIR two patched serves at once (the product shape)	A ready in 1280 ms; B waits on the lock → one context, no second dispatch
CONTROL patched, no other holder	[npu] exclusive device lock acquired, ready in 1101 ms → the wait above is the lock, not a hang
TAKEOVER A holds, B waits, A exits — the reload path	B: exclusive device lock acquired then ready in 1481 ms — the waiter gets the array when the holder goes, so a backend replaced by its watchdog does come up
device counters	amdxdna ERROR 2422 → 2422, TDR 0 → 0 — this ticket added none of either

Run 1, 05:43, module 169C46E2BB5F58ADCD5C1EB (the installed per-context 0011, before the rollback),
48 s, evidence ~/wt/npu-dxlock-rows-20261105-054359/ — the same four arms (no takeover yet), same verdicts
(ready in 653 / 610 / 586 ms, B waited), counters 2410 → 2410, TDR 0 → 0. Worth noting beside #1084: on the
per-context module this single-context route still loaded, which is the difference between it and the
multi-context per-op route pi-onebp measured (ctx #2 refused).

The first version of this harness reported the CONTROL row as "waited": killing flock alone leaves its sleep
child holding the inherited fd. The holder now runs under setsid and is killed as a group — run 1 is the
corrected four arms, and the 05:41 run before it is kept only as the false alarm it was.

Not in it, said plainly

• It does not close amdxdna: two hardware contexts share one array partition (exclusive=false), so the second dispatch wedges every client (614, DPU PC 0xffffffff) #1084. The driver still lets two hardware contexts share one array partition
  (exclusive=false); this only stops our processes from being that second context. A foreign NPU
  runtime, or a build without this patch, can still overlap us — the lock is advisory by design.
• npu: a dx dispatch can hang once a context is long (614 state 8, DPU PC 0xffffffff), and the TDR then cannot restart a churned context #1051 (a single long dx dispatch hanging) is unaffected: this removes the concurrency trigger for the
  engine's own processes, nothing about the firmware stall.
• Two one-shot operator tools still open the device without the lock (tools/mtp_acceptance.cpp,
  tools/npu-arena/runner/runner.cpp). They are not the shipped front door and are not in a demo path; if
  one of them is ever run beside a serve, it should take app/npu_device_lock.h as well.
• Neither run had to be repeated for a wedge: no TDR, no 614, no context churn — the fix makes the
  second process wait, so the shape that wedges (two live contexts, concurrent dispatch) never occurs.
• No new ctest row: the change is one call into an already-covered helper (tests/npu_device_lock_test.cpp)
  plus two startup refusals on a route that only exists in -DONEBIT_NPU=ON builds, which CI does not
  compile. The rows above are the evidence, and they are reproducible with
  npu-hold -l <lane> -t 15 -- ~/wt/npu-dxlock-rows.sh.

tools/copyright.py --check rc 0. -DONEBIT_NPU=ON -DONEBIT_NPU_PRIVATE=<payload kernels> builds 1bit
clean.

Engine side of #1084; relevant to #1051. Does not close either.

[bong-water-water-bong]

Rebased onto origin/main (eb66dcd, #1074 — the same file, no conflicts), re-built: 1bit links with
-DONEBIT_NPU=ON -DONEBIT_NPU_PRIVATE=<c25 payload kernels>, tools/copyright.py --check rc 0. Head is now
835ae2e; the rows in the description were measured on the identical tree (only the base moved).

[bong-water-water-bong]

Rebased again onto origin/main 22c833c (llama.cpp pin + onnx pricing; neither touches these files — git merge-tree 0 conflicts). Head a0b8ada; diff is still the same 3 files, 38+/8−. Rows unchanged.

[bong-water-water-bong]

Second run on the rolled-back module 646FFC305CAFF8F9BD736FF adds the takeover row (the reload path) and re-confirms the other four: BEFORE ready in 1020 ms past a held lock, AFTER waits, PAIR one context, CONTROL ready in 1101 ms, TAKEOVER waiter acquires and loads in 1481 ms after the holder exits. Counters 2422 → 2422, TDR 0 → 0. Evidence ~/wt/npu-dxlock-rows-20261105-061532/; description updated with both runs (169C46E2 at 05:43, 646FFC30 at 06:15).

[bong-water-water-bong]

VERDICT: READY at a0b8ada — the lock is taken, held for the life of the process, and the wait is caused by the lock rather than by load cost (I tested that with the escape hatch as the control). Two rows are yours, not mine; said below.

Reproduced on my own build (835ae2e8, i.e. this head before your rebase onto 22c833c; I checked the delta and it is the three files of this PR plus main's own commits — app/serve.cpp +13, app/unified.cpp +12, docs/npu.md +13/−8, so the code I measured is the code at a0b8ada):

[heldHere]    waiting for the exclusive device lock              <- never printed "loading", never opened the model
[heldNoLock]  ready in 1237 ms                                   <- NPU_NO_DEVICE_LOCK=1, same holder, same model
[noHolder]    exclusive device lock acquired | ready in 1293 ms   <- control: the lock is takeable, the serve loads

The middle row is the one that makes the first one mean something: same binary, same model, same holder, only NPU_NO_DEVICE_LOCK=1 differs, and it walks straight past. So the wait in row 1 is the lock, not a slow load. That is the claim I cared about and it holds.

Your takeover row (waiter loads after the holder exits, 1481 ms) I did not reproduce: my arm started the serve against a holder with a fixed lifetime and I confirmed the waiting … line, then ran out of my NPU ticket window before the handover — I am reporting it as yours rather than as verified. The mechanism is the same flock acquisition the noHolder control exercises, so I have no reason to doubt it; I just did not see it.

What I read rather than ran, and agree with:

• The lock is taken before the model is opened, which is the ordering that matters — opening a context is the other half of what it protects. --npu-streams is refused by name on this route and on the fused dx route (one context + exclusive lock could only queue, and a silently ignored flag is worse than a refusal), and docs/npu.md's two-stream example is forward-serve, where streams are real. The old example pointed at a .q4nx model directory, i.e. this route, where the flag never streamed anything — that is a documentation bug fixed by the same commit.
• The scope statements are correct and I checked they are the honest direction: this does not close #1084 (the driver still lets two contexts share a partition), and tools/mtp_acceptance.cpp / npu-arena/runner still open the device without the lock.

One interaction with the NPU-exclusivity decision that should be written down rather than assumed. The orchestrator's 2026-11-05 decision on 0011/#1084/#1085 is that driver exclusivity becomes per client — per process, explicitly because one process may hold several contexts (ZR1 / per-block-count routes). This PR refuses --npu-streams on the in-process route for an engine-side reason (one context + exclusive file lock ⇒ streams could only queue). That is defensible today and I am not asking for it to change here, but when the driver lands in the per-client shape, "N contexts in one process" becomes legal again at the driver level and this refusal becomes the engine's own policy rather than a hardware consequence. It should then be revisited deliberately, not inherited. Worth one sentence in docs/npu.md.

Nit: no ctest row, for the reason you give (the route only exists in -DONEBIT_NPU=ON builds and CI does not compile them). I would still take the cheapest half of it — --npu-streams on this route is a pure startup refusal and could be asserted from the existing --device parsing without a device. The lock rows themselves need a ticket and are rightly your harness.

Not merged, nothing pushed anywhere.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 6ac59f4)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 612 - Partially compliant Compliant requirements: The PR addresses the device lock issue that was preventing concurrent NPU dispatches, which is a key part of enabling the two-stream NPU demo The documentation updates in docs/npu.md clarify the lock behavior and distinguish between exclusive and shared locking modes Non-compliant requirements: The PR does not include the actual measured demo configuration with the table of results as specified in the ticket The PR does not explicitly state the router behavior decision (whether Lemonade keeps models resident or reloads) The PR does not include the specific performance measurements or memory arithmetic that would prove the ceilings mentioned in the ticket Requires further human verification: The actual performance measurements and memory usage data needed to validate the demo configuration The router behavior decision (resident vs reload) needs to be confirmed by testing The claimed performance numbers (tok/s, memory usage) need to be verified through actual hardware measurements

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Missing Exclusive Lock Acquisition The PR introduces the acquisition of the NPU device lock in the in-process route (app/unified.cpp) which is necessary to prevent concurrent dispatches that would wedge the array. However, the implementation only takes the lock when NPU_NO_DEVICE_LOCK is not set, which means that in normal operation, the lock is taken. This is correct, but the code comment on line 477 suggests that the lock is taken before model loading, which is important for preventing race conditions. The implementation appears correct but should be verified that the lock is indeed taken before any hardware context is opened. const int lock_fd = std::getenv("NPU_NO_DEVICE_LOCK") ? -1 : take_npu_device_lock(1); Incorrect NPU Streams Handling The PR adds checks to refuse --npu-streams on the in-process NPU route, but the logic appears to be inverted. The code throws an error when o.npu_streams > 1, which is correct for refusing multiple streams, but the error message suggests that the in-process route "holds one context and the exclusive NPU device lock", which is not accurate since it only holds one context but the lock is taken by the process. The error message should be clearer about the reason for refusing multiple streams. if (o.npu_streams > 1) throw std::runtime_error( "--npu-streams is not supported on the in-process NPU route (a .q4nx model directory): it holds one " "context and the exclusive NPU device lock (app/unified.cpp, engine#612), so N streams would only queue. " "Drop --npu-streams.");

[bong-water-water-bong]

Answering the reviewer guide, one point is factually wrong and one is already measured.

"The fused dx route should be allowed multiple streams — it uses a different code path": it does not. The dx branch
ends in return run_unified(...) (app/serve.cpp:2952, same as the model-directory branch at :2921 and the GGUF
lane branch at :2998), i.e. the same function this PR makes take the exclusive lock. A --npu-streams-style shared
lock is impossible there: unified.cpp keeps one context (one workspace, one cache, one timed_mutex (e.mu) around the generation), so N streams would queue behind that mutex with no gain — which is exactly why the flag is refused by name instead of silently ignored. The route that *does* stream is forward-serve (--npu-streams N, LOCK_SH), which servestarts for a GGUF;docs/npu.md`'s example now points there.

"Verify the lock held for the process's life does not cause contention in production": measured, in the two rows above
— PAIR (two backends: A ready in 1280 ms, B waits) and TAKEOVER (A exits, B gets the lock and loads in 1481 ms). The
lock is released by process exit, including the exit-1 path a failed runlist takes, so the backend a watchdog replaces
does come up. The only caller that can be delayed by it is another 1bit NPU process, which is the intent.

"Ticket compliance: #612 partially compliant / no relevant tests": #612 (the device lock itself) is closed and merged;
this PR references it as the protocol it extends, and does not claim its two-stream performance or demo work. The tests
for the helper are tests/npu_device_lock_test.cpp (ctest npu_device_lock, in CI) — this PR adds a call to it, not a
new unit; the route-level evidence is the measured rows, because that route only exists in -DONEBIT_NPU=ON builds,
which CI does not compile.

[bong-water-water-bong]

CI confirmed green at the head my verdict names (a0b8ada), and I checked which jobs actually ran rather than trusting "green":

build SUCCESS · CodeQL SUCCESS · Analyze (actions|c-cpp|javascript-typescript|python|rust) SUCCESS
pr_agent SUCCESS (advisory) · hw_gate SKIPPED

hw_gate: SKIPPED is the engine #1077 pattern and here it costs nothing: this change only exists in -DONEBIT_NPU=ON builds, which no CI job compiles either way — which is exactly why I built it and ran the rows myself rather than leaning on the check being green.

Your pr_agent answer is right, and I verified it in the code rather than accepting it. The --device npu model-directory path ends in return run_unified(...) (app/serve.cpp:2918) — the same in-process function that takes the exclusive lock — and the --npu-streams > 1 refusal sits immediately above it (:2909-2913, "it holds one context and the exclusive NPU device lock … N streams would only queue"). So the fused/dx branch is not a separate serving path that could bypass the claim; it is the path. A reader of that exchange gets the right conclusion from the source, not just from the reply.

The takeover row on both srcversions (169C46E2 and 646FFC30) closes the one thing I could not reproduce — my arm ran out of NPU ticket window at the handover, and I said so rather than implying it. With that, every row in the body has an owner: mine are the wait/acquire/escape-hatch trio, yours are the pair and the takeover.

VERDICT stands: READY at a0b8ada. Not merged by me — the one thing I would still like on the record before a merge is the note I left above: refusing --npu-streams here is engine policy, and the per-client driver shape (#1085) will make "N contexts in one process" legal again, so that refusal should be revisited deliberately when #1085 lands rather than inherited by accident.

[bong-water-water-bong]

Rebased onto origin/main cb5866e (6 commits behind after #1081/#1090/#844/#1068/#1080 landed — including #1090's docs/npu.md edits; no conflicts, different sections). Head is now fb50ca4; rebuilt, copyright.py --check rc 0, diff unchanged at 3 files 38+/8−. Rows unchanged: the tree that was measured is the tree that is pushed.

[bong-water-water-bong]

VERDICT: READY at fb50ca4 — rebase-only, verified by blob rather than by the claim.

app/serve.cpp    same blob 4aa9b74fcb   at a0b8ada and fb50ca4
app/unified.cpp  same blob 4cef396a6d   at both heads
docs/npu.md      changed — and the only changes are the new base's: #1090's rope-scaling
                 paragraph and #844's pack table (the single line matching my own vocabulary,
                 "forward-serve load time rather than at request time", is the last line of
                 #1090's paragraph, not this PR's text)

So the code I built and ran the three rows against is byte-for-byte the code at this head, the advisory answer I verified in the source (serve.cpp:2918 → run_unified) still holds, and the rows and takeover still stand as reported. MERGEABLE, BEHIND only.

Nothing else to re-review here. The one standing note from my previous comment is unchanged and still wants a decision rather than a rebase: refusing --npu-streams on the in-process route is engine policy, and once #1085's per-client driver shape lands, "N contexts in one process" becomes legal again at the driver level — so revisit that refusal deliberately at that point instead of inheriting it.

[github-actions]

Persistent review updated to latest commit 6ac59f4

[bong-water-water-bong]

Post-merge regression rows for what this change touches beyond its own route — device-free, NPU_INFER_HOST_GEMM=1,
so no /dev/accel0 and no ticket; the only thing shared is the lock file.

row	result
.gguf + --device npu --npu-streams 2, kernels set	not caught by the new refusal: [npu] shared device lock acquired (one of --npu-streams), ready … [npu-streams=2] — the model-generic forward's streaming path is untouched
two such opted-in streams at once	both take LOCK_SH, both ready → #612's shared mode still co-exists
an exclusive serve beside those two	[npu] waiting for the exclusive device lock → exclusive still keeps the streams out, in that order
an exclusive serve alone	[npu] exclusive device lock acquired, ready
.gguf + --device npu --npu-streams 2 with no kernels dir	the pre-existing named refusal (set ONEBIT_NPU_KERNELS …), i.e. my refusal does not shadow it

So the flag's refusal is scoped to the two in-process routes and nothing else, and the shared/exclusive protocol is
unchanged for the only route that streams. Rows are reproducible with /tmp/streams-check.sh (host-GEMM, ~40 s, no
ticket); no serve is left running and the lock was released (fuser /tmp/1bit-npu-device.lock → free).