fix(hrr): snapshot the pinned host memory kernels read, and restore it at replay by andraghetti · Pull Request #13187 · ROCm/rocm-systems

andraghetti · 2026-10-09T13:41:58Z

Motivation

A kernel can read pinned host memory directly, through hipHostMalloc or hipHostRegister. Capture did not record what that memory held, so replay ran those kernels on whatever its own fresh buffer held. The ROCM-31827 vLLM archive has 147 such launches.

This change records, before each launch, the pinned host memory the kernel can read, and replay puts those bytes back before the kernel runs.

Technical Details

Capture tracks the ranges of hipHostMalloc, hipMallocHost and hipHostRegister, and drops them on free and unregister.
Before each launch, it looks for kernel argument words that point into a tracked range, including pointers inside structs passed by value. It copies each allocation it finds whole, up to the size cap, under the tracking lock, then hashes and writes the copy as a blob, and adds a record to the launch event. A record is written only when the content or the launch stream changed since the allocation's last record. The bytes are those at enqueue time; capture does not wait for the stream, so queued device writes into the allocation that have not finished are restored as they were before them (DESIGN.md lists this).
The tracking lock is taken in a fork handler, before the writer's locks, so a child never inherits it held.
HRR_VERSION goes to 10 (develop took 9 for hipKernelSetAttributeForDevice). Older archives are refused, as with the previous bump.
Replay checks each record before it restores it. The record must name a pinned allocation replay made, lie inside it, and match its blob's length. Anything else is refused and counted. Replay then queues a host function on the launch stream that copies the bytes in ahead of the kernel, without waiting for the stream; hipHostFree and hipHostUnregister wait for every stream, so the allocation outlives the copy. A blob refused by the host memory cap, or a failed enqueue, ends the launch before the kernel runs. Restores are skipped for a launch into a capturing stream, and capture checks the same launch stream. The summary prints Pinned host snapshots: N restored, M refused, K skipped (graph capture).
HIP_HRR_HOST_SNAPSHOTS=0 turns snapshots off. HIP_HRR_HOST_SNAPSHOT_MAX_MB (default 64) caps the size of one allocation. Both only tune capture, so hip_capture.cpp reads them itself, with the same parsing as the CLR flags. The change stays inside the HRR directories.
DESIGN.md describes the records, their limits and a short threat model for replaying untrusted archives (for AIRUNTIME-2853).

Issue Tracking

JIRA ID: AIRUNTIME-2894

Defect: ROCM-31827

Test Plan

Unit_HRR_PinnedHostRoundtrip: a kernel reads a pinned buffer the host changes between launches, through hipHostMalloc, hipMallocHost, hipHostRegister, a struct passed by value and a hipModuleLaunchKernel with extra[]. One launch goes into a graph capture and one onto a second stream while the capture is open. Replay must match the recording byte for byte. With snapshots off, it must not.
Unit_HRR_PinnedHostSnapshotCap: a 2 MiB allocation with the cap at 2 MiB replays correctly. With the cap at 1 MiB it is listed under unreplayable_apis, nothing is restored and replay does not match.
Unit_HRR_PinnedHostStreamsRoundtrip: the pinned-input kernel on one stream queues behind a kernel waiting for a flag that a later kernel on a second stream sets, after a launch there reads the same input. Replay must not hang and must match byte for byte.
Unit_HRR_PinnedHostBlobRefused: a snapshot blob over --max-file-bytes ends the replay before its kernel launches.
Unit_HRR_PinnedHostFork: forks while another thread snapshots a 32 MiB allocation; every child must take the tracking lock and exit.
Unit_HRR_PinnedHostFreedRoundtrip: after hipHostFree, hipFree, hipFreeHost or hipHostUnregister, capture no longer snapshots the address.
Unit_HRR_Format_HostSnapshotRecord: the record round trip, and records refused for bad bounds and short payloads.
Unit_HRR_DivergenceAbortRoundtrip and Unit_HRR_NullOptionalPtrRoundtrip now capture with snapshots off, since their divergence comes from a pinned flag.
Built amdhip64 with -DHIP_HRR_CAPTURE_ENABLED, hrr-playback and the tests for gfx950, and ran every Catch2 case, hidden ones included, on an MI350X.
Negative controls: the same build without snapshots and record checks, single-fix builds (struct pointers, untrack on free, hipMallocHost tracking with untrack on the other frees and the extra[] launch, payload bounds check, fork handler, launch-stream capture state, the blocking restore, per-content-only records), and a build whose cap ignores HIP_HRR_HOST_SNAPSHOT_MAX_MB.
Merged into Daniel's HRR CI branch and ran its matrix, with the negative control alongside.

Test Result

Check	With this change	Without it
Unit cases, MI350X	75 of 75 pass	`Format_HostSnapshotRecord` fails
Integration cases, hidden ones included, MI350X	Every pinned host case passes. Every other case gives the same result as on `develop` at 362ea65125. The ones that fail there fail here too: `Unit_HRR_FirstMallocRoundtrip`, and the `DiskSpace`, `DispatchSwap`, fork and crash `_Direct` workloads run without their visible test	`PinnedHostRoundtrip` fails; the cap control fails only `PinnedHostSnapshotCap`; each single-fix control fails the test written for it
HRR CI matrix (earlier head)	all 15 jobs pass: gfx90a, gfx94x, gfx950, gfx103x, gfx110x, gfx120x, gfx1150, gfx1151, gfx1153 and both unit jobs	`PinnedHostRoundtrip` fails on every family, and `Format_HostSnapshotRecord` on both unit jobs

Submission Checklist

Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.

…more tests

therock-pr-bot · 2026-10-09T13:42:55Z

✅ All Policy Checks Passed

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	✅ Pass	—
🚫 Draft PR	🔜 To Be Enabled	—
🚩 Feature Flag	🔜 To Be Enabled	—
📊 Code Coverage	🔜 To Be Enabled	—

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

Copilot

🟡 Changes recommended

Snapshot tracking has unresolved fork deadlocks, failed-free state corruption, and a pinned-pointer replay edge case.

3 open findings

What changed in this PR

Adds capture and replay support for pinned host-memory snapshots used by kernels.

Changes:

Tracks pinned allocations and snapshots changed contents before launches.
Validates and restores snapshots during replay.
Adds format v8 documentation, configuration, and tests.

File	Description
`projects/hrr/tools/gen_hrr_api_args.py`	Bumps generated format to v8.
`projects/hrr/include/hrr/hrr_api_args.h`	Updates generated format definition.
`projects/clr/rocclr/utils/flags.hpp`	Adds snapshot configuration flags.
`projects/clr/hipamd/src/hrr/hip_capture.cpp`	Implements allocation tracking and snapshot capture.
`projects/hrr/playback/hip_playback.h`	Adds snapshot validation and counters.
`projects/hrr/playback/hip_playback.cpp`	Restores snapshots before launches.
`projects/hrr/playback/hrr_reader.h`	Documents snapshot records.
`projects/hrr/playback/hrr_reader.cpp`	Parses snapshot records.
`projects/hrr/playback/hrr_playback.cpp`	Reports restoration statistics.
`projects/hrr/tests/unit/hrr_format_test.cc`	Tests record parsing and validation.
`projects/hrr/tests/integration/hrr_pinned_host_test.cc`	Adds pinned-memory round-trip tests.
`projects/hrr/tests/integration/hrr_roundtrip_test.cc`	Disables snapshots for divergence-specific tests.
`projects/hrr/tests/integration/CMakeLists.txt`	Registers the new integration tests.
`projects/hrr/tests/hrr_test_common.hh`	Supports capture environment overrides.
`projects/hrr/README.md`	Documents user-facing settings.
`projects/hrr/DESIGN.md`	Documents v8 design, limits, and threat model.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…e low pinned addresses The pinned tracking mutex joins the writer's fork protocol, taken first in prepare. A release call keeps the allocation tracked unless it succeeds, with snapshots held off for its duration. Replay translates a struct word capture flagged inside a pinned host allocation even when its low 32 bits fail the device-address shape check.

mangupta

🤖 AI-assisted pre-review (for @mangupta)

…k and failed-free handling Replay restores no pinned snapshot while a graph capture is open, but capture still made such a record the allocation's last, so the next direct launch on the same bytes got no record and replay ran it on stale memory. A record written while any stream in the process captures no longer becomes the last. The pinned workload now launches into a discarded graph capture before its third launch, and the roundtrip checks that launch is still restored. The pinned mutex no longer joins the fork handlers, and a free or unregister stops tracking the allocation before the call again, whatever it returns. DESIGN.md lists a fork during a snapshot as a known limit again.

mangupta · 2026-10-14T11:02:23Z

🤖 AI-assisted pre-review (for @mangupta)

Re-reviewed the updated head (f43cf807d0fc) — no new findings at or above the posting threshold.

mangupta · 2026-10-14T15:15:45Z

🤖 AI-assisted pre-review (for @mangupta)

Re-reviewed the updated head (aa91d3ffedb8) — no new findings at or above the posting threshold.

danielbolsson

The design is sound, and the replay-side record validation is solid. Two things to address before merge, plus a few minor ones inline.

Stale thread replies. The replies on r4246061970 (fork) and r4246062066 (failed free) still say "Fixed in b3bd5da", but fa1ffc5 reverted both fixes. Please update them so nobody resolves the threads on that basis. Inline at g_pinned_mu: why was the fork handler dropped?
PR description. It says HRR_VERSION goes to 8; the code bumps it to 9. Were the test results re-run after fa1ffc5 and the develop merge?

…ross fork A pinned snapshot hashed the application's memory and then read it again to write the blob, so a host thread or queued GPU work could change the bytes in between. The blob was then filed under a hash that did not name its contents, and the blob store's dedup served those bytes for any later blob with that hash. The snapshot now copies each allocation into a buffer under the tracking lock, releases the lock, and hashes and writes the copy. The buffer is allocated per snapshot and freed once written, before the launch; a failed allocation is listed under unreplayable_apis like an oversized one. The tracking lock is still held while a launch writes its event, so a fork() from another thread can still leave a child with it locked. Its fork handler is back: fa1ffc5 dropped it only to keep this change small, not over lock order. It is registered after the writer's handlers, so fork takes the pinned lock first, then the writer's, the order a launch takes them in. Unit_HRR_PinnedHostFork forks while another thread snapshots a 32 MiB allocation; each child takes the lock through a failing hipFree, and one that does not exit within 10 s fails the case.

Replay skipped every pinned restore while any graph capture was open, and capture kept every record made meanwhile from becoming the allocation's last. A direct launch on a stream that was not capturing lost its restore. Both sides now look at the launch stream only: replay asks hipStreamIsCapturing, capture reads the stream's capture status. The pinned workload launches on a second stream during its graph capture; replay must restore that launch, and the next launch then has nothing new to record. Unit_HRR_PinnedHostStructLowBits_Direct maps its fixed address with MAP_FIXED_NOREPLACE and skips when the range is taken, or when an older kernel treats the address as a hint and maps elsewhere. It runs last in Unit_HRR_PinnedHostRoundtrip, since a skip ends that case.

…raghetti/hrr-pinned-host-min

Replay synchronizes the launch stream before it writes a pinned snapshot back. When another stream was capturing in global mode, that sync invalidated the capture, and its hipStreamEndCapture failed with 901. The pinned workload hits this: it launches on a second stream while its graph capture is open, and Unit_HRR_PinnedHostRoundtrip failed on MI350. The launch stream is not capturing, so waiting for it is safe. Replay now switches the thread to relaxed capture mode for that one sync and puts the old mode back.

HIP_HRR_HOST_SNAPSHOTS and HIP_HRR_HOST_SNAPSHOT_MAX_MB were release flags in rocclr/utils/flags.hpp. They only tune capture, so they now live in hip_capture.cpp, next to the code that uses them, and flags.hpp is back to develop's. The names, defaults and parsing are unchanged: the snapshot switch is on unless set to something other than "true" or a non-zero number, and the cap is 64 MiB unless set. Each is read once, since every launch asks. Unit_HRR_PinnedHostSnapshotCap captures a 2 MiB pinned allocation with the cap at 2 and at 1 MiB. Within the cap, replay restores it and matches. Over it, the manifest lists it as unreplayable, replay restores nothing and the device-to-host checks fail. The case set in integration.yaml gains it and Unit_HRR_PinnedHostLarge_Direct.

develop took HRR_VERSION 9 for hipKernelSetAttributeForDevice (1baee77), so the pinned host snapshot records move to v10. The version notes in hrr_api_args.h, gen_hrr_api_args.py and DESIGN.md keep both entries.

andraghetti · 2026-10-15T17:43:23Z

HRR_VERSION is now 10, since develop took 9 in 1baee776d0. The description and the MI350X results are updated for 5643bd7542.

mangupta · 2026-10-16T03:26:07Z

🤖 AI-assisted pre-review (for @mangupta)

Re-reviewed the updated head (5643bd75427a) — no new findings at or above the posting threshold.

mangupta · 2026-10-16T13:13:12Z

🤖 AI-assisted pre-review (for @mangupta)

Re-reviewed the updated head (274fb026ec4e) — no new findings at or above the posting threshold.

Copilot

🟡 Changes recommended

Snapshot ordering, allocation lifetime, blocking synchronization and error propagation issues can cause incorrect replay, use-after-free or hangs.

6 open issues

3 resolved since last review

_{Review effort: Balanced}

…ng for it Replay copied a launch's pinned snapshots in after hipStreamSynchronize, which hangs when the stream waits on work the replay thread submits later, and left the copy to race a free on another replay thread. The copy is now a host function queued ahead of the kernel; hipHostFree and hipHostUnregister wait for every stream. A blob the host memory cap refuses or a failed enqueue ends the launch before the kernel runs. Since a restore is ordered on its own stream only, capture records again for a launch on another stream than the allocation's last record.

mangupta · 2026-10-19T09:23:53Z

🤖 AI-assisted pre-review (for @mangupta)

Re-reviewed the updated head (d6b99b1ccc6a) — no new findings at or above the posting threshold.

danielbolsson

Re-checked d6b99b1 against my earlier comments: blobs are now hashed from a copy taken under g_pinned_mu, the atfork handler is back in pinned-then-writer order, capture and replay both use the launch stream's capture state, the low-bits test uses MAP_FIXED_NOREPLACE and skips on EEXIST, and the restore is now a host function queued ahead of the kernel instead of a stream sync. The two stale thread replies are updated, and the develop-merge resolutions keep v10 consistent.

Non-blocking nits:

projects/hrr/playback/hip_playback.cpp:490, projects/hrr/DESIGN.md:559 - ihipFree/ihipHostUnregister call SyncAllStreams() only on the allocation's device, so a restore queued on another GPU's stream is not waited for. Say "every stream of the allocating device", or sync the launch device too.
projects/hrr/tests/integration/hrr_pinned_host_test.cc:363 - Unit_HRR_PinnedHostFork passes if capture is silently off. Also require a blob of at least 32 MiB in the archive.
projects/clr/hipamd/src/hrr/hip_capture.cpp:300 - no test races a host writer against the snapshot, so reverting to hashing the live bytes stays green. Add a test only if it can be made deterministic.

…ees and the extra[] launch hipMallocHost was replayed into a pinned buffer but never tracked, so a kernel reading it ran on replay's empty buffer. Test that hipFree, hipFreeHost and hipHostUnregister stop the tracking, and a hipModuleLaunchKernel launch whose arguments come in extra[]. DESIGN.md now says what the enqueue-time copy does to queued device writes into the allocation.

mangupta · 2026-10-19T13:58:57Z

🤖 AI-assisted pre-review (for @mangupta)

Re-reviewed the updated head (a7b0018c1725) — no new findings at or above the posting threshold.

danielbolsson

Re-checked a7b0018 on top of my approval of d6b99b1: the new pinned_hipMallocHost wraps the generated capture_hipMallocHost shim the same way as pinned_hipHostMalloc (tracks only on success, untracked by the existing hipFreeHost/hipFree wrappers), and the DESIGN.md text on queued device writes matches the enqueue-time copy and the threads you answered. The new tests cover hipMallocHost, all four release paths and the extra[] launch; not built or run on GPU here, and only therock-pr-bot fails in CI.

Nit: projects/hrr/DESIGN.md:547 - the edited line is over the file's wrap width; re-flow the paragraph.

…hrr-metadata-only-capture #13187 took archive version 10 for pinned host snapshot records, so the metadata-only flag moves to v11: HRR_VERSION, the generator template, the DESIGN.md history and current-version lines, and the CHANGELOG entry. hrr_capture_direct keeps HIP_HRR_CAPTURE_CONTENT=1 and applies the caller's environment after it. Unit_HRR_ForkDuringBlobWrite (#13696) needs a blob write to hold, and Unit_HRR_PinnedHostFork (#13187) recorded with content on develop, so both now set HIP_HRR_CAPTURE_CONTENT=1.

#13187 writes each hipHostMalloc, hipMallocHost or hipHostRegister allocation a kernel launch points into to blobs/ before the launch. The inventory gets a row for it, in contents mode only since the snapshot goes through write_blob, and the strings list names it. test_privacy_doc.py checks that PinnedSnapshot writes a blob and that the row names the three APIs.

systems-assistant · 2026-10-21T13:32:20Z

TheRock Submodule Bump Activity

Newest first

2026-10-21 — Included in TheRock via ROCm/TheRock#9398.

andraghetti added 3 commits October 9, 2026 10:26

hrr: snapshot pinned host memory kernels read, and restore it at replay

80ccdb0

hrr: test and document pinned host snapshots

4e60f35

hrr: mark a pinned snapshot as last only after its event is written; …

b6583ca

…more tests

github-actions Bot added project: clr project: hrr labels Oct 9, 2026

systems-assistant Bot added the organization: ROCm label Oct 9, 2026

Merge branch 'develop' into users/andraghetti/hrr-pinned-host-min

c32db55

andraghetti requested a balanced review from Copilot October 12, 2026 08:28

Copilot started reviewing on behalf of andraghetti October 12, 2026 08:29

View session

Copilot AI reviewed Oct 12, 2026

View reviewed changes

Comment thread projects/clr/hipamd/src/hrr/hip_capture.cpp Resolved

Comment thread projects/clr/hipamd/src/hrr/hip_capture.cpp Resolved

Comment thread projects/clr/hipamd/src/hrr/hip_capture.cpp Resolved

andraghetti added 2 commits October 12, 2026 10:45

Merge branch 'develop' into users/andraghetti/hrr-pinned-host-min

7d8bac0

andraghetti marked this pull request as ready for review October 13, 2026 12:09

andraghetti requested review from a team as code owners October 13, 2026 12:09

mangupta reviewed Oct 13, 2026

View reviewed changes

Comment thread projects/clr/hipamd/src/hrr/hip_capture.cpp Resolved

Comment thread projects/clr/hipamd/src/hrr/hip_capture.cpp Resolved

Comment thread projects/clr/hipamd/src/hrr/hip_capture.cpp Resolved

Comment thread projects/clr/hipamd/src/hrr/hip_capture.cpp Resolved

mangupta added the lrt-bot: ready label Oct 13, 2026

andraghetti requested a review from danielbolsson October 13, 2026 14:13

Merge branch 'develop' into users/andraghetti/hrr-pinned-host-min

f43cf80

docs(hrr): drop the lock-order note for the removed pinned fork handler

aa91d3f

andraghetti requested a review from mangupta October 15, 2026 08:52

danielbolsson reviewed Oct 15, 2026

View reviewed changes

Comment thread projects/clr/hipamd/src/hrr/hip_capture.cpp Resolved

Comment thread projects/clr/hipamd/src/hrr/hip_capture.cpp Resolved

Comment thread projects/hrr/playback/hip_playback.cpp Resolved

Comment thread projects/hrr/tests/integration/hrr_pinned_host_test.cc Resolved

andraghetti added 4 commits October 15, 2026 16:50

Merge branch 'develop' of github.com:ROCm/rocm-systems into users/and…

89cc5f4

…raghetti/hrr-pinned-host-min

andraghetti added 2 commits October 15, 2026 18:33

Merge branch 'develop' into users/andraghetti/hrr-pinned-host-min

5643bd7

develop took HRR_VERSION 9 for hipKernelSetAttributeForDevice (1baee77), so the pinned host snapshot records move to v10. The version notes in hrr_api_args.h, gen_hrr_api_args.py and DESIGN.md keep both entries.

Merge branch 'develop' into users/andraghetti/hrr-pinned-host-min

274fb02

andraghetti requested a balanced review from Copilot October 19, 2026 07:16

Copilot started reviewing on behalf of andraghetti October 19, 2026 07:17

View session

Copilot AI reviewed Oct 19, 2026

View reviewed changes

danielbolsson approved these changes Oct 19, 2026

View reviewed changes

danielbolsson approved these changes Oct 20, 2026

View reviewed changes

andraghetti merged commit ae73869 into develop Oct 20, 2026
1212 of 1230 checks passed

andraghetti deleted the users/andraghetti/hrr-pinned-host-min branch October 20, 2026 14:29

andraghetti mentioned this pull request Oct 20, 2026

feat(hrr): make capture metadata-only unless HIP_HRR_CAPTURE_CONTENT is set #13700

Closed

1 task done

mangupta removed the lrt-bot: ready label Oct 21, 2026

This was referenced Oct 21, 2026

[rocjitsu] Correct legacy floating min/max and median selection #13898

Merged

[rocjitsu] Expose backed CDNA3 and CDNA4 hardware identity fields #13899

Merged

[rocjitsu] Implement scratch LDS transfers and scalar cache discard #13900

Merged

Conversation

andraghetti commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Technical Details

Issue Tracking

Test Plan

Test Result

Submission Checklist

Uh oh!

therock-pr-bot Bot commented Oct 9, 2026

✅ All Policy Checks Passed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mangupta left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mangupta commented Oct 14, 2026

Uh oh!

mangupta commented Oct 14, 2026

Uh oh!

danielbolsson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andraghetti commented Oct 15, 2026

Uh oh!

mangupta commented Oct 16, 2026

Uh oh!

mangupta commented Oct 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mangupta commented Oct 19, 2026

Uh oh!

danielbolsson left a comment

Choose a reason for hiding this comment

Uh oh!

mangupta commented Oct 19, 2026

Uh oh!

danielbolsson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

systems-assistant Bot commented Oct 21, 2026

TheRock Submodule Bump Activity

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

andraghetti commented Oct 9, 2026 •

edited

Loading