Skip to content

feat(agc): keep the last submissions per queue and dump them on a stall or crash - #4855

Merged
adriadam10 merged 2 commits into
boykopovar:mainfrom
fdfawef324:feat/agc-submission-ring
Oct 16, 2026
Merged

adriadam10 merged 2 commits into
boykopovar:mainfrom
fdfawef324:feat/agc-submission-ring

Conversation

@fdfawef324

@fdfawef324 fdfawef324 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

What

Each GPU queue keeps a ring of its last submissions, and the rings are printed when the Windows crash report runs or when a waiter stalls. Before this, a GPU hang or a crash left no record of what the queues were last given: APS5_TRACE_GPU prints every submission (too much to leave on), and the WAIT_REG_MEM timeout message shows only the packet before the wait on the waiting queue.

  • APS5_PM4_RING=<n> (default 16, 0 to 4096, 0 disables): per queue, the TraceMs time in tenths of a millisecond, serial, size, guest address and the first 512 dwords after indirect buffers are inlined, REWIND tails included. Recorded under the driver lock next to the serial. The ring and the failure report registration are set up at the top of Submit, so recording does not allocate. Disabled, Submit only tests a static flag. Enabled, it costs one bounded copy per submission.
  • The dump decodes packets with Pm4::PacketWords and the new Pm4::KnownName. KnownName returns the decoder's names as views of static strings, or nothing for an unknown opcode, which is printed as UNKNOWN_0x%02x. Pm4::Name now builds its std::string from it. Each line is formatted with integer and string conversions only, into a 512-byte stack buffer, and written with one WriteFile / write(2) on the stderr handle. So the dump does not allocate from the C runtime heap, it does not take the CRT stream lock (fix(libc): write each log line in one call #3871), and lines from different threads do not interleave.
  • libc gets a registry of failure reports (LibcRegisterFailureReport_nid_no_patch, Unregister, Run). Run runs the registered reports the first time it is called and returns at once after that. The Windows crash report calls it in three places: after the thread dump of an unhandled exception, after SIGABRT (this covers the APS5_HANG_DUMP_SECS timed dump, which aborts) and after an invalid CRT parameter.
  • APS5_STALL_WATCHDOG_SECS=<n> (0 to 86400, default 0) makes a waiter that has not progressed for n seconds dump the rings once per wait. The waiters are a WAIT_REG_MEM, a REWIND waiting for the title, and a compute submission held behind queue 0. The last one prints under the driver lock, so submissions and the other queues wait for it; USAGE.md says so. An invalid value of either variable throws at the first submission. The order hold behind queue 0 is released on scope exit.
  • Technical debt (docs/dev/debt/libSceAgcDriver.md) says what the crash-path dump can and cannot do:
    • It does not allocate.
    • It waits about 100 yields for each ring lock. If the lock is still held, it prints that queue as busy, so a crash while recording a submission loses that queue's ring.
    • It reads the rings without checking them. A crash that corrupted them can fault inside the dump. If that happens during the report of an unhandled exception, the process stops in that report's Sleep(INFINITE) and hangs instead of ending.
    • The reports run at most once per process.
    • Linux has no crash report hook. There the rings are printed only by the watchdog.

No packet semantics change. Packet sizes and names are those of the existing decoder (PM4 type-3 header layout as in Mesa src/amd/common/sid.h). This PR makes no speed claim.

Tested

Head 871fe7627, rebased onto main 8856f180c. The CMake conflict resolution keeps main's 30-second timeout on agc_driver_pm4_failure and all ten ring/watchdog test registrations. The ring and stall-watch hooks sit next to main's submission-capture hooks, both records under the driver lock. The original author, author date and commit message are kept. Conventions and diff checks pass. The head also merges with main 34454bd83 without conflicts; that merge was not built.

Every result below is from head 871fe7627: Windows Server 2025, RTX 3060 12 GB (driver 591.86), GCC 15.2 (winlibs) Release, build-dev built with ninja all and the three replay tools.

  • New ctests (agc_driver_pm4_tests modes): agc_driver_pm4_ring, _ring_empty, _ring_disabled, _ring_invalid, _ring_report, _ring_report_heap, _stall_watchdog, _stall_watchdog_invalid, _stall_watchdog_rewind, _stall_watchdog_order. Also windows_backtrace_failure_report and windows_backtrace_failure_report_once. The PM4 catalog test (agc_driver_pm4) checks KnownName against every opcode and custom name. All 18 ^agc_driver_pm4|^windows_backtrace tests pass. The ring tests take 0.02 to 0.5 s and the stall tests about 2.6 to 2.8 s.

  • agc_driver_pm4_ring_report_heap (Windows) swaps the HeapAlloc / HeapReAlloc imports of ucrtbase.dll for counters, so it counts every C runtime allocation made on the reporting thread. It submits a buffer holding INDEX_BUFFER_SIZE (17 characters, longer than the std::string inline buffer) and runs the failure reports. The report must name the packet and make no allocation. As a check on the trap itself, Pm4::Name of the same header must be seen allocating. On Linux the mode returns 77 (skipped).

  • Red with the fixes reverted, green with them:

    • SubmissionRing.cpp decoding names through Pm4::Name and the time through %.1f: agc_driver_pm4_ring_report_heap fails with "the failure report allocated from the C runtime heap".
    • FailureReport.cpp resetting its flag after a run: windows_backtrace_failure_report_once fails, because the FAIL regex finds the report printed twice.
  • Behavioural red: the new files were kept and only the hook sites (QueueWorker.cpp, Submission.cpp, WaitMemory.cpp, CrashReport.cpp) were reverted to the base 8856f180c. These failed:

    • agc_driver_pm4_ring: "the ring dump header is wrong"
    • _ring_report: "the failure report does not dump the rings"
    • _ring_report_heap: "the failure report does not name INDEX_BUFFER_SIZE"
    • _ring_invalid and _stall_watchdog_invalid: "expected PM4 rejection"
    • _stall_watchdog, _rewind and _order: "... did not dump the rings" (about 15.7 s each)
    • windows_backtrace_failure_report: required regex not found

    _ring_empty, _ring_disabled and windows_backtrace_failure_report_once pass without the hooks, as expected.

  • Suite: ctest -R "^agc_|libc|^windows_backtrace" -j 4: 597/600 passed, 3 skipped, 0 failed. The skips are agc_host_import_sparse and agc_driver_sparse_dma_buf_import (udmabuf imports are Linux only) and agc_driver_stencil_ref_export (the device lacks VK_EXT_shader_stencil_export). The 9 failures reported on the earlier base 1c5e628 (agc_driver_flat_loads, agc_driver_global_loads, agc_driver_flat_apertures_2, _5, agc_driver_pixel_interlock and their variants) pass here.

  • python -I tools/check_test_registration.py build-dev: 879 test executables, none built and not run, none new.

  • Not run: the Linux build and tests (the shared code keeps Windows calls behind #ifdef _WIN32, and the heap test is skipped there); no title or homebrew; no test outside the agc, libc and backtrace sets.

Checklist

  • Based on 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 under Silent stubs in their library's technical debt file
  • 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
  • Unblocks: none
  • GPUs tested: NVIDIA GeForce RTX 3060 12 GB, driver 591.86 (32.0.15.9186), Vulkan 1.4.325, Windows Server 2025 10.0.26100
  • AI-assisted: yes

🤖 Generated with Claude Code

Co-Authored-By: Codex noreply@openai.com

@adriadam10

Copy link
Copy Markdown
Collaborator

Rebased onto current main (one include conflict in CrashReport.cpp, kept both includes). Checked: scoped build and ctest for agc driver, libc and libkernel pass, including the ten new agc_driver_pm4_* ring/stall tests. The only failure, agc_driver_tessellation, is a segfault inside libvulkan_radeon and also happens with APS5_PM4_RING=0. Read the dump path (no allocation, bounded line buffer, queue and capacity limits, invalid env throws). The Windows-only parts are covered by the Windows build run before merging.

@adriadam10
adriadam10 force-pushed the feat/agc-submission-ring branch from 3d4473d to 4048e02 Compare October 13, 2026 15:49
@fdfawef324
fdfawef324 force-pushed the feat/agc-submission-ring branch from 4048e02 to f506621 Compare October 13, 2026 17:53
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 13, 2026
@fdfawef324
fdfawef324 force-pushed the feat/agc-submission-ring branch 5 times, most recently from 85fa678 to 33db811 Compare October 14, 2026 02:49
@adriadam10 adriadam10 self-assigned this Oct 14, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 14, 2026
@adriadam10

Copy link
Copy Markdown
Collaborator

Rebased onto current main (the only conflict was two adjacent new sections in docs/user/USAGE.md, both kept). Checked: the scoped build and tests for the AGC driver and libc, including the new agc_driver_pm4_ring* / _stall_watchdog* tests (pass on Linux; the heap-trap test is Windows-only and skips here), the order-hold loop rewrite (poll returns the predicate, same exit condition as before), invalid env values throwing at the first submission, and the technical debt and USAGE entries. Windows behaviour is covered by the full Build before merging; merging once it is green.

@adriadam10
adriadam10 force-pushed the feat/agc-submission-ring branch from 33db811 to 42ece98 Compare October 14, 2026 11:08
@github-actions github-actions Bot added needs rebase Conflicts with main, rebase needed and removed needs rebase Conflicts with main, rebase needed labels Oct 14, 2026
@fdfawef324
fdfawef324 force-pushed the feat/agc-submission-ring branch from 42ece98 to 82de94c Compare October 14, 2026 12:27
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 14, 2026
This was referenced Oct 15, 2026
fdfawef324 and others added 2 commits October 16, 2026 11:18
…ll or crash

A GPU hang or a crash in a title leaves no record of what the queues
were last given: APS5_TRACE_GPU prints every submission (too much to
leave on), and the WAIT_REG_MEM timeout message shows only the packet
before the wait on the waiting queue.

Each queue now keeps a ring of its last 16 submissions
(APS5_PM4_RING=<n>, 0 to 4096, 0 disables): the TraceMs time in tenths
of a millisecond, serial, size, guest address and a copy of the first
512 dwords after indirect buffers are inlined, recorded under the
driver lock next to the serial, REWIND tails included. The ring of a
queue and the failure report registration are set up at the top of
Submit, before any output is reserved, so recording under the lock
does not allocate. Disabled, Submit only tests a static flag; enabled,
it costs one bounded copy per submission.

DumpSubmissionRings decodes the kept dwords with Pm4::PacketWords and
the new Pm4::KnownName, which returns the decoder's names as views of
static strings (empty for an unknown opcode, printed as UNKNOWN_0x%02x);
Pm4::Name now builds its std::string from it. Every line is formatted
with integer and string conversions only into a 512-byte stack buffer
and written with one WriteFile / write(2) on the stderr handle, so the
dump does not allocate from the C runtime heap, lines from other
threads do not interleave and the CRT stream lock is not taken (the
rule of boykopovar#3871). The header gives the number of queues and the ring
size, and each queue line the submissions recorded and kept.

libc gets a small registry of failure reports
(LibcRegisterFailureReport_nid_no_patch, Unregister, Run); Run runs
the registered reports the first time it is called and returns at once
after that. The Windows crash report runs it after the thread dump of
an unhandled exception, after SIGABRT (which also covers the
APS5_HANG_DUMP_SECS timed dump, since it aborts) and after an invalid
CRT parameter, so an abort after a reported exception does not print
the rings twice. The driver registers its dump at the first submission
and unregisters at static destruction; the dump there waits a bounded
time for each ring lock. What the crash-path dump cannot do (a ring
held by the crashed thread, rings corrupted by the crash, Linux) is
recorded in technical debt.

APS5_STALL_WATCHDOG_SECS=<n> (0 to 86400, 0 disables) makes a waiter
that has not progressed for n seconds dump the rings once per wait: a
WAIT_REG_MEM (progress as for APS5_GPU_WAIT_TIMEOUT_MS: another packet
runs or completes), a REWIND waiting for the title, and a compute
submission held behind queue 0. The last one prints under the driver
lock, so submissions and the other queues wait for it (USAGE.md says
so). Both variables are read at the top of Submit, so an invalid value
of either throws at the first submission and never later inside a GPU
worker. The order wait behind queue 0 releases its order hold on scope
exit, so a throwing dump cannot leave it counted.

No packet semantics change; packet sizes and names are those of the
existing decoder (PM4 type-3 header layout as in Mesa
src/amd/common/sid.h). The queue bound of Submit and of the rings is
one constant, SubmissionQueueEnd.

Tests: agc_driver_pm4_tests gets ten modes, each its own ctest:
agc_driver_pm4_ring (APS5_PM4_RING=4, six submissions on queue 0 and
two on 0x20: the dump holds boykopovar#3..boykopovar#6 and boykopovar#1..boykopovar#2 in order, nothing
older), agc_driver_pm4_ring_empty (one line), _ring_disabled (one
line), _ring_invalid and agc_driver_pm4_stall_watchdog_invalid
(APS5_STALL_WATCHDOG_SECS=1x: the submission throws and is not
executed), _ring_report (LibcRunFailureReports_nid_no_patch dumps the
rings), _ring_report_heap (Windows: with ucrtbase.dll's HeapAlloc and
HeapReAlloc imports counting calls from the reporting thread, the
failure report of a submission holding INDEX_BUFFER_SIZE names it and
allocates nothing, while Pm4::Name of the same header is seen
allocating), and with APS5_STALL_WATCHDOG_SECS=1 a WAIT_REG_MEM
(agc_driver_pm4_stall_watchdog), a REWIND (_stall_watchdog_rewind) and
a compute submission held behind a queue 0 rendering wait
(_stall_watchdog_order), each held for about 2.5 s, dump once and
complete when released. The PM4 catalog test checks KnownName against
every opcode and custom name. windows_backtrace_failure_report checks
that abort() runs the registered reports once and not an unregistered
one; windows_backtrace_failure_report_once that abort() does not run
them again after an earlier run. With the dump decoding through
Pm4::Name, agc_driver_pm4_ring_report_heap fails with "the failure
report allocated from the C runtime heap"; with Run resetting its
flag, windows_backtrace_failure_report_once prints the report twice.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The mode returns 77 on Linux with a message that tools/check_skips.py does not accept as a device limitation, so the full Build failed the skip check.
@adriadam10

Copy link
Copy Markdown
Collaborator

The full Build failed check_skips on linux: agc_driver_pm4_ring_report_heap exits 77 on Linux with "the C runtime heap trap needs ucrtbase.dll", which is not one of the accepted device limitations, so the job would lose that coverage silently. I changed the CMake registration so ring-report-heap is only added on Windows (own commit on your branch, rebased onto current main). Scoped build and ctest otherwise pass; agc_driver_tessellation segfaults in radeon and also fails on main. Merging once the full Build is green.

@adriadam10
adriadam10 force-pushed the feat/agc-submission-ring branch from 871fe76 to 4859af0 Compare October 16, 2026 09:26
@adriadam10
adriadam10 merged commit a4782fe into boykopovar:main Oct 16, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants