diff --git a/dm/games/0400486024DEC000/README.md b/dm/games/0400486024DEC000/README.md index a001444dbb..aaf4e8bdd3 100644 --- a/dm/games/0400486024DEC000/README.md +++ b/dm/games/0400486024DEC000/README.md @@ -184,6 +184,7 @@ From an empty save: at about 45 s `A` (title), `A` (New Game), then the movie. F - [A cold start leaves a file per timer flush, and the next start compacts them all](../../../docs/reference/rendering.md#a-cold-start-leaves-a-file-per-timer-flush-and-the-next-start-compacts-them-all): a cold start wrote 80 run files and the next start captured 8,859 pipelines again, 762 s before the first frame on an M4 with 16 GB; with the change 5 files, and 3.3 s (issue #1747). - [Archive lookups asked every file, so a few files made a start compile every base again](../../../docs/reference/rendering.md#archive-lookups-asked-every-file-so-a-few-files-made-a-start-compile-every-base-again): 9 files of play beside a compaction's 5 made the next start capture 8,543 bases again, 713 s before the first frame (PR #1880); an index beside each file makes a lookup ask one file whatever the number of files. - [Texture IDs of one storage share one packaged texture](../../../docs/reference/nvn-objects.md#texture-ids-of-one-storage-share-one-packaged-texture) +- [A write missed while opening shared pool pages](../../../docs/reference/nvn-objects.md#a-write-missed-while-opening-shared-pool-pages): a tracker race reproduced with synthetic memory while investigating issue #2086. The race is fixed; its connection to the reported terrain corruption is still unverified in play. - [A texture shared by its packaged texture's serials left the residency set with one of them](../../../docs/reference/nvn-objects.md#a-texture-shared-by-its-packaged-textures-serials-left-the-residency-set-with-one-of-them): GPU page faults 15-45 minutes into play (issue #1326), on 260x260 terrain tiles made GPU textures at the GPU's first write. - [Finalizing a copy of a texture object](../../../docs/reference/nvn-objects.md#finalizing-a-copy-of-a-texture-object) - [Dynamic resolution that never moves, and draw passes that wait for each other](../../../docs/reference/frame-rate.md#dynamic-resolution-that-never-moves-and-draw-passes-that-wait-for-each-other) diff --git a/dm/gfx/nvn/nvn_dirty.c b/dm/gfx/nvn/nvn_dirty.c index 1fbb98357a..76972e7f02 100644 --- a/dm/gfx/nvn/nvn_dirty.c +++ b/dm/gfx/nvn/nvn_dirty.c @@ -215,26 +215,28 @@ void nvn_dirty_open(const void *addr, size_t len) { pthread_mutex_lock(&g_slot_mu); g_nopen = 0; uintptr_t a = (uintptr_t)addr, e = a + len; int n = atomic_load(&g_nr); + // Take and hash every range before opening any page: adjacent pools can share + // a host page. Opening one first let a write reach its neighbour before that + // neighbour's hash, so the neighbour's before/after hashes missed the write. for (int k = 0; len && k < n; k++) { range_t *r = rg(k); if (!live_at(k) || e <= r->base || a >= r->end) continue; - uintptr_t lo = a > r->base ? a : r->base, hi = e < r->end ? e : r->end, p = (lo - r->base) >> g_shift, q = (hi - 1 - r->base) >> g_shift, run = 0; int open = 0; - for (uintptr_t i = p; i <= q + 1; i++) { - // A protected page is read-only and, with g_slot_mu held, stays protected until someone unprotects it: hashed first, then taken; one unprotected meanwhile is not ours - uint8_t st = i <= q ? atomic_load(&r->state[i]) : PG_NONE; int took = 0; + uintptr_t lo = a > r->base ? a : r->base, hi = e < r->end ? e : r->end, p = (lo - r->base) >> g_shift, q = (hi - 1 - r->base) >> g_shift; + for (uintptr_t i = p; i <= q; i++) { + uint8_t st = atomic_load(&r->state[i]); if (st == PG_PROT) { uint64_t h = page_hash((const void *)(r->base + (i << g_shift))); - if ((took = atomic_compare_exchange_strong(&r->state[i], &st, PG_BUSY))) open_add(r, i, h); + if (atomic_compare_exchange_strong(&r->state[i], &st, PG_BUSY)) open_add(r, i, h); } - if (took) { if (!open) { run = i; open = 1; } continue; } - if (open) { - mp(r->base + (run << g_shift), (i - run) << g_shift, PROT_READ | PROT_WRITE); - for (uintptr_t j = run; j < i; j++) atomic_store(&r->state[j], PG_OPEN); - open = 0; - } - if (st == PG_BUSY) busy_wait(r, i); // being unprotected: writable once it is done (with g_slot_mu held nothing protects it), and the buffer must not be made before + if (st == PG_BUSY) busy_wait(r, i); // another thread's unprotect: it finishes without taking g_slot_mu or waiting for another page } } + for (size_t j = 0; j < g_nopen; ) { + range_t *r = g_open[j].r; size_t s = j++; + while (j < g_nopen && g_open[j].r == r && g_open[j].p == g_open[s].p + (j - s)) j++; + mp(r->base + (g_open[s].p << g_shift), (j - s) << g_shift, PROT_READ | PROT_WRITE); + for (size_t k = s; k < j; k++) atomic_store(&r->state[g_open[k].p], PG_OPEN); + } atomic_fetch_add(&g_opened, g_nopen); } void nvn_dirty_close(void) { diff --git a/dm/gfx/tests/t_dirty_race.c b/dm/gfx/tests/t_dirty_race.c index c1480400a7..cc969cf163 100644 --- a/dm/gfx/tests/t_dirty_race.c +++ b/dm/gfx/tests/t_dirty_race.c @@ -35,7 +35,7 @@ static int g_fail; if (want_ != got_) g_fail++; \ } while (0) -static uint8_t *g_page; static _Atomic int g_armed, g_go, g_b_done; +static uint8_t *g_page, *g_shared_write; static _Atomic int g_armed, g_go, g_b_done; static int g_publish_armed, g_observed_clean; static uint8_t *g_publish_page; static const volatile void *g_publish_gen; @@ -50,7 +50,13 @@ static void t_before_store(const volatile void *object) { static void *thread_b(void *arg) { (void)arg; while (!atomic_load(&g_go)) usleep(200); prep_write(g_page, 16); atomic_store(&g_b_done, 1); return NULL; } int t_mprotect(void *a, size_t n, int prot) { if (prot == PROT_READ && atomic_exchange(&g_armed, 0)) { atomic_store(&g_go, 1); usleep(100000); } // Before A's mprotect lands: let B in and wait until it runs to the end or gets stuck waiting - return mprotect(a, n, prot); + int result = mprotect(a, n, prot); + // A guest store after the first range opens a shared page, before the next range hashes it. + if (!result && prot == (PROT_READ | PROT_WRITE) && g_shared_write && + (uintptr_t)g_shared_write >= (uintptr_t)a && (uintptr_t)g_shared_write < (uintptr_t)a + n) { + *g_shared_write = 9; g_shared_write = NULL; + } + return result; } // A page another thread is unprotecting (its mprotect to writable done and the new generation stored, the state still BUSY) must be waited out by nvn_dirty_protect and @@ -149,6 +155,26 @@ static void open_close(size_t pg) { EXPECT("unwritten page: a later write is seen", 1, nvn_dirty_since(m + 2 * pg, pg, later)); nvn_dirty_remove_range(m); munmap(m, 3 * pg); } +// Adjacent 4 KB-aligned pools share a 16 KB host page. Opening A makes B's bytes +// writable too, before B takes its own hash. A write in that window must dirty B +// even though B's own before/after hashes match. No game data or GPU is involved. +static void open_shared_page(size_t pg) { + if (pg <= 4096) return; // no shared host page for 4 KB-aligned pools + uint8_t *m = mmap(NULL, 3 * pg, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0); + if (m == MAP_FAILED) { perror("mmap"); g_fail++; return; } + size_t na = pg - 4096, nb = pg + 4096; + uint8_t *b = m + na; + nvn_dirty_add_range(m, na); nvn_dirty_add_range(b, nb); + uint64_t stamp = nvn_dirty_now(); + nvn_dirty_protect(m, na); nvn_dirty_protect(b, nb); + nvn_dirty_open(m, 2 * pg); nvn_dirty_close(); + EXPECT("shared page with no writes: B stays clean", 0, nvn_dirty_since(b, nb, stamp)); + g_shared_write = b + 16; + nvn_dirty_open(m, 2 * pg); nvn_dirty_close(); + EXPECT("the guest store reached B's shared-page bytes", 9, b[16]); + EXPECT("write while A opens: B's texture is dirty", 1, nvn_dirty_since(b, nb, stamp)); + nvn_dirty_remove_range(b); nvn_dirty_remove_range(m); munmap(m, 3 * pg); +} // A page another thread is unprotecting (BUSY, still read-only) when nvn_dirty_open looks: the buffer must not be made over it while it is read-only, so the open waits for it static void open_waits_busy(size_t pg) { uint8_t *m = mmap(NULL, pg, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0); @@ -193,5 +219,6 @@ int main(void) { generation_publish(pg, 1); open_close(pg); open_waits_busy(pg); + open_shared_page(pg); printf("t_dirty_race: %s\n", g_fail ? "FAILED" : "all passed"); return g_fail != 0; } diff --git a/docs/agents/symptoms.md b/docs/agents/symptoms.md index 96bf7c4243..15fa6f6741 100644 --- a/docs/agents/symptoms.md +++ b/docs/agents/symptoms.md @@ -603,6 +603,7 @@ One line per symptom in the [subsystem references](../reference/): what you see - textures that should hold CPU-written data come out magenta (ASTC's error colour) or black. → [Sampled textures in a sparse pool uploaded through the page table](../reference/nvn-objects.md#sampled-textures-in-a-sparse-pool-uploaded-through-the-page-table) - the submit thread spends most of its time in `nvn_dirty_since` under `nvn_sparse_since`, called from `tex_mtl` (render-target and texture lookups of textures in a sparse pool). → [A sparse texture's CPU-write check at every lookup](../reference/nvn-objects.md#a-sparse-textures-cpu-write-check-at-every-lookup) - some textures are re-uploaded every frame (several MB per frame). → [Dirty-page ranges of pools that share a host page](../reference/nvn-objects.md#dirty-page-ranges-of-pools-that-share-a-host-page) +- a CPU-written texture stays stale although its storage changed while the host made a zero-copy pool buffer. → [A write missed while opening shared pool pages](../reference/nvn-objects.md#a-write-missed-while-opening-shared-pool-pages) - packaged textures re-uploaded at every use with unchanged content; upload staging and frame times rising. → [Writes beside a packaged texture in a shared host page](../reference/nvn-objects.md#writes-beside-a-packaged-texture-in-a-shared-host-page) - with `tex_dirty_upload`, textures the CPU never writes (window textures, render targets) are logged as `CPU-written texture …` or `registered texture written by the CPU after its first use: uploaded from its storage` soon after boot. The present thread then re-uploads a window texture from its storage, and the host aborts: `[RED] upload batches belong to the submit thread only: up_begin called on thread 'nvn-present'`, with `pkg_upload` ← `tex_mtl_` ← `present_now` on the stack. → [Writes beside a registered texture in a shared host page](../reference/nvn-objects.md#writes-beside-a-registered-texture-in-a-shared-host-page) - about one run in four stalled early (around the opening movie or the title), with no log output, not even the time-limit report. → [Dirty-page tracker race and early stalls](../reference/nvn-objects.md#dirty-page-tracker-race-and-early-stalls) diff --git a/docs/reference/nvn-objects.md b/docs/reference/nvn-objects.md index c158f14d9b..0a17240c8b 100644 --- a/docs/reference/nvn-objects.md +++ b/docs/reference/nvn-objects.md @@ -664,6 +664,14 @@ Games read this count as a position in their own command memory and size their h - **Found on:** the source names no game. - **Check:** `t_dirty_slots` covers it. +### A write missed while opening shared pool pages + +- **Symptom:** a CPU-written texture stays stale although its storage changed while the host made a zero-copy pool buffer. +- **Cause:** `nvn_dirty_open` hashed and opened each pool range in turn. Adjacent 4 KB-aligned pools can share a 16 KB host page. Opening the first pool made the second pool's bytes writable before the second pool's hash was taken. A CPU write between those steps changed the first range's hash, but the second range's before/after hashes matched, leaving its texture clean with stale content. +- **Fix:** take and hash all protected ranges before making any of their pages writable. Closing still re-protects the pages and stamps only changed content; unchanged pages do not force texture uploads. +- **Found on:** a synthetic reproduction while investigating [Ocarina of Time](../../dm/games/0400486024DEC000/README.md) [issue #2086](https://github.com/seedds/nxnative/issues/2086). Whether this race caused that game's reported terrain corruption remains open; no game files or gameplay run were used. +- **Check:** `t_dirty_race` places two adjacent pools in one host page and makes a real store to the second pool immediately after the first becomes writable. Without the fix its texture is incorrectly clean; with the fix it is dirty. Opening and closing without a write stays clean. The existing fault, host-write, busy-page and pool-retirement tests also run without game files. + ### Writes beside a packaged texture in a shared host page - **Symptom:** packaged textures re-uploaded at every use with unchanged content; upload staging and frame times rising.