diff --git a/runtime/nvn_api_cmd.c b/runtime/nvn_api_cmd.c index 5ff7ec8e..1ba31c6e 100644 --- a/runtime/nvn_api_cmd.c +++ b/runtime/nvn_api_cmd.c @@ -557,12 +557,13 @@ static uint8_t *flatten_recording(const uint8_t *p, size_t n) { // memory (cmd_t.ctl is accounting only), so the 16 bytes at a copy's control position are whatever the game left // there, and a fingerprint of them that no longer matches says the game wrote there since. A reuse that writes the // same bytes (zeros, often) is not seen: the counts are a lower bound. -// The "nvn copy sweep" thread does every read of guest memory, at its lowest priority: a recording thread taking the -// fingerprint at EndRecording could fault into the lazy heap (an untouched control block) and wait there. Every -// RETIRE_PERIOD_MS it fingerprints the copies made since its last pass (a queue copy_add fills, under retire_lock), -// then visits up to RETIRE_BATCH of the copies it knows, from a cursor, and counts those no submission used for -// COPY_IDLE_MINS whose bytes changed or whose memory is gone. It takes copy_lock shared only, RETIRE_CHUNK lookups at a -// time; a copy that is no longer live (retired, or its handle reused: made_at and bytes differ) leaves its records. +// The "nvn copy sweep" thread does every read of guest memory, at normal priority (retire_main says why): a recording +// thread taking the fingerprint at EndRecording could fault into the lazy heap (an untouched control block) and wait +// there. Every RETIRE_PERIOD_MS it fingerprints the copies made since its last pass (a queue copy_add fills, under +// retire_lock), then visits up to RETIRE_BATCH of the copies it knows, from a cursor, and counts those no submission +// used for COPY_IDLE_MINS whose bytes changed or whose memory is gone. It takes copy_lock shared only, RETIRE_CHUNK +// lookups at a time; a copy that is no longer live (retired, or its handle reused: made_at and bytes differ) leaves its +// records. #define RETIRE_PERIOD_MS 5000 #define RETIRE_BATCH 4096 #define RETIRE_CHUNK 64 @@ -677,9 +678,15 @@ static void retire_pass(void) { } } +// The sweep runs at normal priority, as the threads it shares locks with: nearly every step of a pass holds one they +// take (copy_lock shared, which copy_add, release_copies and the submission take exclusive; retire_lock, which +// retire_queue takes on the recording thread; memory.c's lock through mem_read_hold and lz_lock through +// mem_lazy_touches). A holder below their priority, preempted by any thread above its own (the pipeline workers at +// BELOW_NORMAL are enough), would keep the recording and submitting threads waiting until the scheduler's starvation +// boost ran it again, seconds later. A pass is at most RETIRE_BATCH lookups and 16-byte reads every RETIRE_PERIOD_MS. static void retire_main(void *arg) { (void)arg; - plat_thread_priority(PLAT_PRIO_LOWEST); + plat_thread_priority(PLAT_PRIO_NORMAL); // set, not inherited from the recording thread that started it for (;;) { plat_sleep_ms(RETIRE_PERIOD_MS); retire_pass(); diff --git a/tests/copy_sweep_check.py b/tests/copy_sweep_check.py index db234b13..1d30ecb3 100644 --- a/tests/copy_sweep_check.py +++ b/tests/copy_sweep_check.py @@ -9,7 +9,9 @@ table of live copies and stub guest memory, and checks that without a read; - a retired copy, or one whose handle a new copy took, leaves its records and the counts; - the visits go on where the last pass stopped: two passes over 5000 copies look each up at least once and none - more than twice. + more than twice; + - the sweep thread does not lower its priority below normal (retire_main), as nearly every step of a pass holds a lock + the recording and submitting threads take. No game files. Usage: python tests/copy_sweep_check.py [--cc ] """ @@ -151,6 +153,12 @@ def main(): env['PATH'] = str(MSYS_BIN) + os.pathsep + env.get('PATH', '') src = (ROOT / 'runtime' / 'nvn_api_cmd.c').read_text(encoding='utf-8') body = cut(src, '// ---------- recording copies whose control memory the game reused', '\nstatic void retire_main(') + sweep = cut(src, '\nstatic void retire_main(', '\n}\n') + lowered = [p for p in ('PLAT_PRIO_IDLE', 'PLAT_PRIO_LOWEST', 'PLAT_PRIO_BELOW_NORMAL') if p in sweep] + if lowered: + print(f'FAIL: retire_main lowers the sweep thread\'s priority ({", ".join(lowered)}); it holds locks the ' + 'recording and submitting threads take') + return 1 with tempfile.TemporaryDirectory(prefix='nx2navi-copy-sweep-') as tmp: c = Path(tmp) / 'check.c' c.write_text(HARNESS % body, encoding='utf-8')