# Claude Sonnet 5 ## What I found: a real, documented invariant violation — but it doesn't leak the way I expected `sys/vm/vm_page.h` documents the locking contract explicitly: > *"The page must be hard-busied to make the following changes: (1) ... (2) Any transition of m->wire_count to 0 or from 0. Other transitions (e.g. 2→1, 1→2, etc) are allowed without locks."* And in the confirmed-fixed commit, `vm_page_unwire()` was changed specifically to let `pmap_removed_pte()` skip that rule: ```c - KKASSERT(m->busy_count & PBUSY_LOCKED); + KKASSERT(activate < 0 || (m->busy_count & PBUSY_LOCKED)); ``` `pmap_removed_pte()` (called during ordinary PTE teardown — `munmap`, process exit, page replacement) calls `vm_page_unwire(m, -1)` **without busying the page at all**. That's a genuine, documented-rule-breaking capability that didn't exist before this commit: wire_count can now transition to/from 0 concurrently, behind the back of anyone who's holding the page busied specifically because they believed that couldn't happen. That's exactly the shape of bug I expected to find feeding into `vfs_vmio_release()`'s `"vmiopg"` site — it busies the page, calls `vm_page_unwire()`, then does a **plain, unsynchronized re-read** of `m->wire_count` to decide what to do next: ```c vm_page_busy_wait(m, FALSE, "vmiopg"); ... vm_page_unwire(m, ...); if (m->wire_count == 0) { /* <-- exactly the race window I was hunting for */ ... } ``` But when I traced every branch this leads into (`vm_page_try_to_cache()`, `vm_page_try_to_free()`, and the direct-wakeup branches), **every single exit path calls `vm_page_wakeup(m)`** — including the defensive re-check paths (`vm_page_try_to_cache` re-checks `m->wire_count` itself at the top and just wakes-and-bails if it's non-zero). Dillon wrote this code carefully enough that even a wire_count that gets yanked out from under it mid-sequence doesn't produce a missing wakeup — worst case it produces a page that's *permanently stuck wired* (if the race corrupts wire_count via underflow, since `uint32_t` wraps to a huge nonzero value and can never legitimately return to 0 again), but that page still gets unbusied. Wrong outcome, but not your symptom. So: **the invariant violation is real, but I can't pin the specific `vmiopg` busy-leak on it via this path.** That's useful negative information — it tells us the leak isn't in `vfs_vmio_release`/`getnewbuf`'s own decision logic. ------ Tracing the write path (`dmtc_bio_write_encrypt_start`/`dmtc_bio_write_encrypt`) turned up something structurally important that I'd missed by focusing only on the read/completion side. ```c static int dm_target_crypt_strategy(dm_table_entry_t *table_en, struct buf *bp) { ... case BUF_CMD_WRITE: bio = push_bio(&bp->b_bio1); bio->bio_offset = bp->b_bio1.bio_offset + priv->block_offset * DEV_BSIZE; bio->bio_caller_info1.ptr = priv; dmtc_bio_write_encrypt_start(bio); /* <-- called SYNCHRONOUSLY, inline */ break; ``` ```c void dmtc_bio_write_encrypt(struct bio *bio, uint8_t *data_buf, size_t data_buf_sz) { ... memcpy(data_buf, bio->bio_buf->b_data, bytes); bio->bio_buf->b_error = dmtc_bio_encdec(priv, data_buf, bytes, bio->bio_offset, CRYPTOAPI_CIPHER_ENCRYPT); ... bio->bio_buf->b_data = data_buf; /* rewire to the crypto scratch buffer */ bio->bio_done = dmtc_bio_write_done; vn_strategy(priv->pdev->pdev_vnode, bio); /* only NOW does the real disk I/O start */ } ``` `push_bio()` doesn't clone the `struct buf` — the new `struct bio` still points at the **same** `bio_buf` (`bp`), the same one whose `b_xio.xio_pages[]` are the real VM pages that generic `bwrite()`/`vfs_busy_pages()` already busied *before* `VOP_STRATEGY()` was ever called. `dm_target_crypt` never touches that busy state — it just sits there, busied, for however long everything downstream takes. For a **plain** (unencrypted) device, the window between "pages busied" and "real disk I/O dispatched" is essentially nothing — `VOP_STRATEGY()` goes almost straight to the driver. For an **encrypted** device, that same window now includes, running synchronously in the calling thread (which could be a HAMMER2 `h2xop` worker holding whatever HAMMER2-level locks it normally holds for a write): a `memcpy` of up to `DMTC_BUF_SIZE` (~`MAXPHYS`, likely 256KB), a full AES-NI encrypt pass over that buffer, and — if `mpipe_alloc_callback()` doesn't get a buffer immediately because the pool is at its cap — an indefinite deferral until some *other* in-flight encrypted write completes and frees one back. **The original buffer's pages stay `PG_BUSY` for the entire extended duration**, not just for the underlying disk transfer. The read side has the mirror-image effect: `dmtc_bio_read_done()` runs the decrypt (memcpy + AES-NI) synchronously in the disk-completion context *before* calling `biodone(obio)`, so the read buffer's pages stay busied for hardware-completion-time **plus** decrypt time, instead of just hardware-completion-time. **Net effect, in both directions: dm-crypt measurably and structurally lengthens how long a buffer-cache buffer's backing pages stay `PG_BUSY`, compared to the same I/O on a plain device.** Under `-j16 buildworld`, with many threads doing mmap-based faults on the same HAMMER2 vnodes that are concurrently being written/read, this directly widens the window during which a concurrent `vm_fault()` (`"vmpfw"`) or buffer-recycle (`"vmiopg"`) can land on a page that's mid-flight through this longer pipeline — exactly the kind of probabilistic widening that turns a race that's nearly unreachable on a raw device into something "easily repeatable" on real hardware under load, without requiring `dm_target_crypt` itself to contain a logic bug. ### Why this doesn't contradict "mpipe threads were idle" This mechanism doesn't require mpipe exhaustion to matter — the synchronous memcpy+AES-NI cost alone, paid on *every single* read and write bio, is enough to shift timing. Pool exhaustion would make it worse (a real, if bounded, backlog of busied-and-waiting buffers), but the baseline effect exists even when the pool is comfortably idle. So "mpipe idle at observation time" is fully consistent with this being the widener rather than the ultimate holder-of-the-bug — which fits the running theory: dm-crypt makes the window big enough to hit, but *something else* (most plausibly the busy-bypassing `vm_page_unwire(m, -1)` path, or an as-yet-unfound corner case) is what actually leaves a page stuck once the window gets hit. ### What this suggests testing directly * Artificially inflate the busy-hold window without encryption As a more direct test of the theory, patch a raw (non-crypt) path to insert a deliberate delay (e.g. DELAY(50) or a spin) at the equivalent point — after pages are busied, before vn_strategy() dispatches to the real device — proportional to what AES-NI encryption of a MAXPHYS buffer costs. If this alone reproduces the vmiopg/vmpfw hangs on a plain HAMMER2 mount, it confirms the mechanism is purely about busy-hold duration, independent of dm-crypt's code at all. Step 3 (the artificial-delay test on a plain, non-encrypted HAMMER2 mount) is the one I'd prioritize — it's the cleanest possible experiment to separate "encryption widens a pre-existing generic race" from "encryption's own code has a bug." If a bare `DELAY()` inserted at the equivalent point reproduces the exact same `vmiopg`/`vmpfw` hang signature with zero crypto code involved, that's about as close to proof as static analysis can get you without finding the exact missing line.