[dean0396]

What and why

mtl_pool_del (from 1d37720, #1392) gets a retired pool buffer's fence from a single expression:

uint64_t fence = g_fence + (atomic_load(&g_sub_on) ? 1 : 0);

C doesn't sequence the two operands of + against each other, so nothing guarantees the open flag is read before g_fence, and clang reads g_fence first: in main's dmhost (-O2) the two loads are an ldar of g_fence, then an ldar of g_sub_on. mtl_sub_commit clears g_sub_on only after its ++g_fence and its last encoding. Suppose a deleter reads g_fence before a commit's ++g_fence and then reads the flag after that commit clears it. It gets the fence before that submission's own fence, and that one may already have passed. defer_kick would then take the buffer out of the residency set and drop it right away. The submission looked the pool up before the generation bump, so it could still be queued or running on the GPU at that point.

The window is between those two loads, so the deleting thread has to be preempted there while a commit runs from its ++g_fence to its clear. I found this by reading the code. I haven't seen it happen in a game.

The fix reads the flag first, in its own statement, and reads g_fence after it. Both are seq_cst atomic loads, after the existing full fence:

• A 0 read written by a commit's clear means the g_fence read after it already counts that submission.
• A 1 read gives g_fence + 1, which is at most one fence later than needed.

A test hook sits between the two reads: g_pool_del_between, a static function pointer declared beside g_sub_on, which the host never sets. It lets t_pool_retire act out a commit landing there. The compiler drops the hook from dmhost: there is no symbol, and the two loads stay adjacent. The Fix and Check lines of the reference entry "Retired pool buffers pile up while a game submits nothing" now describe this.

Checked

• Tests (M3 Max, macOS 26.7.2, GAME=0400486024DEC000, on main 8fb5ffe):
  • make -C dm/gfx/tests check_t_pool_retire: the new case uses the hook to do a commit's ++g_fence and clear the open flag between the two reads. The buffer waits for at least that submission's fence, stays while the submission is open, and goes once it and one more submission complete (the 1 read gives a fence one later than needed). 18 checks, 0 failed, in 10 runs.
  • With the reads put back in the old order (g_fence, the hook, then the flag), the new case fails 2 checks in 5 of 5 runs: the fence is the one before the submission's, and the buffer is gone while the submission is still open.
  • The whole graphics test run (make -k -C dm/gfx/tests run) passes except for one t_feedback check, the same as on unmodified main. That includes t_pool_chunks, t_pool_notes, t_pool_standin, t_pool_seen_replace, t_defer_chain, t_gmem_words, t_readback, t_rt_recycle, t_alias_takeover, t_reorder_chain, t_direct_upload and t_blend_zero.
  • The t_feedback check (with nothing running: a commit) fails in some runs and passes in others, on unmodified main as on this branch, and it is always the same single check. This change doesn't touch the segment cut it checks.
  • The Mac host (make -C dm/host GAME=0400486024DEC000) builds with the same 58 warning lines as main. Its mtl_pool_del now loads g_sub_on, then g_fence (read from the disassembly; I didn't run the host).
  • dm/tools/tests/test_docs.py passes.
• Games run: none, and regress.py check wasn't run. The change only swaps the order of two loads and adds a hook the host never sets. Behaviour differs only when a commit lands between the two reads.
• Not run: lint_c.py (I don't have clang-tidy installed), iOS.
• Overlapping PRs: none of the open PRs changes this fence expression. git merge-tree gives every open PR that touches these three files the same conflicts with this branch as with main.
  • Ocarina of Time (Switch 2): GPU-written texture ranges fix the terrain holes; opt-in texture release saves ~2.3 GB; a page on playing it #1447 adds rel_pool_gone to the top of mtl_pool_del. It already conflicts with main, and this change adds no conflict to it.
  • Soft textures: a large block-compressed texture kept without its finest level, its level-0 pages back #1599 adds mtl_pool_wrapped right above mtl_pool_del. The hook is declared beside g_sub_on instead, so the two merge cleanly.
  • Retire obsolete NVN chunks after whole-pool mapping #1417 adds its own t_pool_retire.m, which already collides with main's test of the same name. This PR adds a case to main's file.

Docs

• The finding is in its docs/reference/memory.md entry (Fix and Check of "Retired pool buffers pile up while a game submits nothing"), and python3 dm/tools/docs_symptoms.py was run (no change to the index, since the Symptom line is unchanged)
• No game files, keys or data extracted from a game in the change or the attachments

[seedds]

Thank you, this is a real race, and your test made it easy to pin down. It landed on main as 259f4a5, with you as co-author, in a slightly different form:

• Since this PR, the expression moved into pool_gone_release, which mtl_pool_del and mtl_pool_replace (sparse pool runs) both call, so the fix is there.
• Reading a 0/1 flag first is correct but can still be one fence late: if the flag reads 1 and the commit lands before g_fence is read, the buffer waits for g_fence + 1 after that commit, which a game that stops submitting never reaches. So the flag (now g_sub_open) holds the open submission's own fence (g_fence + 1 at mtl_sub_begin, 0 after the commit's clear), and pool_gone_release uses it when nonzero and reads g_fence only on a 0. That fence is exact in every order.
• Your hook (renamed g_pool_gone_between) and test case are kept, with the check tightened to exactly the submission's fence and the buffer gone with no submission after it. With g_fence read first, the case fails 2 checks (freed while open); with the 0/1 flag read first, 2 checks (one fence late), in 3 of 3 runs each.

The reference entry in docs/reference/memory.md links this PR. Closing it, as the change is on main.