Skip to content

fix(agc): release retired imports while waiting for pins - #2864

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
MarcZX10:fix/agc-release-deferred-imports
Oct 10, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
MarcZX10:fix/agc-release-deferred-imports

Conversation

@MarcZX10

@MarcZX10 MarcZX10 commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

What

Break the guest allocation pin wait cycle when import retirement runs under GpuMutex. After synchronizing, hand off deferred recorder keeps so the retired import can release its allocation range before the waiter returns. Add a focused unit shadow CTest and correct its expectations for bytes published while the old full registration is still visible.

Fixes #2833.

Tested

Linux x86_64, GCC 12.2, Release: built agc_driver_recorder_tests. agc_unit_shadow was registered and correctly skipped because this host does not support the required asynchronous userfaultfd write watch. The test uses a 3 second pin wait and a 30 second CTest timeout. git diff --check and python3 tools/check_conventions.py --base upstream/main passed.

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 in TechnicalDebt
  • 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
  • AI-assisted: yes

Additional hardware validation (2026-10-10)

On Arch Linux with GCC 16.2.1, Release, and ANYPS5_REQUIRE_VULKAN=1, fma-rc ran the focused agc_unit_shadow regression on an NVIDIA RTX 5080 and AMD Radeon 610M (RADV): with this fix it passed 3/3 on each GPU; with the waiter hand-off removed it failed 3/3 after the two 3-second pin waits. The focused host-import and recorder tests also passed on both GPUs.

The full agc_driver_recorder_tests executable passes the repaired unit-shadow step but still stops at the unrelated (s) results pending when their memory left the watch did not reach it assertion. That assertion also fails on the parent before the pin-wait change. The full executable is not green yet. Full setup and results.

@fma-rc

fma-rc commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

The run you asked for, on a Linux host where the write-watch path is active (the test runs, it is not skipped): the fix removes the pin wait on an NVIDIA and on an AMD device.

Setup: PR head b2a8590 merged onto main 57f96fd. Arch Linux 7.2.9, GCC 16.2.1, Release, ANYPS5_REQUIRE_VULKAN=1. NVIDIA RTX 5080 (615.78.08); AMD Radeon 610M (RADV, Mesa 26.1.4).

"Without the fix" is the same merge with Graphics/src/GuestBufferMemory.cpp checked out from main, so the new test and its expectations stay and only the hand-off in the waiter is gone.

ctest -R '^agc_unit_shadow$', 3 runs each RTX 5080 RADV, Radeon 610M
With the fix passes 3 of 3, 0.1 to 0.2 s passes 3 of 3, 0.1 s
Without the fix fails 3 of 3 after 6.2 s fails 3 of 3 after 6.2 s

The failures are the reported stop, with the test's 3 s wait taken twice:

guest allocation 0x7f475d3f0000+0x1100000 is still pinned by GPU work or an import (waited 3000 ms for the lease (APS5_PIN_WAIT_MS))

ctest -R '^agc_unit_shadow$|agc_host_import|recorder' (3 tests) also passes on both devices with the fix.

Full run (agc_driver_recorder_tests with no arguments, APS5_PIN_WAIT_MS=3000), one run per cell:

RTX 5080 RADV, Radeon 610M
With the fix exit 1 after 1 s: (s) results pending when their memory left the watch did not reach it same
Without the fix exit 1 after 6 s: still pinned … waited 3000 ms same

So with the fix the full run gets past the unit shadow step, corrected expectations included, and stops at the (s) assertion. That is the stop the full run already had on 2f655b1, the parent of the commit that introduced the pin wait, so it is not caused by this PR; the full run is still not green.

Not run: the rest of the suite with this PR, and any title.

AI-assisted: yes

@eliyarson

Copy link
Copy Markdown
Contributor

Third device data point for the new regression test, since the description says it skipped on the author's host.

Setup: PR head b2a8590 as submitted (not merged onto current main), CachyOS (Arch), Linux 7.2.2, GCC, Release, -DBUILD_TESTING=ON. NVIDIA RTX 3080 Ti, driver 610.57.04. The startup probe prints host imports resolve write protection: no (0 of 4 … 4 after the import); imported ranges stay watched.

Run Result
ctest -R agc_unit_shadow, 5 runs ran (not skipped), passed 5 of 5
Full ANYPS5_REQUIRE_VULKAN=1 agc_driver_recorder_tests, 3 runs no pin wait; stops at once on (s) results pending when their memory left the watch did not reach it, as in #2833
Full ctest -j12, 699 tests 2 failures, both pre-existing and unrelated: agc_driver_dcc_keys_first_write (#2767) and guest_sce_net (#2820). No regressions

Not checked: that agc_unit_shadow fails without the fix; the test does not exist on main.

AI-assisted: yes. Runs and this report were produced with AI assistance; I take responsibility for the content.

@adriadam10

Copy link
Copy Markdown
Collaborator

Rebased onto current main (adjacency conflict in Recorder.hpp with KeepBytes/OpenKeptBytes; both kept). Libraries build and the AgcDriver/Agc ctest scope are green.

Checked FlushDeferredReleases() after both Sync() calls in WaitForLeasesAndImports, the new agc_unit_shadow registration (skip 77, 30 s timeout) and the corrected unit-shadow expectations. The test cannot run on my host (host imports are compared, not watched), so it relies on the 3/3 pass with / 3/3 fail without the fix reported by @fma-rc (RTX 5080, RADV) and the 5/5 pass by @eliyarson (RTX 3080 Ti). Merging once the full Build is green.

@adriadam10
adriadam10 force-pushed the fix/agc-release-deferred-imports branch from b2a8590 to f5618d9 Compare October 10, 2026 23:13
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 10, 2026
@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

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.

Pull request Conflicting files (lines on main) Same exports Same new files
#637 core/libs/prx/libSceAgcDriver/tests/Recorder.cpp L4754

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* build

Projects

None yet

Development

Successfully merging this pull request may close these issues.

agc_driver_recorder_tests (full run): allocation stays pinned for the whole lease wait since cf5c3ebe, on NVIDIA and RADV

4 participants