Repository navigation
fix(agc): hold a guest allocation lease while a queue stores idle results - #6246
Conversation
…ults Once every queue has been idle for 50 ms, the queue 0 worker stores pending storage image results on its own (boykopovar#3777, boykopovar#4410). That store runs at no ordering point of the guest and took no lease of the guest allocations, so a guest unmap could run alongside it. The compare of memory outside the write watch (StorageTexture::compareUntracked) then read memory the unmap had released: the driver forgets its cached page states only when the unmap's registry mutation ends, after the memory is gone, so the range still read as mapped. The idle store now holds a lease (GuestAllocationsAcquire) under the GPU mutex, the order the other lease holders in the driver use. An unmap waits in GuestAllocationsRequireUnpinned until the store is done, and a store that starts during an unmap waits for the mutation to end, finds the range no longer registered and drops its results. ProsperoEden (PPSA99008, homebrew catalog, run without app0/eboot.bin) closes its launcher, its GL driver unmaps its direct memory at 0x200000000+0x14000000 while both 1920x1080 color buffers there still have results pending, and the queue worker faulted in compareUntracked under FlushAllPending("idle"): FATAL: unhandled exception 0xc0000005 on thread 69080 'queue worker 0x0' read of 0x0000000200000000 On main d92a8da that happens in 3 of 3 cold runs. With this change 6 of 6 runs exit with code 0 and log "storage image 0x200000000+0x10e0000: memory no longer registered; 1 pending ranges dropped" for both images. New test agc_driver_idle_store_over_unmap unmaps an untracked arena surface with a pending result and releases the GPU mutex while the unmap is in progress. Without the lease the queue worker faults at the same compare (3 of 3); with it the result stays pending until the unmap ends and is then dropped. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
Intel and AMD Windows data: the new test passes on both GPUs with this change and crashes on both without it, as on the RTX 4070. Windows 11 Pro 26300, WinLibs GCC 15.2.0 r7, Release, every execution test built,
Full CTest ( AI-assisted: yes (Claude Code) |
|
Thanks @savvaniss for the Intel UHD 770 and RX 9070 XT runs, including the without-fix crash on both. I added them to Tested and GPUs tested, and added the |
What
Once every queue has been idle for 50 ms, the queue 0 worker stores pending storage image results on its own (#3777, #4410). That store took no lease of the guest allocations, so a guest unmap could run alongside it, and the compare of memory outside the write watch (
StorageTexture::compareUntracked,Texture.cpp:2106) read memory the unmap had released: the driver forgets its cached page states only when the unmap's registry mutation ends (GuestAllocationsEnd→ForgetPages), after the memory is gone, soAccessible()still answered yes.StoreIdleResults(VulkanDevice.cpp:1916) now holds aGuestAllocationsAcquirelease under the GPU mutex, in the order the driver's other lease holders use. An unmap waits inGuestAllocationsRequireUnpinneduntil the store is done; a store that starts during an unmap waits for the mutation to end, finds the range no longer registered and drops its results through the existing path. One line in the driver plus a new GPU test,agc_driver_idle_store_over_unmap(about 1 s), which unmaps an untracked arena surface with a pending result while the GPU mutex is released; it faults at the same compare 3 of 3 without the lease and passes 3 of 3 with it.Title: ProsperoEden PPSA99008 (homebrew catalog, #1578), relinked with
--trap-sdk-syscall-stubsand run withoutapp0/eboot.bin. Its launcher cannot read its font, quits, and its GL driver unmaps its direct memory at0x200000000+0x14000000while both 1920x1080 color buffers there still have results pending. Onmaind92a8dad3, 3 of 3 cold runs:Order of events with
APS5_TRACE_MEMORY=1 APS5_TRACE_FLUSH=1:With this change the title exits with code 0 in 6 of 6 runs (3 cold, 3 traced) and logs
storage image 0x200000000+0x10e0000: memory no longer registered; 1 pending ranges droppedfor both images. Withapp0/eboot.binstaged the launcher shows its menu and renders at 59.9 FPS for 60 s with and without the change (3454 vs 3325 frames).For reviewers: the lease pins every registered range, so a guest unmap or heap free waits a few ms while an idle write-back runs. Write-backs on other workers that already run without a lease are unchanged. A guest that unmaps a surface and maps new memory at the same address before the idle store runs still gets the stale results written over the new memory; that was already the case and is not addressed here.
Tested
Windows 11 Pro 26220, Ryzen 7 5700X, NVIDIA RTX 4070 (driver 596.21), WinLibs MinGW-w64 GCC 15.2.0 r7, Ninja, Release,
BUILD_TESTING=ON, full build includinglibs, onmaind92a8dad3.agc_driver_idle_store_over_unmap: 3 of 3 pass with the change; 3 of 3 fault at the same address and frames against unfixedmain's driver.agc_driver_store_idle_results,agc_driver_store_over_untracked: 3 of 3.agc_driver_*andvideo_out_flip*: 440 of 440 passed, 12 skipped.ctest -j8 --timeout 120 -E "^agc_driver_|^video_out_flip": 811 of 814 passed, 2 skipped;hardware_oracle_comparetimed out under load and passes alone in 8.7 s.python tools/check_conventions.py --base main,git diff --check: pass.agc_driver_idle_store_over_unmap3 of 3 pass with the change on both GPUs and crashes 3 of 3 without it on both.Checklist
main; no other open PR implements the same functions.mdfiles or images added to the repository🤖 Generated with Claude Code