Skip to content

fix(libc): defer resident alias demotion during host writes - #6264

Merged
oneandonlydean merged 3 commits into
boykopovar:mainfrom
MarcZX10:codex/host-write-map-alias-6260
Oct 17, 2026
Merged

oneandonlydean merged 3 commits into
boykopovar:mainfrom
MarcZX10:codex/host-write-map-alias-6260

Conversation

@MarcZX10

@MarcZX10 MarcZX10 commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

What

Fixes #6260. MapAlias declines a resident direct-memory run when any page in that run has an open HostWrite. It returns immediately rather than waiting, since host writes such as socket receives can block. The guest-memory test checks that an alias is refused while the write is open, then succeeds after the write closes and preserves both pages' data.

The production guard and regression test retain Romans Pokrovskis's original authorship; the test also retains Pipodi's original regression credit. This change does not include #5851's resident alias expansion or automatic AGC alias routing.

Tested

  • Linux Release build of guest_memory_tests, libc.prx, and libkernel.prx completed.
  • python3 tools/check_conventions.py --base upstream/main passed.
  • git diff upstream/main...HEAD --check passed.
  • The guest_memory test cannot run on this Linux host: its CPU lacks LZCNT, BMI1/2, MOVBE, AVX2, and FMA. The issue records five failing runs against main and five passing runs with the guard on Windows 11; those remain the runtime evidence.

Checklist

  • Based on current main; one topic
  • No investigation notes or images added to the repository
  • Generic behavior, not specific to one title
  • Depends on: none
  • AI-assisted: yes

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Additional local validation: I cross-compiled core/libs/tests/GuestMemory.cpp with MinGW and _WIN32, so the Windows-only regression test body compiles. I also cross-compiled core/libs/prx/libc/src/GuestArena.cpp; this container's MinGW headers lack the placeholder-memory constants used by the project, so I supplied those flag definitions for the compile. This checks the production translation unit's C++ syntax, not Windows runtime behavior. The Windows execution evidence remains the five-failure/five-pass run recorded in #6260.

@github-actions github-actions Bot added the libc label Oct 15, 2026
@MarcZX10
MarcZX10 force-pushed the codex/host-write-map-alias-6260 branch 2 times, most recently from aa8708f to 312170c Compare October 15, 2026 10:14
@MarcZX10

Copy link
Copy Markdown
Contributor Author

After rebasing onto current main, I rebuilt guest_memory_tests successfully on Linux, including the new regression code. CTest could not execute the test here: the emulator rejects this host CPU (Intel Xeon E5-1620 v2) because it lacks LZCNT, BMI1/BMI2, AVX2 and FMA. The Windows runtime evidence already recorded in the PR remains the relevant execution result. Diff-check and repository conventions pass on the rebased head.

This was referenced Oct 15, 2026
@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

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 1191 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 1324 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 2434 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 4448 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 4449 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
core/libs/tests/GuestMemory.cpp
PR 5021 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
core/libs/tests/GuestMemory.cpp
PR 5372 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 6044 core/libs/tests/GuestMemory.cpp
PR 6146 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 6177 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 6197 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 6222 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 6292 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 6320 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 6447 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 6481 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 6548 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 6565 core/libs/tests/GuestMemory.cpp
PR 6615 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 6636 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 6947 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 7154 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 7201 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp

Retain Pipodi safety guard from closed PR5851 without its resident alias expansion or automatic AGC alias routing. Main still refuses aliases over partial resident runs. Five unmodified-main regression runs fail before this guard; positive validation pending. Fork proposal only; no duplicate upstream PR intended.

(cherry picked from commit 078a574)
Retain the regression originally supplied by Pipodi in closed PR5851 (2209141), adapted here through e1c6af6. Current-main-equivalent libc/kernel source returns an alias during the open HostWrite and fails this assertion five times. This test is a reproduction, not a new implementation or an upstream PR.

(cherry picked from commit 312170c)
@MarcZX10
MarcZX10 force-pushed the codex/host-write-map-alias-6260 branch from 312170c to 50748f7 Compare October 15, 2026 14:54
@MarcZX10

Copy link
Copy Markdown
Contributor Author

Rebased the two focused commits onto current main and removed the unrelated draw-pipelining history. The change remains the host-write guard plus its regression test, with original commit authorship preserved. Linux Release build and conventions pass; the guest_memory runtime test is blocked here by missing LZCNT, BMI1/2, MOVBE, AVX2, and FMA. I updated the PR body to make the host-only runtime evidence and limitation clear.

@oneandonlydean oneandonlydean self-assigned this Oct 17, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

The guard looks right for MapAlias itself, but its one production caller doesn't expect a null return: GuestBufferMemory.cpp around line 1151 only handles a thrown refusal, and with entry.alias == nullptr the import falls through to entry.base (line 334), the write-watched view the alias was meant to avoid. Could you either have the caller treat a null alias as a transient refusal (CPU copies for now, not recorded as a permanent import failure), or keep MapAlias throwing with a distinct reason the caller can handle? A Windows title run that exercises an alias import would also help, since our bench can only run the Linux side.

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Addressed in f7e613156: when MapAlias declines a run because a host write is open, importAllocation now returns nullptr immediately. That leaves the refusal transient and lets the existing caller use its copy path instead of proceeding with a host-pointer import from the write-watched base view. No permanent import-failure entry is recorded.

Validation: tools/check_conventions.py --base upstream/main and git diff --check pass. The changed production translation unit passes a MinGW syntax compile with _WIN32; the installed MinGW headers required a compile-only compatibility shim for their older PROCESSOR_RELATIONSHIP declaration. This is not Windows runtime validation. The existing guest_memory runtime coverage of the alias refusal remains unavailable on this host due the emulator CPU-feature guard; hosted CI has not reported checks yet.

@github-actions github-actions Bot added the agc libSceAgc* label Oct 17, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

Thanks, the early nullptr return closes the fall-through, and not recording it as a failure is right. One non-blocking follow-up to consider: the refusal happens after the overlapping imports are retired and the LRU eviction runs (around lines 1084-1090), so while a blocking recv keeps a host write open, every lookup of that range evicts and then gets refused again. Checking for the open host write (or trying the alias) before retiring would avoid that churn.

Before merging I'd like one Windows title run on f7e613156 that exercises an alias import, ideally a networked title where a socket receive holds a host write open, with the log and the commit you tested. Our bench can only run the Linux side, so this code isn't covered there. If anyone on Windows can run one, please post the result here.

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Good catch. MapAlias can reject while a host write remains open, but importAllocation currently retires overlaps and may evict LRU imports before making that call. Repeated lookups can therefore churn imports during the host-write window. I agree a preflight check before retirement is worth evaluating, with the existing MapAlias check retained to handle races. I do not have a Windows title run for head f7e613156; the available evidence is the focused Windows run already recorded in #6260 plus compile-only validation here. I will avoid claiming the requested title scenario is verified.

@github-actions github-actions Bot added the fix error Closes an issue (Fixes #N) label Oct 17, 2026
@screamm

screamm commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Windows run for @oneandonlydean's request, on Windows 11 Pro 26220, NVIDIA RTX 4070 (596.21), Ryzen 7 5700X, MinGW GCC 15.2. PR head f7e613156 merged on main 5f61879a9 (clean merge; also merges cleanly on the newer adb0e7a8c). Before-runs on main 3b10c7ef0.

  • Full build and libs: pass.
  • guest_memory, --repeat until-fail:3: 3 of 3 pass (repeated after a rebuild: 3 of 3). One earlier series had a failure at GuestMemory.cpp:1649 in CheckRemappedDirectMemoryIsChargedOnce, which measures the system-wide commit total and was disturbed by other builds on the host; this PR does not touch it.
  • Negative control: with both production hunks reverted to main and the test kept, guest_memory fails at GuestMemory.cpp:1281, the MapAlias(...) == nullptr check while the HostWrite is open. With the PR restored it passes again.
  • Non-GPU ctest: 852 of 876 pass, 2 skipped, 22 timeouts under load. Run serially, 16 pass. The six conventions_function_bodies* shards hit their fixed 30 s limit again, but pass when run directly (4-11 s each).
  • tools/check_conventions.py and git diff --check: pass.

Title runs, 120 s each, main vs this PR:

  • ProsperoRadio PPSA99001 (HTTP streaming): I pressed Cross at 45 s to start the first station. In 3 runs on main and 2 with this PR, RTL plays live ("NOW PLAYING · LIVE", about 1:07 on air at 115 s). No stop line, no [gpu] errors. Each run was killed at the limit. Frames: 6537 on main on an idle host, 6224 and 6105 with the PR. Two main runs that overlapped a build gave 3131 and 3599.
  • Stremio Plus PPSA74126 (libcurl over HTTPS, no input): main 2 of 2 and PR 2 of 2 get to "Link your account" with a fresh code. No stop line, no [gpu] errors. Frames: PR 5142 and 6352; main 2392 and 2776 (both main runs overlapped a build).

Whether the alias path was reached: it was not. I added temporary counters in a local build only; they are not part of this PR. They count GuestArenaMapAlias_nid_postfix calls, entries into importRunAt's alias branch, and host writes by caller module. With main's code and with this PR, both titles made 0 MapAlias calls, 0 alias-branch entries, and had 0 declined and 0 granted aliases. Every host import they made was of writable private memory.

  • Both titles receive through libkernel's socket recv, not libSceNet.
  • ProsperoRadio's receive buffers are never in resident direct memory.
  • Stremio's heap is in direct memory. About 540 of its 4100 host writes landed on resident runs, but none stayed open longer than 1 ms, because libcurl's sockets are non-blocking.

So these runs show no regression in two networked titles, but they do not exercise the case you asked about (an alias import refused while a blocking receive holds a host write open). I don't have a local title that reaches it. The code matches your churn note: the overlap retirement and LRU eviction run before the alias attempt. I could not measure that churn here.

Not checked: a title that hits the alias path, Linux, the agc_driver_* GPU tests.

Written with Claude Code assistance (validation by a subagent, reviewed in this session).

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Thanks for running both networked titles and instrumenting whether the alias path was reached. The 0-call / 0-decline result makes the limit clear: these runs are useful non-regression checks, but they do not exercise the open-HostWrite refusal or measure the repeated retirement/eviction churn. The requested alias-path title validation remains outstanding; I will not describe these runs as covering it.

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Thanks for the follow-up. I checked the current path against the existing regression and the Windows title report. The MapAlias guard runs before demotion, and the existing GuestMemory test verifies it refuses without creating an alias while HostWrite is open. I cannot reproduce the repeated-import eviction/churn from that test: it exercises MapAlias directly, and the two reported title runs made zero alias-import calls. I am leaving the implementation unchanged until there is an import-level reproduction that demonstrates the churn and can validate a fix.

@oneandonlydean
oneandonlydean merged commit e5961b2 into boykopovar:main Oct 17, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* fix error Closes an issue (Fixes #N) libc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows: MapAlias succeeds during an open HostWrite on a resident run

4 participants