Repository navigation
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>
…ed range drops it A storage image has results pending over a block whose neighbour block already left the write watch. An import of both blocks that should stay watched cannot vouch for its pages, so the range leaves the watch, and the image's next store has to find the guest bytes the unwatch hook saved. agc_storage_unwalked_import (Linux) runs that flow through HostImportFor. The host import of the first creation is refused through the import budget so that the import under test is the first one over the block. staleGenerationTests runs the same flow for an import that does not stay watched; it now exists for both.
…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 boykopovar#6308 also requires the hook to have run before the import.
…m the write watch CheckUnwatchHooks runs Unwatch, an import that does not stay watched, an import whose page walk vouches for its pages and, on Linux, an import whose range loses a block during the import. The hook has to run once, over the dropped range, while the range is still watched: before the import for the first two, after it for the last, and not at all for the vouched import.
APS5_TRACE_WATCH prints each exclusion from the CPU write watch with its site and whether pending storage results were saved first, skipped saves, and, for each pending unit stored without saved bytes, the exclusion that left its blocks unwatched. APS5_TRACE_WATCH=unsaved prints only exits that left watched blocks without a save and the failures they cause. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HAtTu2pw6GTkMNwsjBDatU
The cases where a surface leaves the write watch print that they were not run on Windows, where host imports of the guest mapping are compared and never write-watched. Registering the test again with APS5_NO_HOST_IMPORT keeps the surface write-watched so they execute. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HAtTu2pw6GTkMNwsjBDatU
|
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.
|
github-actions
Bot
added
build
docs
agc
12 tasks done
Contributor
|
Rebase handoff — rebased onto current Verified on Linux (CachyOS, kernel 7.2.9, GCC 16.2.1, Ninja, Release, To pick it up: Head is |
12 tasks done
Collaborator
|
@spiff637 thanks for this. The branch sits on an old head of #7505, which carries #6308's commit. #6308 has since closed as superseded by #8172, and #8172 rewrote the Could you rebase c1c13de and 0e5269c straight onto current main, without #7505 or #6308 underneath? Then map the trace sites to main's exits:
On main, the two span exits run without the unwatch hook, so |
Contributor
|
Hey @spiff637, following the standalone-rebase request, I put together a port of your two diagnostic commits onto The port maps the before/after walk failures onto main's span exits, keeps never-watched gaps out of the exit history, and labels hook invocation rather than claiming that bytes were saved. The storage safety refusal remains intact. Both full Windows builds and the libs targets passed with #8310's This validates the pinned revision, not later main commits or the full registered test suite. NHL 26, Linux/macOS, and Texture's actual unsaved-unit report and 16-unit cap notice weren't exercised; timing neutrality isn't established. The local candidate is ZIP SHA-256: |
Contributor
|
Quick follow-up on the coverage gap in my earlier handoff: the real unsaved-unit report and its 16-entry limit are now exercised by a synthetic Windows fixture. The test uploads watched images without CPU snapshots, unwatches them while idle, completes a Vulkan clear and reads back a changed pixel, then marks the results pending. Each bounded flush has to hit the existing saved-byte safety refusal. Every attempt checks the entire 3 MiB guest allocation is unchanged, the results remain pending, and no comparison snapshot appeared. Three fresh processes cover tracing unset, full and I also tested deliberately wrong caps of 15 and 17, and a removed safety guard. Each built and failed the intended assertion. Those temporary changes were restored byte for byte before the final passing build; the attached commit changes tests only. The separate test commit is Final default Release build, This remains validation of the pinned diagnostic tree based on ZIP SHA-256: |
12 tasks done
12 tasks done
12 tasks done
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds
APS5_TRACE_WATCH, a trace of every range that leaves the CPU write watch, and runs the store-over-untracked leave cases on Windows.Unwatch,ImportLeave,ImportStaysUncovered,ImportStaysWalkFailed,MarkWritten) and whether the unwatch hook saved pending storage results first.saveBeforeUnwatchreports a failed collect and each unit it leaves unsaved, andforgetSavedOverreports clearing a pending unit's saved bytes.unsavedOutsideWatchRequire, up to 16 lines name each unit stored without saved bytes and the exclusion that left its blocks unwatched. That is recorded per 64 KiB block, and only while tracing.APS5_TRACE_WATCH=unsavedprints only exits that left watched blocks without a save, skipped saves and unsaved stores. The full trace changes timing enough to hide a race.agc_driver_store_over_untracked_unimportedrunsagc_driver_store_over_untracked_testswithAPS5_NO_HOST_IMPORT=1. On Windows the surface's host import is compared and unwatched, soagc_driver_store_over_untrackedprints "not run, the surface is not write-watched" for every leave case, including fix(agc): run the unwatch hook before an import whose range leaves the write watch whatever the import does #7505's partly unwatched import. The new registration runs them.This branch is #7505's head merged with current
main, plus 2 commits. Until #7505 (and #6308) are merged, the diff also shows their commits.Tested
mainfrom before fix(agc): run the unwatch hook when a watched import leaves the write watch #6308/fix(agc): run the unwatch hook before an import whose range leaves the write watch whatever the import does #7505, aborted about 27 s in atTexture.cpp's "storing storage image results pending over memory outside the write watch without the guest bytes saved before them is not implemented".APS5_TRACE_WATCH=unsavednamed the cause:exclude 0x11f1258000+0x21da8000 by ImportStaysUncovered hook none new-blocks 8666 generation 4633unsaved image 0x1202ef3600+0x90000 … layerPendingGeneration 4497 … ImportStaysUncovered=10 … hooked none host-region 0ctest -R "^agc_": 712 of 713 pass, 3 skipped.agc_driver_bda_devicefails withvkWaitForFences graphics: Vulkan result -4on this machine, identically onmainwithout this PR.agc_driver_store_over_untracked_unimported: every leave case runs; only the two copy cases print "no host import".tools/check_conventions.py --base origin/mainandtools/check_test_registration.py buildpass.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)🤖 Generated with Claude Code
https://claude.ai/code/session_01HAtTu2pw6GTkMNwsjBDatU