fix(ep): keep D2H handles in device memory - #7
Closed
fergusfinn wants to merge 1 commit into
Closed
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the EP runtime’s device-read D2H handle storage to live fully in device memory (instead of managed memory + prefetch), aiming to eliminate a suspected source of device-side handle corruption under load.
Changes:
- Switch
d_handle_objsandd_handlesfromcudaMallocManagedtocudaMalloc. - Build matching host-side staging vectors for handle objects and pointer tables.
- Initialize on host once and copy the fully initialized arrays to device via
cudaMemcpy.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+408
to
410
| host_handle_objs[i].init_from_dev_ptr(dev_ptr); | ||
| host_handles[i] = reinterpret_cast<uint64_t>(&d_handle_objs[i]); | ||
| #else |
Comment on lines
+413
to
415
| host_handle_objs[i].init_from_host_value(h); | ||
| host_handles[i] = reinterpret_cast<uint64_t>(d_handle_objs + i); | ||
| #endif |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Allocate the EP runtime's device-read D2H handle arrays (
d_handle_objsand thed_handlespointer table) with plaincudaMalloc+ a one-time host-stagedcudaMemcpy, instead ofcudaMallocManaged+cudaMemPrefetchAsync+cudaDeviceSynchronize. These arrays are written once at init and then read hot bythe device on every dispatch (
D2HHandle::ringis dereferenced to enqueuedevice-to-host commands). The device-visible contents are unchanged; only the
allocation type changes.
Classification
Bug fix / hardening of device-read-hot control state (GH200 managed-memory
corruption).
Mechanism
A
cudaMallocManagedtable of ring pointers is subject to Unified-Memory pagemanagement (migration, eviction, access-counter heuristics, read-duplication). On
GH200 under sustained load this table has been observed to read back zeroed on
the device while a kernel is reading it hot, turning a handle into a null ring
pointer: the device then enqueues D2H commands into nothing and the host proxy never
receives them — surfacing downstream as a dispatch-side CPU recv timeout, null-pointer
faults in the combine/notify readers, and small-batch illegal accesses. It is a
Heisenbug: any host-side engagement with the pages (host scans, host-pinned mapping,
launch-blocking) suppresses it.
cudaMallocpins the table in HBM and removes themanaged page-management surface entirely.
Validation
Two complementary results:
In-situ A/B (this hardware, full serving load): with the handle table in
managed memory the engine is killed within a few high-concurrency waves; the
identical code with the table in plain
cudaMallocdevice memory runs clean acrossrepeated waves. The allocation type was the only variable. This is the evidence
that the change fixes a real under-load failure.
Isolated single-GPU probe (this change, in detail): a faithful model of the
access pattern — a small managed array of ring pointers, device-read-hot inside a
replayed CUDA graph, checked device-side each cycle, against a
cudaMalloccontrol — was stressed with HBM oversubscription, host<->device page-location
thrashing (60k iterations), concurrent multi-stream readers, and host-engagement
on/off. The isolated probe did not reproduce the zeroing. This is consistent
with the Heisenbug: the probe necessarily engages the host each iteration (graph
launch, periodic device-side checks), and host engagement is exactly what
suppresses the corruption. The negative isolated result therefore does not weaken
the in-situ A/B; it matches the documented suppression behaviour.
Verdict
confirmed-fix (in-situ A/B), with the isolated probe consistent via the documented
host-engagement suppression. Keep / land. Device-read-hot, write-once control
state (handle and pointer tables) should not live in managed memory on GH200; this
change moves it to pinned device memory, which is both correct and strictly more
predictable. A fresh single-node graphed-serve A/B of this minimal branch alone
(managed base vs this change) is the one remaining step to re-confirm the extracted
change in isolation end-to-end.