Name every thaw that fetches under the freeze lock in the lazy write-back's comments, and say what LZ_SLOTS does when it runs short.

The review found that the comments on frz_lock (memory.c, above FRZ_SHIFT), lazy_fly_run and vk_memory.c lazy_fetch
said a fetch runs under frz_lock only in a mapping change's thaw or with NX2NAVI_WB_LAZY_UNLOCKED=0. In fact
thaw_begin_locked's lazy_drop_region_locked runs in every thaw under the lock: a mapping change, the lazy heap's commit,
and thaw_fault on a write lazy_fault did not take. lazy_drop_region_locked's own comment said "a write fault in a
frozen region", but a frozen region holds no armed ranges. And the LZ_SLOTS comment called four slots more than the
threads that touch armed pages together, where the reference session had six such threads and the flight table holds
64. The comments now give that list of thaws, and LZ_SLOTS says that fetches past it wait 20 us at a time for a slot.
Comments only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
chrg6
2026-11-13 20:05:40 -05:00
parent a8d18ef474
commit b5e4243cc4
2 changed files with 19 additions and 15 deletions

View File

@@ -72,8 +72,9 @@ static int reserve_at(uint64_t at, uint64_t size, int watch) { return plat_mem_r
// read still in flight writes its buffer later. Nothing writes guest memory under frz_lock. It is not a leaf: lz_lock
// and vm_lock come inside it, a strict thaw's page calls run under it (thaw_pages_vm), and a holder may wait for a
// range of the lazy write-back in flight, whose fetch (vk_memory.c lazy_fetch) takes the queue lock and waits for the
// GPU ("Ranges in flight"; in a mapping change's thaw, and with NX2NAVI_WB_LAZY_UNLOCKED=0, the fetch itself runs
// under frz_lock). So no holder of the queue lock may touch guest memory or take frz_lock.
// GPU ("Ranges in flight"; in a thaw under the lock: a mapping change, the lazy heap's commit or a write fault
// lazy_fault did not take; and with NX2NAVI_WB_LAZY_UNLOCKED=0 the fetch itself runs under frz_lock). So no holder of
// the queue lock may touch guest memory or take frz_lock.
#define FRZ_SHIFT 21
#define FRZ_SIZE (1ull << FRZ_SHIFT)
#define FRZ_HOLD_MS 5000u
@@ -348,9 +349,10 @@ static int lazy_fly_start_locked(LzSeg s) {
return k;
}
// No lock held (frz_lock in a mapping change's thaw, in mem_lazy_skip's give-back and with NX2NAVI_WB_LAZY_UNLOCKED=0):
// the n segments s[] in flight in the entries k[] get the mirror's bytes, the stale ones in one call of lz_fetch, then
// their pages' access, and leave flight. n is at most LZ_BATCH.
// No lock held (frz_lock in a thaw under the lock: a mapping change, the lazy heap's commit or a write fault lazy_fault
// did not take; in mem_lazy_skip's give-back, which fetches nothing; and with NX2NAVI_WB_LAZY_UNLOCKED=0): the n
// segments s[] in flight in the entries k[] get the mirror's bytes, the stale ones in one call of lz_fetch, then their
// pages' access, and leave flight. n is at most LZ_BATCH.
static void lazy_fly_run(const int *k, const LzSeg *s, int n) {
MemLazyCopy cp[LZ_BATCH];
int nc = 0;
@@ -480,10 +482,11 @@ static int lazy_take_batch_locked(const uint64_t (*rg)[3], int nr, int *k, LzSeg
return n;
}
// frz_lock held exclusively (a thaw: a mapping change, a commit of the lazy heap, a write fault in a frozen region):
// every segment that reaches into region r goes, its fetch under frz_lock, since the caller's page calls follow under
// it; then the ranges other threads have in flight there end first. Not a fault on an armed page: no hold. mem_thaw
// releases its regions before it takes the lock (lazy_release_regions), so what is left here is rare.
// frz_lock held exclusively (a thaw under the lock: a mapping change, a commit of the lazy heap, a write fault
// lazy_fault did not take): every segment that reaches into region r goes, its fetch under frz_lock, since the
// caller's page calls follow under it; then the ranges other threads have in flight there end first. Not a fault on an
// armed page: no hold. mem_thaw releases its regions before it takes the lock (lazy_release_regions), so what is left
// here is rare.
static void lazy_drop_region_locked(uint64_t r) {
uint64_t rg[1][3] = {{r << FRZ_SHIFT, (r + 1) << FRZ_SHIFT, 0}};
for (;;) {

View File

@@ -3776,9 +3776,9 @@ static void snap_free(void *arg) { // completion thread: the submission is don
// from the prologue of a part split off earlier) it writes back, as ever, what it skipped and finds no longer armed;
// the snapshot for that merge is taken in the part's own prologue, before its GPU work. From that check to the part's
// hand-over (vk_mem_lazy_handed) memory.c's window keeps a fault from releasing a range unseen.
// Fetches run on several threads at once (memory.c "Ranges in flight": no memory.c lock is held around one), each in a
// slot of its own: a command pool (used by one thread at a time), its command buffer and a fence. LZ_SLOTS is more than
// the threads that touch armed pages together; a fetch that finds every slot busy waits 20 us at a time.
// Fetches run on several threads at once (memory.c "Ranges in flight": mostly no memory.c lock is held around one, see
// lazy_fetch), each in a slot of its own: a command pool (used by one thread at a time), its command buffer and a fence.
// Fetches past LZ_SLOTS at once wait 20 us at a time for a slot (lz_slot_waits counts them).
#define LZ_SLOTS 4
static struct { VkCommandPool pool; VkCommandBuffer cb; VkFence fence; } lz_slot[LZ_SLOTS];
static volatile uint32_t lz_slot_free; // bit i: slot i is free
@@ -3808,9 +3808,10 @@ static int lz_slot_take(void) {
}
}
// memory.c's fetch, with no memory.c lock held (frz_lock in a mapping change's thaw or with
// NX2NAVI_WB_LAZY_UNLOCKED=0): each copy cp[i], [off, off + size) of the mirror h0 into the imported buffer h1, after
// all the GPU work handed over so far, in one submission with one wait.
// memory.c's fetch, with no memory.c lock held (frz_lock in a thaw under the lock: a mapping change, the lazy heap's
// commit or a write fault lazy_fault did not take; and with NX2NAVI_WB_LAZY_UNLOCKED=0): each copy cp[i],
// [off, off + size) of the mirror h0 into the imported buffer h1, after all the GPU work handed over so far, in one
// submission with one wait.
static void lazy_fetch(const MemLazyCopy *cp, int n) {
if (vk_device_is_lost() || n <= 0) return; // the run is ending (vk_device.c vk_device_lost_check): bytes stay
VkCtx *c = vk_ctx();