Repository navigation
fix(agc): run the unwatch hook when a watched import leaves the write watch - #6308
LeandroLP23 wants to merge 2 commits into
Conversation
… watch An import that should stay watched still drops its range from the watch when the range was only partly watched or its import window cannot be resolved. That fallback skipped the unwatch hook, so a storage image with results pending in the range lost the watch without its guest bytes saved, and its next store threw "storing storage image results pending over memory outside the write watch without the guest bytes saved before them is not implemented". Since boykopovar#6060 settled private imports on Windows take this path, and ProsperoPuzzles (PPSA99006) stopped 5 s in (boykopovar#6301). The fallback now releases the tracker lock, runs the hook over the page range, and unwatches it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
NVIDIA run, for the "Not tested: NVIDIA, Intel, Linux, other titles." line. Windows 11 (26200), RTX 3060 Laptop GPU (NVIDIA 617.42), WinLibs GCC 15.2.0 r7 (UCRT), Release,
Not run: ProsperoPuzzles or any other title, Intel, Linux. AI-assisted: yes (Claude Code) |
… watch A MarkWritten whose fresh collect fails (a no-access or unwatchable resident page inside the 64 KiB block span) took the span out of the write watch without running the unwatch hook, so pending storage-image results over it kept no saved guest bytes and their write-back refused: storing storage image results pending over memory outside the write watch without the guest bytes saved before them is not implemented (boykopovar#7017, Palworld PPSA23126 among others). The hook now runs over the block span before the blocks leave the watch, as Unwatch and the import fallback do, with the tracker lock released for it like boykopovar#6308 does for imports. The debt entry drops the failed collect from the unsaved cases. The check makes a collect fail over a no-access arena page and requires the hook to have covered the block; it needs the WindowsMappings protection failure, so it runs in the Windows-only section of the write tracking test.
|
Still needed on current
Both AI-assisted |
… watch A MarkWritten whose fresh collect fails (a no-access or unwatchable resident page inside the 64 KiB block span) took the span out of the write watch without running the unwatch hook, so pending storage-image results over it kept no saved guest bytes and their write-back refused: storing storage image results pending over memory outside the write watch without the guest bytes saved before them is not implemented (boykopovar#7017, Palworld PPSA23126 among others). The hook now runs over the block span before the blocks leave the watch, as Unwatch and the import fallback do, with the tracker lock released for it like boykopovar#6308 does for imports. The debt entry drops the failed collect from the unsaved cases. The check makes a collect fail over a no-access arena page and requires the hook to have covered the block; it needs the WindowsMappings protection failure, so it runs in the Windows-only section of the write tracking test.
github-actions
Bot
added
the
unblocks title
|
Marvel's Wolverine (PPSA03671) stops on this Setup: Windows 11 Pro 26300, RTX 4080 SUPER 16 GB (NVIDIA 617.42), Ryzen 7 5800X3D, 32 GB RAM, WinLibs GCC 15.2.0 r7, Release. Content 01.001.005;
Not checked: anything past the splash card, Linux. AI-assisted: yes |
… watch A MarkWritten whose fresh collect fails (a no-access or unwatchable resident page inside the 64 KiB block span) took the span out of the write watch without running the unwatch hook, so pending storage-image results over it kept no saved guest bytes and their write-back refused: storing storage image results pending over memory outside the write watch without the guest bytes saved before them is not implemented (boykopovar#7017, Palworld PPSA23126 among others). The hook now runs over the block span before the blocks leave the watch, as Unwatch and the import fallback do, with the tracker lock released for it like boykopovar#6308 does for imports. The debt entry drops the failed collect from the unsaved cases. The check makes a collect fail over a no-access arena page and requires the hook to have covered the block; it needs the WindowsMappings protection failure, so it runs in the Windows-only section of the write tracking test.
|
Thanks for this and for the title reports. #8172 (the fix for #8103) has now merged and rewrites the Either way, the hook is still missing on the path that remains: on #8172, a covered span whose CPU walk fails before the import (dropped before
#7505 (@AlxcBTW) contains this PR plus the save-early change, so it'd be simplest to land one combined PR. Please agree between you which branch carries it. |
|
Rechecked on current ProsperoPuzzles (PPSA99006):
|
|
Superseded by #8172, so I'm closing this. Data is in #6301: ProsperoPuzzles stops at 4 s on #8172's parent (e9cc4a8, 0 of 2 alive at 60 s), and runs with #8172 (2 of 2) and on current With #8172, a stays-watched import keeps the spans that were watched before it. A span leaves the watch only when the walk after the import fails ( AI-assisted: yes (Claude Code) |
|
ProsperoPuzzles (PPSA99006) on Written with Claude Code assistance. |
… watched import leaves the write watch) at 93df5c4
…lready fails An import that should stay watched runs the unwatch hook only when its range leaves the write watch, after the import. When the range is partly outside the watch or the walk before the import fails, the range leaves the watch whatever the import does, so the hook can run first, as it does for the import that does not stay watched. After the import, the walk that tells an import's own page marks from CPU writes (the window compare) is skipped in this case, so the hook collects those marks as CPU writes. A driver that marks the pages of an import written (the NVIDIA driver on Windows does, see Pages under an import window) makes the hook keep the blocks they cover for the CPU unless their bytes are proven unchanged, and those blocks do not receive the GPU results. A failure of the walk after the import still runs the hook after it. The check of #6308 also requires the hook to have run before the import.
What
Fixes #6301. Since #6060, ProsperoPuzzles (PPSA99006) stops 5 s in on Windows:
GuestMemory::ImportWatchedwithstaysWatchedstill drops the range from the write watch when the range was only partly watched (beforeis false) or the import window can't be resolved. That fallback calledunwatchLockedwithoutbeforeUnwatch, so the storage-image hook (StorageTexture::SaveBeforeUnwatch) never saved the guest bytes of results pending there. The!staysWatchedpath always runs the hook first.Before #6060, Windows direct imports never took this path. Now settled private imports do. In ProsperoPuzzles an 8 MiB import at
0x21f800000took the fallback with the range only partly watched, and unwatched the storage image at0x21fc00000with results pending.The fallback now releases the tracker lock, runs the hook over the page range, takes the lock again and unwatches. The hook takes the pending-texture mutex and
GpuMutex(and, with #5671, the tracker lock), so it can't run under the tracker lock.agc_write_trackinggetsCheckImportWindowOverUnwatchedPart. It unwatches the middle block of three, imports all three withstaysWatched, and requires the hook to have covered the range.This complements #5671, which widens the span the hook gets to whole 64 KiB blocks. The two touch different lines.
Tested
Windows 11 (26300), Radeon 860M (RDNA 3.5, driver 32.0.13058.8001), WinLibs GCC 15.2.0, Release, on
maine49ec591d.--windows --trap-sdk-syscall-stubs, new run folder, 60 s, no input): 3 of 3 alive at 60 s, 19.0–19.2 FPS. Onmain, every run stops at 5–7 s (data in AGC: since #6060, ProsperoPuzzles (PPSA99006) stops at 5 s with the storage-image write-back throw on Windows #6301). Before perf(agc): keep Windows host imports of write-watched private memory watched #6060 it ran at 9.5–13.7 FPS.agc_write_trackingpasses. With theGuestMemory.cppchange reverted, the new check fails:an import that left the write watch did not run the unwatch hook over its range.ctest -R '^agc_|guest_memory|write_watch' -j 1from an emptytests/shader_cache: 583 of 585 pass. The two failures fail onmainhere too:agc_driver_quad_pixel_mask(fix(shader): kill demoted quad-mask pixels when the pixel program returns #6255) andagc_driver_layered_volume_vertex_no_gpl(AMD Windows: GPL-mode cache failures (alpha/wave32 pass after local driver update) #4768).tools/check_conventions.py --base origin/mainpasses;git diff --checkis clean.Fork CI (run): Windows 1264 of 1265 and Linux 1241 of 1242, the one failure in each being the fork-only
conventions_ci_gate. The first Linux attempt also failedagc_driver_preparation_scope_settle(the link did not wait for the vertex shader's registration preparation), a shader-registration timing test that uses no imports. It passed on the job rerun and 10 of 10 locally.Not tested: NVIDIA, Intel, other titles.
Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)