diff --git a/docs/building.md b/docs/building.md index ce103f42..e85d7c3e 100644 --- a/docs/building.md +++ b/docs/building.md @@ -784,10 +784,11 @@ GPU, no game files. A change to a synchronous submission's deferred binds (`runtime/nvn_submit_guard.h`, `runtime/nvn_api_cmd.c` `nvn_submit_prepare`, `nvn_submit_done`, `resolve_deferred`) passes `python tests\submit_clone_check.py`, which builds the -header with the platform layer: four threads submit two recording copies while another rewrites the arrays their -deferred binds read, and no replay sees its handles change while both the in-place and the clone path run; then the -bind resolution step by step (changed and unchanged arrays, binds it leaves alone, calls 16 deep and not 17, a corrupt -record). No GPU, no game files. +header with the platform layer: four threads submit two recording copies, a quarter of the calls giving theirs twice, +while another rewrites the arrays their deferred binds read, and no replay sees its handles change while the in-place, +the given-again and the clone path run; then step by step a copy given twice in one call (in place both times, cloned +both times when another submission has it), and the bind resolution (changed and unchanged arrays, binds it leaves +alone, calls 16 deep and not 17, a corrupt record). No GPU, no game files. A change to how `runtime/memory.c` `mem_copy_readable` reads the lazy heap (`NX2NAVI_HEAP_LAZY=1`) passes `python tests\heap_lazy_copy_check.py`, which builds `memory.c` with the Windows platform layer and its fault handler: diff --git a/runtime/nvn_api_cmd.c b/runtime/nvn_api_cmd.c index 8d759eed..9ed95254 100644 --- a/runtime/nvn_api_cmd.c +++ b/runtime/nvn_api_cmd.c @@ -1447,7 +1447,8 @@ static int resolve_deferred(const uint8_t *p) { // returns the number of defer // Synchronous replay. Each copy counts the submissions between this prepare and their nvn_submit_done (busy): the first // in flight resolves the copy in place; one another thread makes while that is replayed resolves a clone of its own -// (nvn_submit_guard.h), unless the copy is known to hold no deferred binds. out: the handles to replay. +// (nvn_submit_guard.h), unless the copy is known to hold no deferred binds. A handle the call gives twice stays in +// place both times when its first occurrence does. out: the handles to replay. void nvn_submit_prepare(const uint64_t *handles, uint64_t *out, int n) { uint64_t now = __atomic_add_fetch(&nsubmits, 1, __ATOMIC_RELAXED), min = copy_minute(), k = 0, cloned = 0; plat_mutex_lock_shared(©_lock); // keeps every copy found here alive while it is cloned @@ -1461,7 +1462,11 @@ void nvn_submit_prepare(const uint64_t *handles, uint64_t *out, int n) { __atomic_store_n(&pf->used_at, now, __ATOMIC_RELAXED); copy_age(pf, min); out[i] = handles[i]; - if (nvn_copy_enter(&pf->busy) || __atomic_load_n(&pf->deferred, __ATOMIC_RELAXED) == 1) continue; + // in place: the first submission in flight, a copy known to hold no deferred binds, or the same handle again in + // this call after an occurrence that resolves in place (nvn_copy_again; only this branch pays for its scan) + if (nvn_copy_enter(&pf->busy) || __atomic_load_n(&pf->deferred, __ATOMIC_RELAXED) == 1 || + nvn_copy_again(handles, out, i)) + continue; size_t sz = sizeof *pf + pf->bytes; copy_prefix *cl = malloc(sz); if (!cl) { diff --git a/runtime/nvn_submit_guard.h b/runtime/nvn_submit_guard.h index 1169fe60..8115b490 100644 --- a/runtime/nvn_submit_guard.h +++ b/runtime/nvn_submit_guard.h @@ -18,10 +18,24 @@ #include "nvn_obj.h" // 1: no other submission has the copy in flight; resolve it in place and call nvn_copy_leave after the replay. 0: -// another one has; take a clone and call nvn_copy_leave once the clone is made. +// another one has, or this call holds it already (nvn_copy_again); else take a clone and call nvn_copy_leave once the +// clone is made. static inline int nvn_copy_enter(uint32_t *busy) { return __atomic_fetch_add(busy, 1, __ATOMIC_ACQ_REL) == 0; } static inline void nvn_copy_leave(uint32_t *busy) { __atomic_fetch_sub(busy, 1, __ATOMIC_RELEASE); } +// After nvn_copy_enter returned 0 for handles[i] of one submit call: whether the same handle came earlier in this call +// and resolves in place there (out[j] == handles[j]). Then the count that made it busy is this thread's own: the copy +// is this submission's to resolve in place, and another thread that submits it meanwhile takes a clone. So this +// occurrence stays in place too: its count is left after the replay like the first one's (nvn_submit_done, per +// occurrence), and no clone is made or logged for it. The +// asynchronous path keeps a repeat in one call in place by its own rule (nvn_api_cmd.c nvn_submit_prepare_async: +// used_at != now). +static inline int nvn_copy_again(const uint64_t *handles, const uint64_t *out, int i) { + for (int j = 0; j < i; j++) + if (handles[j] == handles[i] && out[j] == handles[j]) return 1; + return 0; +} + // Told of a bind whose handles changed, before they are written: b holds the handles at record, now the game's, nth // how many changed binds came before this one (the changed count). typedef void (*NvnBindNote)(const p_bind_handles *b, const uint64_t *now, uint64_t nth); diff --git a/tests/submit_clone_check.c b/tests/submit_clone_check.c index 747d11ad..1519de8e 100644 --- a/tests/submit_clone_check.c +++ b/tests/submit_clone_check.c @@ -1,9 +1,10 @@ // tests/submit_clone_check.py builds this: runtime/nvn_submit_guard.h, a synchronous submission's deferred binds when // another guest thread may be replaying the same recording copy. Four threads submit two hand-built copies while a -// "game" thread rewrites the handle arrays their deferred binds read: a replay must see its copy's handles unchanged -// from its resolve to its end, the in-flight count must be back to 0, and both the in-place and the clone path must -// run. Then nvn_resolve_binds step by step. "old" resolves in place without the guard (0.10.1) and only prints what it -// sees, to show that the harness catches the race. +// "game" thread rewrites the handle arrays their deferred binds read; a quarter of the calls give their copy twice. A +// replay must see its copy's handles unchanged from its resolve to its end, the in-flight count must be back to 0, +// both the in-place and the clone path must run, and a copy given twice in one call must stay in place when its first +// occurrence does. Then nvn_copy_again and nvn_resolve_binds step by step. "old" resolves in place without the guard +// (0.10.1) and only prints what it sees, to show that the harness catches the race. #include #include #include @@ -25,7 +26,7 @@ static uint8_t *copy[COPIES]; static size_t copy_bytes; static uint32_t busy[COPIES]; static volatile int32_t stop; -static uint64_t torn, clones, in_place, reads, changed; +static uint64_t torn, clones, in_place, reads, changed, again; static uint8_t *bind_rec(uint8_t *q, uint32_t op, int first, int n, const uint64_t *src) { *(op_hdr *)q = (op_hdr){op, (uint32_t)(8 + sizeof(p_bind_handles))}; @@ -60,33 +61,42 @@ static void snap(const uint8_t *p, uint64_t out[BINDS][32]) { out[b][i] = __atomic_load_n(&((const p_bind_handles *)(q + 8))->h[i], __ATOMIC_RELAXED); } -// nvn_api_cmd.c nvn_submit_prepare, the replay, nvn_submit_done +// nvn_api_cmd.c nvn_submit_prepare, the replay, nvn_submit_done; handles are copy numbers + 1, a clone's out is 0 static void submitter(void *arg) { uint64_t r = (uint64_t)(uintptr_t)arg * 0x2545F4914F6CDD1Dull + 7; static uint64_t a[THREADS][BINDS][32], b2[THREADS][BINDS][32]; int me = (int)(uintptr_t)arg - 1; for (int k = 0; k < ROUNDS; k++) { r ^= r << 13, r ^= r >> 7, r ^= r << 17; - int c = (int)(r % COPIES); - uint8_t *use = copy[c], *clone = NULL; - int alone = old_mode ? 1 : nvn_copy_enter(&busy[c]); - if (!alone) { - clone = malloc(copy_bytes); - memcpy(clone, copy[c], copy_bytes); - nvn_copy_leave(&busy[c]); - use = clone; - __atomic_add_fetch(&clones, 1, __ATOMIC_RELAXED); - } else { - __atomic_add_fetch(&in_place, 1, __ATOMIC_RELAXED); + int c = (int)(r % COPIES), n = (r >> 8) % 4 == 0 ? 2 : 1; + uint64_t h[2] = {(uint64_t)c + 1, (uint64_t)c + 1}, out[2]; + uint8_t *use[2], *clone[2] = {NULL, NULL}; + for (int i = 0; i < n; i++) { + out[i] = h[i], use[i] = copy[c]; + if (old_mode || nvn_copy_enter(&busy[c])) { + __atomic_add_fetch(&in_place, 1, __ATOMIC_RELAXED); + } else if (nvn_copy_again(h, out, i)) { + __atomic_add_fetch(&again, 1, __ATOMIC_RELAXED); + } else { + clone[i] = malloc(copy_bytes); + memcpy(clone[i], copy[c], copy_bytes); + nvn_copy_leave(&busy[c]); + use[i] = clone[i], out[i] = 0; + __atomic_add_fetch(&clones, 1, __ATOMIC_RELAXED); + } } const uint8_t *bad = NULL; - nvn_resolve_binds(use, 0, &reads, &changed, NULL, &bad); - snap(use, a[me]); // the replay reads the binds at its start ... - for (volatile int s = 0; s < 300; s++) {} - snap(use, b2[me]); // ... and at its end (or a held continuation copies them) - if (memcmp(a[me], b2[me], sizeof a[me])) __atomic_add_fetch(&torn, 1, __ATOMIC_RELAXED); - if (clone) free(clone); - else if (!old_mode) nvn_copy_leave(&busy[c]); + for (int i = 0; i < n; i++) nvn_resolve_binds(use[i], 0, &reads, &changed, NULL, &bad); + for (int i = 0; i < n; i++) { + snap(use[i], a[me]); // the replay reads the binds at its start ... + for (volatile int s = 0; s < 300; s++) {} + snap(use[i], b2[me]); // ... and at its end (or a held continuation copies them) + if (memcmp(a[me], b2[me], sizeof a[me])) __atomic_add_fetch(&torn, 1, __ATOMIC_RELAXED); + } + for (int i = 0; i < n; i++) { + if (clone[i]) free(clone[i]); + else if (!old_mode) nvn_copy_leave(&busy[c]); + } } } @@ -97,15 +107,16 @@ static void stress(void) { for (int i = 0; i < THREADS; i++) plat_thread_join(t[i]); __atomic_store_n(&stop, 1, __ATOMIC_RELEASE); plat_thread_join(g); - printf("submit_clone_check: %s: %d submissions, %llu in place, %llu clones; %llu deferred binds read, %llu changed; " - "%llu replays saw their handles change under them\n", old_mode ? "old (prints only)" : "new", - THREADS * ROUNDS, (unsigned long long)in_place, (unsigned long long)clones, (unsigned long long)reads, - (unsigned long long)changed, (unsigned long long)torn); + printf("submit_clone_check: %s: %d submit calls, %llu copies in place, %llu of them given again in their call, " + "%llu clones; %llu deferred binds read, %llu changed; %llu replays saw their handles change under them\n", + old_mode ? "old (prints only)" : "new", THREADS * ROUNDS, (unsigned long long)(in_place + again), + (unsigned long long)again, (unsigned long long)clones, (unsigned long long)reads, (unsigned long long)changed, + (unsigned long long)torn); if (old_mode) return; for (int c = 0; c < COPIES; c++) CHECK(!busy[c], "copy %d left with %u submissions in flight", c, busy[c]); CHECK(!torn, "%llu replays saw their handles change under them", (unsigned long long)torn); - CHECK(in_place && clones, "a path never ran (%llu in place, %llu clones)", (unsigned long long)in_place, - (unsigned long long)clones); + CHECK(in_place && clones && again, "a path never ran (%llu in place, %llu again in place, %llu clones)", + (unsigned long long)in_place, (unsigned long long)again, (unsigned long long)clones); } // ---- nvn_resolve_binds, one thread @@ -116,7 +127,29 @@ static void note(const p_bind_handles *b, const uint64_t *now, uint64_t nth) { if (b->h[0] == now[0]) printf("submit_clone_check: FAIL: a note came after the handles were written\n"), fails++; } +// nvn_copy_again with nvn_copy_enter as nvn_submit_prepare uses them, one thread: occurrence i of handles h stays in +// place (1) or is cloned (0); a clone's out is 0 and leaves its count at once. +static int prepare_one(uint32_t *b, const uint64_t *h, uint64_t *out, int i) { + out[i] = h[i]; + if (nvn_copy_enter(b) || nvn_copy_again(h, out, i)) return 1; + nvn_copy_leave(b); + out[i] = 0; + return 0; +} + static void steps(void) { + // a copy given twice in one call with nothing else in flight: both in place, two counts, both left after the replay + uint32_t b = 0; + uint64_t h[3] = {11, 22, 11}, out[3]; + int p0 = prepare_one(&b, h, out, 0), p1 = prepare_one(&(uint32_t){0}, h, out, 1), p2 = prepare_one(&b, h, out, 2); + CHECK(p0 && p1 && p2 && b == 2, "a copy given twice in one call was cloned (%d %d %d, %u in flight)", p0, p1, p2, b); + nvn_copy_leave(&b), nvn_copy_leave(&b); + // another thread's submission in flight: both occurrences cloned, the count back to that one's + b = 1; + p0 = prepare_one(&b, h, out, 0), p2 = prepare_one(&b, h, out, 2); + CHECK(!p0 && !p2 && b == 1, "a copy in flight elsewhere was resolved in place (%d %d, %u in flight)", p0, p2, b); + CHECK(!nvn_copy_again(h, out, 0), "the first occurrence was taken for a repeat"); + static uint64_t arr[4][32], called_arr[32]; static uint8_t buf[2048], callee[512], chain[18][16]; for (int i = 0; i < 32; i++) arr[0][i] = 100 + i, arr[1][i] = 200 + i, called_arr[i] = 300 + i; diff --git a/tests/submit_clone_check.py b/tests/submit_clone_check.py index ff3788a2..c3575419 100644 --- a/tests/submit_clone_check.py +++ b/tests/submit_clone_check.py @@ -1,8 +1,10 @@ """Build and run tests/submit_clone_check.c: runtime/nvn_submit_guard.h, a synchronous submission's deferred binds when another guest thread may be replaying the same recording copy (nvn_api_cmd.c nvn_submit_prepare, nvn_submit_done). -Four threads submit two hand-built copies 200,000 times each while a "game" thread rewrites the arrays their deferred -binds read: no replay sees its copy's handles change between its resolve and its end, the in-flight counts end at 0, -and both the in-place and the clone path run. nvn_resolve_binds step by step: a changed array is copied in and counted, +Four threads submit two hand-built copies 200,000 times each, a quarter of the calls giving their copy twice, while a +"game" thread rewrites the arrays their deferred binds read: no replay sees its copy's handles change between its +resolve and its end, the in-flight counts end at 0, and the in-place, the given-again and the clone path run. +nvn_copy_again step by step: a copy given twice in one call stays in place both times, and is cloned both times when +another submission has it in flight. nvn_resolve_binds step by step: a changed array is copied in and counted, an unchanged one is not counted, binds without src, with n 0 or over 32 stay untouched, OP_CALL is followed 16 deep and refused at 17, and a corrupt record is reported where it is. Then the unguarded 0.10.1 shape runs through the same harness and only prints what it saw, to show that the harness catches the race.