Repository navigation
Revert "fix(rocr/thunk): Revoke SVM-API GPU access attributes on deregister" - #9187
Merged
Merged
Conversation
…gister (…" This reverts commit ce90901.
❌ PR Check — Action Required
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
amd-chiranjeevi
marked this pull request as ready for review
July 24, 2026 13:53
amd-chiranjeevi
requested review from
cfreeamd,
dayatsin-amd,
kentrussell and
xiaogang-chen-amd
as code owners
July 24, 2026 13:53
Closed
1 task done
14 tasks done
17 tasks done
ammallya
pushed a commit
that referenced
this pull request
Sep 24, 2026
) ## Summary Restore teardown of host ranges registered through the KFD SVM API, behind a debug env var that is **off by default**. Those registrations only set SVM attributes and do not create a `vm_object`, so `hsaKmtDeregisterMemory` was a silent no-op. After a revoke, a later register of a reused VA (typical of jemalloc) could still see `NO_ACCESS` in the kernel. When enabled, this change: - tracks SVM-API host ranges (keyed by the user pointer, refcounted) - issues `NO_ACCESS` for only the sub-ranges no surviving registration still covers, on last deregister - re-grants `ACCESS_IN_PLACE` plus the coherency flags on re-register - serializes the grant against the revoke (see below) ### Enabling it `HSA_SVM_HOST_UNREGISTER_DEBUG` set to a non-`0` value turns the unregister path on. Unset - the default - deregistering an SVM-API host range stays the no-op it has always been, and register only sets the two coherency flags, exactly as before this PR. The restored teardown therefore ships dormant and is opted into for testing rather than opted out of. With it off, `svm_api_mutex` is never taken and no range tracking happens. ### Register/revoke serialization The first commit picked the revoke ranges under `svm_api_mutex` but issued `NO_ACCESS` after dropping it, and register granted `ACCESS_IN_PLACE` before inserting into the tracking tree. Either gap lets a `NO_ACCESS` reach the kernel after a later grant for the same pages, so a live registration loses GPU access and the copy in flight on it faults with `Write access to a read-only page`. This was easy to hit because `hipMemcpy2DAsync` pins the page-rounded-out host range: two adjacent `malloc(65536)` buffers get 17-page pins that share one page, and that page was revoked whenever either buffer's refcount reached zero. On 8x gfx950 `Unit_hipMemset2DAsync_MultiThread` aborted on every run. The fix tracks the range before the grant ioctl and holds `svm_api_mutex` across both the grant and the revoke, so the tree and the kernel agree on one order. The reference is given back if the grant fails. ### History: what the first attempt broke, and what is different here This re-lands #6699 (`ce90901121a`), reverted in #9187 after #9202 reported ~20 math-lib jobs on gfx942/gfx950 **returning wrong values** - not crashing. The revoke was too broad: it keyed the tracking by the page-aligned base and revoked the whole page-rounded extent, so deregistering one buffer dropped GPU access to pages a *neighbouring* live registration still owned. A kernel then read from a range the GPU could no longer translate. Four things address that here: - keyed by the exact user pointer, so two sub-page buffers sharing a page-aligned base are separate registrations rather than one merged extent - interval subtraction at revoke time, so only the sub-ranges no surviving registration still covers are revoked - `ACCESS_IN_PLACE` is re-granted on re-register, so a VA an allocator hands back out does not stay revoked - the grant and the revoke are serialized under `svm_api_mutex`, so a `NO_ACCESS` cannot land after a later grant for the same pages **With the flag unset - the default - none of that machinery runs at all.** `svm_api_mutex` is never taken, no range is tracked, no allocation happens on the register path, and deregister returns without issuing any ioctl, exactly as before this PR. The only way to reach the new code is to opt in. ## JIRA ID JIRA ID : ROCM-30561 ### Rebased onto current develop Re-landed on develop after the `pagedSvm` / SVM-paged-host-memory work went in. The textual conflict was one declaration line. The functional overlap needed a fifth commit: `fmm_allocate_host_gpu()` is now a second caller of `fmm_register_mem_svm_api()`, and that path releases the object instead of deregistering it, so the tracking reference was taken and never given back. `__fmm_release()` now drops it for `is_svm_paged` objects. Revalidated on 8x gfx950 (amdgpu 7.1.9-2402117 plus the XCC-parallel TLB flush patch, ROCm 10.2.0), rebuilt from this branch: - [x] `rocrtst64`: 112/112 passed - [x] ioctl attribute counts unchanged: register 10 attrs, map 1, revoke 8 - [x] reproducer: 1916 copies/s with the path on, 28457 with it off, no faults ## Test plan Built `libhsa-runtime64` against `/opt/rocm` and loaded it ahead of the installed copy. "off" is the default; "on" is `HSA_SVM_HOST_UNREGISTER_DEBUG=1`. Full suites on **8x gfx950** (amdgpu 7.1.9-2402117, ROCm 10.2.0), run in both modes: - [x] `rocrtst64`: 108/108 passed off and on (`Memory_Max_Mem` excluded) - [x] HIP catch (gfx950 ignore list): **4941/4941 passed off and on** - [x] rocprim / rocthrust / rocrand, 20 binaries: 0 failures off and on - [x] `rocsparse-test` (`*bsrmv*:*bsrilu0*:*gebsr2gebsr*`): 7395/7395 off and on Race fix, same 8x gfx950 box, unregister on: - [x] `Unit_hipMemset2DAsync_MultiThread`: 0-1 of 15 passed before the serialization fix, 20/20 after - [x] Threaded `hipMemset2DAsync` + `hipMemcpy2DAsync` reproducer: 0/10 passed before, 20/20 after - [x] Both 12/12 again on the final tree, off and on Race fix, 1x gfx950 in CPX (8 partitions), which exercises the same 8-way attribute arrays: - [x] `Unit_hipMemset2DAsync_MultiThread`: 9/15 passed before the fix at default xnack and 2/15 with `HSA_XNACK=0`; 15/15 after the fix in both - [x] The reproducer hangs rather than aborts there, with `MAPPING_ERROR` and a retry-fault storm in dmesg; 20/20 clean after the fix - [x] Single-threaded and page-aligned-buffer variants pass either way, confirming the failure was the register/revoke ordering rather than the revoke extent Suites on 1x gfx950, both modes: - [x] `rocrtst64`: 109/109 passed in both - [x] HIP catch: same 2 failures in both, and both also fail with the stock `libhsa-runtime64.so.1` (`Unit_hipStreamAttachMemAsync_Negative`, `Unit_hipIpcMemAccess_Semaphores`) - [x] rocprim / rocthrust / rocrand / rocsparse, 21 binaries incl. 7395 `rocsparse-test` cases: 0 failures in both - [x] `AMDKFD_IOC_SVM` ioctl sizes confirm the two modes differ: off sends the 2-attr register only, on also sends the per-GPU `NO_ACCESS` revoke and the per-GPU `ACCESS_IN_PLACE` re-grant --------- Co-authored-by: Cursor <cursoragent@cursor.com>
anujshuk-amd
pushed a commit
that referenced
this pull request
Sep 25, 2026
) ## Summary Restore teardown of host ranges registered through the KFD SVM API, behind a debug env var that is **off by default**. Those registrations only set SVM attributes and do not create a `vm_object`, so `hsaKmtDeregisterMemory` was a silent no-op. After a revoke, a later register of a reused VA (typical of jemalloc) could still see `NO_ACCESS` in the kernel. When enabled, this change: - tracks SVM-API host ranges (keyed by the user pointer, refcounted) - issues `NO_ACCESS` for only the sub-ranges no surviving registration still covers, on last deregister - re-grants `ACCESS_IN_PLACE` plus the coherency flags on re-register - serializes the grant against the revoke (see below) ### Enabling it `HSA_SVM_HOST_UNREGISTER_DEBUG` set to a non-`0` value turns the unregister path on. Unset - the default - deregistering an SVM-API host range stays the no-op it has always been, and register only sets the two coherency flags, exactly as before this PR. The restored teardown therefore ships dormant and is opted into for testing rather than opted out of. With it off, `svm_api_mutex` is never taken and no range tracking happens. ### Register/revoke serialization The first commit picked the revoke ranges under `svm_api_mutex` but issued `NO_ACCESS` after dropping it, and register granted `ACCESS_IN_PLACE` before inserting into the tracking tree. Either gap lets a `NO_ACCESS` reach the kernel after a later grant for the same pages, so a live registration loses GPU access and the copy in flight on it faults with `Write access to a read-only page`. This was easy to hit because `hipMemcpy2DAsync` pins the page-rounded-out host range: two adjacent `malloc(65536)` buffers get 17-page pins that share one page, and that page was revoked whenever either buffer's refcount reached zero. On 8x gfx950 `Unit_hipMemset2DAsync_MultiThread` aborted on every run. The fix tracks the range before the grant ioctl and holds `svm_api_mutex` across both the grant and the revoke, so the tree and the kernel agree on one order. The reference is given back if the grant fails. ### History: what the first attempt broke, and what is different here This re-lands #6699 (`ce90901121a`), reverted in #9187 after #9202 reported ~20 math-lib jobs on gfx942/gfx950 **returning wrong values** - not crashing. The revoke was too broad: it keyed the tracking by the page-aligned base and revoked the whole page-rounded extent, so deregistering one buffer dropped GPU access to pages a *neighbouring* live registration still owned. A kernel then read from a range the GPU could no longer translate. Four things address that here: - keyed by the exact user pointer, so two sub-page buffers sharing a page-aligned base are separate registrations rather than one merged extent - interval subtraction at revoke time, so only the sub-ranges no surviving registration still covers are revoked - `ACCESS_IN_PLACE` is re-granted on re-register, so a VA an allocator hands back out does not stay revoked - the grant and the revoke are serialized under `svm_api_mutex`, so a `NO_ACCESS` cannot land after a later grant for the same pages **With the flag unset - the default - none of that machinery runs at all.** `svm_api_mutex` is never taken, no range is tracked, no allocation happens on the register path, and deregister returns without issuing any ioctl, exactly as before this PR. The only way to reach the new code is to opt in. ## JIRA ID JIRA ID : ROCM-30561 ### Rebased onto current develop Re-landed on develop after the `pagedSvm` / SVM-paged-host-memory work went in. The textual conflict was one declaration line. The functional overlap needed a fifth commit: `fmm_allocate_host_gpu()` is now a second caller of `fmm_register_mem_svm_api()`, and that path releases the object instead of deregistering it, so the tracking reference was taken and never given back. `__fmm_release()` now drops it for `is_svm_paged` objects. Revalidated on 8x gfx950 (amdgpu 7.1.9-2402117 plus the XCC-parallel TLB flush patch, ROCm 10.2.0), rebuilt from this branch: - [x] `rocrtst64`: 112/112 passed - [x] ioctl attribute counts unchanged: register 10 attrs, map 1, revoke 8 - [x] reproducer: 1916 copies/s with the path on, 28457 with it off, no faults ## Test plan Built `libhsa-runtime64` against `/opt/rocm` and loaded it ahead of the installed copy. "off" is the default; "on" is `HSA_SVM_HOST_UNREGISTER_DEBUG=1`. Full suites on **8x gfx950** (amdgpu 7.1.9-2402117, ROCm 10.2.0), run in both modes: - [x] `rocrtst64`: 108/108 passed off and on (`Memory_Max_Mem` excluded) - [x] HIP catch (gfx950 ignore list): **4941/4941 passed off and on** - [x] rocprim / rocthrust / rocrand, 20 binaries: 0 failures off and on - [x] `rocsparse-test` (`*bsrmv*:*bsrilu0*:*gebsr2gebsr*`): 7395/7395 off and on Race fix, same 8x gfx950 box, unregister on: - [x] `Unit_hipMemset2DAsync_MultiThread`: 0-1 of 15 passed before the serialization fix, 20/20 after - [x] Threaded `hipMemset2DAsync` + `hipMemcpy2DAsync` reproducer: 0/10 passed before, 20/20 after - [x] Both 12/12 again on the final tree, off and on Race fix, 1x gfx950 in CPX (8 partitions), which exercises the same 8-way attribute arrays: - [x] `Unit_hipMemset2DAsync_MultiThread`: 9/15 passed before the fix at default xnack and 2/15 with `HSA_XNACK=0`; 15/15 after the fix in both - [x] The reproducer hangs rather than aborts there, with `MAPPING_ERROR` and a retry-fault storm in dmesg; 20/20 clean after the fix - [x] Single-threaded and page-aligned-buffer variants pass either way, confirming the failure was the register/revoke ordering rather than the revoke extent Suites on 1x gfx950, both modes: - [x] `rocrtst64`: 109/109 passed in both - [x] HIP catch: same 2 failures in both, and both also fail with the stock `libhsa-runtime64.so.1` (`Unit_hipStreamAttachMemAsync_Negative`, `Unit_hipIpcMemAccess_Semaphores`) - [x] rocprim / rocthrust / rocrand / rocsparse, 21 binaries incl. 7395 `rocsparse-test` cases: 0 failures in both - [x] `AMDKFD_IOC_SVM` ioctl sizes confirm the two modes differ: off sends the 2-attr register only, on also sends the per-GPU `NO_ACCESS` revoke and the per-GPU `ACCESS_IN_PLACE` re-grant --------- Co-authored-by: Cursor <cursoragent@cursor.com>
dgaliffiAMD
pushed a commit
that referenced
this pull request
Sep 25, 2026
) ## Summary Restore teardown of host ranges registered through the KFD SVM API, behind a debug env var that is **off by default**. Those registrations only set SVM attributes and do not create a `vm_object`, so `hsaKmtDeregisterMemory` was a silent no-op. After a revoke, a later register of a reused VA (typical of jemalloc) could still see `NO_ACCESS` in the kernel. When enabled, this change: - tracks SVM-API host ranges (keyed by the user pointer, refcounted) - issues `NO_ACCESS` for only the sub-ranges no surviving registration still covers, on last deregister - re-grants `ACCESS_IN_PLACE` plus the coherency flags on re-register - serializes the grant against the revoke (see below) ### Enabling it `HSA_SVM_HOST_UNREGISTER_DEBUG` set to a non-`0` value turns the unregister path on. Unset - the default - deregistering an SVM-API host range stays the no-op it has always been, and register only sets the two coherency flags, exactly as before this PR. The restored teardown therefore ships dormant and is opted into for testing rather than opted out of. With it off, `svm_api_mutex` is never taken and no range tracking happens. ### Register/revoke serialization The first commit picked the revoke ranges under `svm_api_mutex` but issued `NO_ACCESS` after dropping it, and register granted `ACCESS_IN_PLACE` before inserting into the tracking tree. Either gap lets a `NO_ACCESS` reach the kernel after a later grant for the same pages, so a live registration loses GPU access and the copy in flight on it faults with `Write access to a read-only page`. This was easy to hit because `hipMemcpy2DAsync` pins the page-rounded-out host range: two adjacent `malloc(65536)` buffers get 17-page pins that share one page, and that page was revoked whenever either buffer's refcount reached zero. On 8x gfx950 `Unit_hipMemset2DAsync_MultiThread` aborted on every run. The fix tracks the range before the grant ioctl and holds `svm_api_mutex` across both the grant and the revoke, so the tree and the kernel agree on one order. The reference is given back if the grant fails. ### History: what the first attempt broke, and what is different here This re-lands #6699 (`ce90901121a`), reverted in #9187 after #9202 reported ~20 math-lib jobs on gfx942/gfx950 **returning wrong values** - not crashing. The revoke was too broad: it keyed the tracking by the page-aligned base and revoked the whole page-rounded extent, so deregistering one buffer dropped GPU access to pages a *neighbouring* live registration still owned. A kernel then read from a range the GPU could no longer translate. Four things address that here: - keyed by the exact user pointer, so two sub-page buffers sharing a page-aligned base are separate registrations rather than one merged extent - interval subtraction at revoke time, so only the sub-ranges no surviving registration still covers are revoked - `ACCESS_IN_PLACE` is re-granted on re-register, so a VA an allocator hands back out does not stay revoked - the grant and the revoke are serialized under `svm_api_mutex`, so a `NO_ACCESS` cannot land after a later grant for the same pages **With the flag unset - the default - none of that machinery runs at all.** `svm_api_mutex` is never taken, no range is tracked, no allocation happens on the register path, and deregister returns without issuing any ioctl, exactly as before this PR. The only way to reach the new code is to opt in. ## JIRA ID JIRA ID : ROCM-30561 ### Rebased onto current develop Re-landed on develop after the `pagedSvm` / SVM-paged-host-memory work went in. The textual conflict was one declaration line. The functional overlap needed a fifth commit: `fmm_allocate_host_gpu()` is now a second caller of `fmm_register_mem_svm_api()`, and that path releases the object instead of deregistering it, so the tracking reference was taken and never given back. `__fmm_release()` now drops it for `is_svm_paged` objects. Revalidated on 8x gfx950 (amdgpu 7.1.9-2402117 plus the XCC-parallel TLB flush patch, ROCm 10.2.0), rebuilt from this branch: - [x] `rocrtst64`: 112/112 passed - [x] ioctl attribute counts unchanged: register 10 attrs, map 1, revoke 8 - [x] reproducer: 1916 copies/s with the path on, 28457 with it off, no faults ## Test plan Built `libhsa-runtime64` against `/opt/rocm` and loaded it ahead of the installed copy. "off" is the default; "on" is `HSA_SVM_HOST_UNREGISTER_DEBUG=1`. Full suites on **8x gfx950** (amdgpu 7.1.9-2402117, ROCm 10.2.0), run in both modes: - [x] `rocrtst64`: 108/108 passed off and on (`Memory_Max_Mem` excluded) - [x] HIP catch (gfx950 ignore list): **4941/4941 passed off and on** - [x] rocprim / rocthrust / rocrand, 20 binaries: 0 failures off and on - [x] `rocsparse-test` (`*bsrmv*:*bsrilu0*:*gebsr2gebsr*`): 7395/7395 off and on Race fix, same 8x gfx950 box, unregister on: - [x] `Unit_hipMemset2DAsync_MultiThread`: 0-1 of 15 passed before the serialization fix, 20/20 after - [x] Threaded `hipMemset2DAsync` + `hipMemcpy2DAsync` reproducer: 0/10 passed before, 20/20 after - [x] Both 12/12 again on the final tree, off and on Race fix, 1x gfx950 in CPX (8 partitions), which exercises the same 8-way attribute arrays: - [x] `Unit_hipMemset2DAsync_MultiThread`: 9/15 passed before the fix at default xnack and 2/15 with `HSA_XNACK=0`; 15/15 after the fix in both - [x] The reproducer hangs rather than aborts there, with `MAPPING_ERROR` and a retry-fault storm in dmesg; 20/20 clean after the fix - [x] Single-threaded and page-aligned-buffer variants pass either way, confirming the failure was the register/revoke ordering rather than the revoke extent Suites on 1x gfx950, both modes: - [x] `rocrtst64`: 109/109 passed in both - [x] HIP catch: same 2 failures in both, and both also fail with the stock `libhsa-runtime64.so.1` (`Unit_hipStreamAttachMemAsync_Negative`, `Unit_hipIpcMemAccess_Semaphores`) - [x] rocprim / rocthrust / rocrand / rocsparse, 21 binaries incl. 7395 `rocsparse-test` cases: 0 failures in both - [x] `AMDKFD_IOC_SVM` ioctl sizes confirm the two modes differ: off sends the 2-attr register only, on also sends the per-GPU `NO_ACCESS` revoke and the per-GPU `ACCESS_IN_PLACE` re-grant --------- 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.
Reverts PR: #6699
Fixes Issue: #9202
Test Run: https://github.com/ROCm/TheRock/actions/runs/30089750380/job/89484623849?pr=6839 --> all the math lib tests passed