Repository navigation
fix(agc): retain map import failures across unrelated mappings - #8802
Conversation
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed the failure-cache path and exact diff. Import failures are stored with the attempted run's mapped owner ranges, and importRunAt already retries when sameRun no longer matches; removing the blanket registry-generation erasure therefore preserves the unchanged owner's memo while still allowing a replacement owner to retry. The deterministic probe exercises an unrelated mapping during vkAllocateMemory, verifies only one attempt for the unchanged owner, then verifies a retry for the replacement. The reported negative control and RADV validation-layer run cover this behavior. Hosted eligibility, conventions, intake, labels, and report checks pass; platform build jobs are skipped, so I rely on the reported Linux test runs. The author has documented that #8622 may require adapting this change if it lands first. No blocking issue found.
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
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.
|
…time importMappedFailureGenerationTests (boykopovar#8802) maps a single 64 KiB range and expects ImportMappedRanges to try its import. Ranges of 64 KiB are now left to their first GPU use, so the probe saw no allocation. The fixture maps 128 KiB as one window, which is still imported at map time. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What
Keep a failed map-time host import cached when an unrelated allocation changes the guest registry during the Vulkan allocation call.
ImportMappedRangescurrently erases the failure for any generation change; the next map callback retries the same failed owner/run. Existing owner-aware failure matching already permits retries when the allocation is replaced.Remove the blanket erasure and add a deterministic allocation-probe regression that changes an unrelated mapping during
vkAllocateMemory, checks that the unchanged owner is not retried, then replaces the owner and checks that it is retried.This is independent of #8091 and makes no title-unblock or FPS claim.
Tested
Arch Linux, AMD Radeon RX 6650 XT (RADV NAVI23), Mesa 26.2.4-arch1.1. Matched current-main and candidate runs use required Vulkan, shader cache disabled, and the Khronos validation layer loaded explicitly from the local layer package. Full pins, loader diagnostics and logs are attached below.
ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1 \ VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation \ VK_ADD_LAYER_PATH=/mnt/sas-ssd/AnyPS5-work/workspace/doom-videoout/vulkan-layer/usr/share/vulkan/explicit_layer.d \ VK_LOADER_DEBUG=layer,warn,error \ VK_DRIVER_FILES=/usr/share/vulkan/icd.d/radeon_icd.json \ ctest --test-dir build-full -R '^(agc_driver_graphics|agc_driver_host_import_failure_handles)$' -VMain
3b5a24c0d611af1fc7ddd843dfa66fd6563a5dc5: 2/2 pass. Candidated2eeb4931c562094ae3d56999934acdf622cdebd: 2/2 pass. Loader diagnostics show Khronos instance/device layer insertion; both logs contain zeroVUID-errors and no missing-layer messages.Negative control: apply only the new test to the same main. The companion GPU test passes; graphics fails with
an unrelated mapping during a failed import discarded its owner failure memo. Restoring the production change makes both pass. The replacement-owner assertion also passes, preserving retries for a genuinely new owner.Full logs, source patch, environment, source pins and overlap report (SHA-256
5f49933c547561fbe08d292543bdb1337db50a2902166d4e1469e5d7f96be009). Conventions andgit diff --checkpass.The graphics harness reports heap-mirror subcases unavailable because its guest arena is not write-watched; those subcases are not claimed as covered. The new map-import regression executes in the mock allocation probe, and the companion host-import failure test runs on the GPU. No native title performance comparison was run.
Overlap: #8091 is our draft and shares the allocation-probe fixture; it remains draft and will be rebased if this lands first. #8622 changes adjacent map-import logic to defer repeated address imports until GPU use, whereas this fix retains owner-aware failure caching for an unchanged owner. There is no implementation dependency. If #8622 lands first, this patch and its replacement-owner assertion need adapting to that new map-time policy before merging. Other stale shared-file branches could not all be compared; the complete overlap report is attached.
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)