Repository navigation
feat(rocr): Restore SVM-API host unregister with debug skip flag - #356
Closed
dayatsin-amd wants to merge 4 commits into
Closed
dayatsin-amd wants to merge 4 commits into
dayatsin-amd wants to merge 4 commits into
Conversation
Reverts 1471c9c8 ("Krussell/jemallocrevert (#850)"), which backed out the
SVM-API deregister series:
be481399 rocr/thunk: Tear down SVM-API registrations on deregister (#770)
04ed129b rocr/thunk: revoke only unshared sub-ranges on SVM-API deregister
3534936b kfdtest: cover SVM-API register/deregister revoke edge cases
3e32044f fix(rocr/thunk): accept same-base registrations with differing spans
That series revoked GPU access on deregister but never re-granted it on
register. An allocator such as jemalloc hands the same VA back out without
munmap, so the stale NO_ACCESS survived and the GPU faulted on the next
register of that address (ROCM-28520).
The commits that follow supply the missing re-grant and bound the revoke to
the pages a registration actually owns, so the series is safe to carry again.
Restored here as one commit so the completing fixes apply on top.
Signed-off-by: Amir Shetaia <Amir.Shetaia@amd.com>
Deregister revokes GPU access with NO_ACCESS, and allocators like jemalloc reuse the VA without munmap so that attribute persists on the kernel SVM range. Registering the same VA again only re-set the coherency flags, so the GPU faulted (VM_L2_PROTECTION_FAULT) on the re-registered buffer. Re-grant ACCESS_IN_PLACE on every GPU at register. Must be ACCESS_IN_PLACE, not ACCESS: userptr memory can't migrate, so ACCESS makes the retry-fault handler attempt a migration that fails and still faults. Signed-off-by: Amir Shetaia <Amir.Shetaia@amd.com>
Backport of ROCm/rocm-systems#9861 (86a39a38e7, d567045deb, 41833a9b68) to rel-7021. Registrations were tracked by page-aligned base, so distinct sub-page buffers sharing a page were conflated into one refcounted entry whose extent had to cover the largest of them. Deregistering a small buffer then revoked GPU access across a span it never owned. Key the tree by the exact user pointer instead, which is what register and deregister both carry. Register rounds out to page boundaries because that is what has to be mapped; deregister revoked that same rounded range, so bytes past either end lost GPU access too, and those belong to other live allocations since an allocator packs many objects per page. Track a separate inward-rounded revoke extent, [PAGE_ALIGN_UP(ptr), PAGE_ALIGN_DOWN(ptr + size)), so a buffer filling no whole page revokes nothing. The leftward sweep for surviving overlapping registrations started at the immediate predecessor of base, which misses a longer range further left that still covers base. Bound it by svm_api_max_extent instead, and reset that once nothing is tracked so one large registration does not widen every later scan for the life of the process. Stop clearing the coherency flags on deregister. They are page granular and COHERENT is the kernel default, so clearing them drops whatever else shares those pages below default; on gfx942/950 that also turns a live mapping cached. NO_ACCESS is what the unmap and eviction skip key off. Track the registration only after the kernel has the attributes, so a failed register leaves nothing behind and there is no reserved extent to roll back. Signed-off-by: Amir Shetaia <Amir.Shetaia@amd.com>
Host ranges registered through the KFD SVM API have no vm_object, so deregister was a silent no-op and a later jemalloc reuse of the same VA could still see NO_ACCESS. Track those ranges, revoke GPU access on the last drop, re-grant ACCESS_IN_PLACE on re-register, and allow skipping that path with HSA_SKIP_SVM_HOST_UNREGISTER_DEBUG. Co-authored-by: Cursor <cursoragent@cursor.com>
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
Restore teardown of host ranges registered through the KFD SVM API, and add a temporary debug env var so that path can be A/B tested.
This is the ROCR-Runtime counterpart of ROCm/rocm-systems#11411, based on
release/rocm-rel-7.0.2.1.Those registrations only set SVM attributes and do not create a
vm_object, sohsaKmtDeregisterMemorywas a silent no-op. After a revoke, a later register of a reused VA (typical of jemalloc) could still seeNO_ACCESSin the kernel. This change:NO_ACCESSfor only the unshared sub-ranges on last deregisterACCESS_IN_PLACEplus coherency flags on re-registerHSA_SKIP_SVM_HOST_UNREGISTER_DEBUGis set to a non-0value (coherency flags only, matching the previous no-op unregister behavior)JIRA ID
JIRA ID : ROCM-30561
Test plan
libhsa-runtime64on e3-2b (gfx950) against/opt/rocmand ran withLD_LIBRARY_PATHpointing at the new libraryrocrtst64: 109/109 passed withHSA_SKIP_SVM_HOST_UNREGISTER_DEBUG=0and=1Unit_hipStreamAttachMemAsync_Negative,Unit_hipIpcMemAccess_Semaphores)/opt/rocm/lib/libhsa-runtime64.so.1Made with Cursor