[janzezno]

What

A host read now waits for a storage image store that another thread recorded while the read's flush hook was running.

FlushForAccess checks the recorder's pending-write snapshot first. Only after that does it look for overlapping images, pending (AccessKeptByCpu / FlushPending) or flushing (WaitForFlushing, from #7744's fix). A flusher moves an image the other way: pending, then flushing, then recorded into the open batch (which puts it in the snapshot), then landed. So a reader can miss the store:

1. The reader checks the snapshot while the flusher holds the image in flushing. The store isn't recorded yet, so the snapshot doesn't overlap, and the reader skips the sync.
2. The flusher's writeBack records the copy into the open batch, and Unregister removes the image from flushing.
3. The reader's WaitForFlushing finds flushing empty and returns false. stored and published stay false, so the final SyncThrough doesn't run, and the CPU reads guest memory before the recorded copy has run.

The final sync now also runs when the snapshot overlaps the range after the image checks. The reader then checks each state in the order the image goes through them, so an image it doesn't find pending or flushing is either in the snapshot or already landed. When nothing is recorded over the range, the extra cost is one lock-free snapshot lookup.

This is the remaining cause of the agc_driver_storage_read_during_idle_store flake. #7984 is related but does not overlap. It makes the idle flusher sync its own unit-shadow publish, but this window is on the reader's side, for a store that writeBack recorded. With #7984 merged and without this change, the forced-delay run below still has 20 of 20 stale reads. The two PRs touch different files.

pr_overlap.py could not compare 13 open PRs that conflict with main and share Recorder.cpp, so I checked their diffs by hand. Only #6682 changes FlushForAccess. It changes the waited argument passed to PublishShadowsOnly and FlushPending, in the lines just above the condition this PR changes. It does not change the condition itself. Whichever merges second needs a one-line textual merge; neither changes what the other does. #7464's hunk adds code after the function. The other 11 do not touch FlushForAccess.

Tested

Arch Linux, kernel 7.2.11, AMD Radeon RX 7900 GRE (RADV NAVI31), Mesa 26.2.5, GCC 16.2.1, Release, BUILD_TESTING=ON, main eea640a5b.

The window can be forced with a local 5 ms delay after the snapshot block of FlushForAccess, on threads that do not hold the GPU mutex (the idle thread's own store also goes through the hook). The delay is not part of this PR:

             recorder->SyncThrough(address, bytes, true);
         }
     }
+    if (const char* delay = std::getenv("APS5_DEBUG_HOOK_DELAY_MS"); delay != nullptr && bytes >= 0x20000 && !GuestMemory::GpuMutex().HeldByThisThread()) std::this_thread::sleep_for(std::chrono::milliseconds(std::atoi(delay)));

agc_driver_storage_read_during_idle_store:

	main	this PR
forced 5 ms delay	18 of 20 and 20 of 20 stale reads (fails every run)	0 of 20 on 6 runs (5 ms and 50 ms)
no delay, 12 in parallel	1 of 300 runs had a stale read	0 of 600 runs
no delay, single run	passes	passes

Instrumenting the hook on main showed the failing read taking snapshot overlaps = 0, images = 0, stored = 0, published = 0 after the idle thread's flush ended. With the change, the same read syncs.

ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1 VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation ctest --test-dir build -R 'agc|shader|recompiler|request_serializer' -V:

• main: 842 of 845 pass. This PR: 842 of 845 pass.
• The same three fail on both, and are tracked elsewhere: agc_driver_guest_memory_lifetime (agc_driver_guest_memory_lifetime fails on RADV since the host-import cut larger-side keep (16473dbbe, #6499 family) #7784), agc_driver_image_array_window_past_layer_limit (test(agc): match array sample expectations to the view #8361), agc_driver_tessellation (Mesa, agc_driver_tessellation segfaults on RADV with Mesa 26.2+: nir_opt_varyings corrupts the TCS input offset's use list (Mesa bug) #5444).
• The same VUID- set and counts on both, none new: 00118 ×2, 02251 ×2, 02275 ×6, 00605 ×26, 01123 ×2. Neither log contains was not found but was requested.

Checklist

• Based on current main; no other open PR implements the same functions
• One topic per PR; follow-ups go in a new PR
• No comments in code except technical debt
• Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
• No notes, investigation .md files or images added to the repository (attach them to this PR)
• New third-party code is a submodule built from source
• Generic behaviour, not specific to one title
• Depends on: none
• Fixes: none
• Unblocks: none
• GPUs tested: AMD Radeon RX 7900 GRE, RADV (Mesa 26.2.5), Arch Linux (kernel 7.2.11)
• AI-assisted: yes

[MarcZX10]

Reviewed head 89f829b5d8356783431796408cf27246422e422e. The one-line final SnapshotOverlaps check appears to close the documented transition window: a writer can leave flushing after the first snapshot check but before this reader's WaitForFlushing, so the reader must re-check the published write ranges before deciding not to sync. The check is lock-free and the SyncThrough below remains range-targeted. The forced-delay and no-delay counts in the description provide useful regression evidence. Hosted eligibility, conventions, intake, labels, and report checks pass; Linux/Windows/runtime jobs are skipped, so this is not full platform CI validation.

[MarcZX10]

Reviewed the current head 89f829b5d8356783431796408cf27246422e422e. The final atomic snapshot-overlap recheck appears to close the documented transition from flushing to recorded writes before the reader decides whether to sync. The range-targeted SyncThrough remains intact. The PR reports a forced-delay regression that fails on main and passes repeatedly with the change, plus no-delay runs; eligibility, conventions, intake, labels, and report checks pass. Hosted Linux/Windows and runtime jobs are skipped, so those are not independently validated here.

[github-actions]

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.

Pull request	Shared files not compared
PR 5431	core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp
PR 5609	core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp
PR 6186	core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp
PR 6292	core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp
PR 6539	core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp
PR 6615	core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp
PR 7622	core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp
PR 7642	core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp
PR 8963	core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp
PR 9356	core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp
PR 9387	core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp
PR 9400	core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp
PR 9508	core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp
PR 9898	core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp

[oneandonlydean]

@janzezno thanks, the window you describe matches the order of the checks in FlushForAccess, and the forced-delay numbers are convincing. One case the new condition reaches that it didn't before: completion actions.

A copied buffer's write-back store runs inside finish() (with completionDepth set) and reaches this hook. A range leaves the snapshot only after its batch's completions have run, so during a batch's completion its own write ranges and every later in-flight batch's are still in pendingWrites. SnapshotOverlaps is then true for that write-back, and the final block takes the completionDepth != 0 branch, which calls the full Recorder::Sync(). Unlike SyncThrough, Sync() has no completion guard: it finishes every later batch in flight first, and then the completing batch's older write-back lands over their bytes. That is the order SyncThrough deliberately skips there (the comment above its completionDepth check). Before this change that branch was taken only when a storage image was stored or published.

Since the race you are fixing is on reader threads, could the new term exclude completion actions, e.g. stored || published || (completionDepth == 0 && SnapshotOverlaps(address, bytes))?

[janzezno]

@oneandonlydean thanks, you're right. A batch's write ranges are republished only by the scope guard at the end of finish(), after its completions have run. So a completion's own write-back matches SnapshotOverlaps and now reaches the completionDepth != 0 → Sync() branch, which skips the completion guard in SyncThrough. Readers are the only callers this race needs, so I've restricted the new term as you suggested:

if (stored || published || (completionDepth == 0 && SnapshotOverlaps(address, bytes))) {

Inside a completion action the branch is now taken only for a stored or published image, as on main. The agc suite gives the same results as main on my machine.

[savvaniss]

AMD Windows data with the validation layer: with the 5 ms hook delay from the description, agc_driver_storage_read_during_idle_store fails 5 of 5 on main and passes 5 of 5 with this PR on an RX 9070 XT. The recorder suite passes on both, with the same VUID- errors.

Windows 11 Pro 26300, WinLibs GCC 15.2.0 r7, Release, AMD Radeon RX 9070 XT (1002:7550), driver 32.0.31041.1004, with VkCache emptied first, Vulkan SDK 1.4.363.0. Every run had ANYPS5_GPU=AMD, ANYPS5_REQUIRE_VULKAN=1, ANYPS5_NO_SHADER_CACHE=1 and VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation, serial ctest -V. No log has was not found but was requested by env var VK_INSTANCE_LAYERS. Every device the tests open is the RX 9070 XT (Selected GPU and Vulkan device: lines). This head 1fcfacb27e merged into main 0cece6ebbc (clean), compared with the same main.

Tests	main 0cece6ebbc	This head
agc_driver_storage_read_during_idle_store and the 77 ctest entries of agc_driver_recorder_tests, ×3	78 of 78 pass	78 of 78 pass
VUID- IDs	VUID-VkImageViewCreateInfo-usage-02275, only in agc_sampled_storage_base_level_usage	the same, in the same test
agc_driver_storage_read_during_idle_store with the delay line from the description and APS5_DEBUG_HOOK_DELAY_MS=5, ×5	fails 5 of 5: AGC graphics: idle store read: a host read did not wait for the image another thread was storing	passes 5 of 5

The delay line is the one in the description, added after the snapshot block of FlushForAccess on both sides; it is not part of the PR. agc_idle_publish_wait skips its idle publish case on both sides on this machine (host import of the idle publish test block refused or not watched).

AI-assisted: yes (Claude Code)

[janzezno]

@savvaniss thanks for the Windows/AMD run. It's good to see the forced-delay result hold on a second platform, with no new VUIDs.

[MarcZX10]

The latest head incorporates the completion-depth guard suggested in the thread. In the recorded-write case, SyncThrough now runs only for readers (completionDepth == 0), while completion actions retain the previous stored/published behavior; this resolves the reentrancy concern from the preceding review. The PR reports forced-delay red/green results on Linux/RADV and Windows/AMD under validation, with no new VUIDs. Local diff whitespace and conventions pass. I have not run Vulkan tests on this host.

One current-main integration blocker remains: tools/check_main_sync.py --base upstream/main --head <head> reports 2 commits ahead / 1897 behind current upstream/main (merge base eea640a5b), though it merges cleanly. CONTRIBUTING requires branches to be kept up to date. Please merge current main and rerun the focused reader/recorder tests with the validation layer on the resulting head. This request is about integration freshness, not intake volume.