On 13.10.25 09:19, Khatri, Sunil wrote: > > On 10/13/2025 12:42 PM, Christian König wrote: >> On 10.10.25 16:07, Sunil Khatri wrote: >>> At times we need a bo reference for hmm and for that add >>> a new struct amdgpu_hmm_range which will hold an optional >>> bo member and hmm_range. >>> >>> Use amdgpu_hmm_range instead of hmm_range and let the bo >>> as an optional argument for the caller if they want to >>> the bo reference to be taken or they want to handle that >>> explicitly. >>> >>> Signed-off-by: Sunil Khatri <[email protected]> >>> --- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 2 +- >>> .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 6 ++-- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h | 2 +- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 6 ++-- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4 +-- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c | 36 +++++++++++++------ >>> drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.h | 19 ++++++---- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 6 ++-- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 7 ++-- >>> drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 1 - >>> drivers/gpu/drm/amd/amdkfd/kfd_migrate.h | 1 - >>> drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 14 ++++---- >>> drivers/gpu/drm/amd/amdkfd/kfd_svm.h | 1 - >>> 13 files changed, 61 insertions(+), 44 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h >>> index 9e120c934cc1..8bdfcde2029b 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h >>> @@ -71,7 +71,7 @@ struct kgd_mem { >>> struct mutex lock; >>> struct amdgpu_bo *bo; >>> struct dma_buf *dmabuf; >>> - struct hmm_range *range; >>> + struct amdgpu_hmm_range *range; >>> struct list_head attachments; >>> /* protected by amdkfd_process_info.lock */ >>> struct list_head validate_list; >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c >>> index 22c1bdc53d2e..56097fb6eecd 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c >>> @@ -1057,7 +1057,7 @@ static int init_user_pages(struct kgd_mem *mem, >>> uint64_t user_addr, >>> struct amdkfd_process_info *process_info = mem->process_info; >>> struct amdgpu_bo *bo = mem->bo; >>> struct ttm_operation_ctx ctx = { true, false }; >>> - struct hmm_range *range; >>> + struct amdgpu_hmm_range *range; >>> int ret = 0; >>> mutex_lock(&process_info->lock); >>> @@ -1089,7 +1089,7 @@ static int init_user_pages(struct kgd_mem *mem, >>> uint64_t user_addr, >>> return 0; >>> } >>> - range = amdgpu_hmm_range_alloc(); >>> + range = amdgpu_hmm_range_alloc(NULL); >>> if (unlikely(!range)) { >>> ret = -ENOMEM; >>> goto unregister_out; >>> @@ -2574,7 +2574,7 @@ static int update_invalid_user_pages(struct >>> amdkfd_process_info *process_info, >>> } >>> } >>> - mem->range = amdgpu_hmm_range_alloc(); >>> + mem->range = amdgpu_hmm_range_alloc(NULL); >>> if (unlikely(!mem->range)) >>> return -ENOMEM; >>> /* Get updated user pages */ >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h >>> index a716c9886c74..2b5e7c46a39d 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h >>> @@ -38,7 +38,7 @@ struct amdgpu_bo_list_entry { >>> struct amdgpu_bo *bo; >>> struct amdgpu_bo_va *bo_va; >>> uint32_t priority; >>> - struct hmm_range *range; >>> + struct amdgpu_hmm_range *range; >>> bool user_invalidated; >>> }; >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>> index 087e5b5497e4..87872c0282e4 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>> @@ -29,7 +29,6 @@ >>> #include <linux/pagemap.h> >>> #include <linux/sync_file.h> >>> #include <linux/dma-buf.h> >>> -#include <linux/hmm.h> >>> #include <drm/amdgpu_drm.h> >>> #include <drm/drm_syncobj.h> >>> @@ -886,7 +885,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser >>> *p, >>> bool userpage_invalidated = false; >>> struct amdgpu_bo *bo = e->bo; >>> - e->range = amdgpu_hmm_range_alloc(); >>> + e->range = amdgpu_hmm_range_alloc(NULL); >>> if (unlikely(!e->range)) >>> return -ENOMEM; >>> @@ -895,7 +894,8 @@ static int amdgpu_cs_parser_bos(struct >>> amdgpu_cs_parser *p, >>> goto out_free_user_pages; >>> for (i = 0; i < bo->tbo.ttm->num_pages; i++) { >>> - if (bo->tbo.ttm->pages[i] != >>> hmm_pfn_to_page(e->range->hmm_pfns[i])) { >>> + if (bo->tbo.ttm->pages[i] != >>> + hmm_pfn_to_page(e->range->hmm_range.hmm_pfns[i])) { >>> userpage_invalidated = true; >>> break; >>> } >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c >>> index 82ddc8c22b64..ce073e894584 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c >>> @@ -530,7 +530,7 @@ int amdgpu_gem_userptr_ioctl(struct drm_device *dev, >>> void *data, >>> struct drm_amdgpu_gem_userptr *args = data; >>> struct amdgpu_fpriv *fpriv = filp->driver_priv; >>> struct drm_gem_object *gobj; >>> - struct hmm_range *range; >>> + struct amdgpu_hmm_range *range; >>> struct amdgpu_bo *bo; >>> uint32_t handle; >>> int r; >>> @@ -571,7 +571,7 @@ int amdgpu_gem_userptr_ioctl(struct drm_device *dev, >>> void *data, >>> goto release_object; >>> if (args->flags & AMDGPU_GEM_USERPTR_VALIDATE) { >>> - range = amdgpu_hmm_range_alloc(); >>> + range = amdgpu_hmm_range_alloc(NULL); >>> if (unlikely(!range)) >>> return -ENOMEM; >>> r = amdgpu_ttm_tt_get_user_pages(bo, range); >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c >>> index b582fd217bd0..9da1c5c69632 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c >>> @@ -168,12 +168,13 @@ void amdgpu_hmm_unregister(struct amdgpu_bo *bo) >>> int amdgpu_hmm_range_get_pages(struct mmu_interval_notifier *notifier, >>> uint64_t start, uint64_t npages, bool readonly, >>> void *owner, >>> - struct hmm_range *hmm_range) >>> + struct amdgpu_hmm_range *range) >>> { >>> unsigned long end; >>> unsigned long timeout; >>> unsigned long *pfns; >>> int r = 0; >>> + struct hmm_range *hmm_range = &range->hmm_range; >>> pfns = kvmalloc_array(npages, sizeof(*pfns), GFP_KERNEL); >>> if (unlikely(!pfns)) { >>> @@ -226,25 +227,38 @@ int amdgpu_hmm_range_get_pages(struct >>> mmu_interval_notifier *notifier, >>> return r; >>> } >>> -bool amdgpu_hmm_range_valid(struct hmm_range *hmm_range) >>> +bool amdgpu_hmm_range_valid(struct amdgpu_hmm_range *range) >>> { >>> - if (!hmm_range) >>> + if (!range) >>> return false; >>> - return !mmu_interval_read_retry(hmm_range->notifier, >>> - hmm_range->notifier_seq); >>> + return !mmu_interval_read_retry(range->hmm_range.notifier, >>> + range->hmm_range.notifier_seq); >>> } >>> -struct hmm_range *amdgpu_hmm_range_alloc(void) >>> +struct amdgpu_hmm_range *amdgpu_hmm_range_alloc(struct amdgpu_bo *bo) >>> { >>> - return kzalloc(sizeof(struct hmm_range), GFP_KERNEL); >>> + struct amdgpu_hmm_range *range; >>> + >>> + range = kzalloc(sizeof(struct amdgpu_hmm_range), GFP_KERNEL); >>> + if (!range) >>> + return NULL; >>> + >>> + if (bo) >>> + range->bo = amdgpu_bo_ref(bo); >> The functions amdgpu_bo_ref() and ... >> >>> + >>> + return range; >>> } >>> -void amdgpu_hmm_range_free(struct hmm_range *hmm_range) >>> +void amdgpu_hmm_range_free(struct amdgpu_hmm_range *range) >>> { >>> - if (!hmm_range) >>> + if (!range) >>> return; >>> - kvfree(hmm_range->hmm_pfns); >>> - kfree(hmm_range); >>> + kvfree(range->hmm_range.hmm_pfns); >>> + >>> + if (range->bo) >>> + amdgpu_bo_unref(&range->bo); >> ... amdgpu_bo_unref() have intergrated NULL checks. So checking it here is >> superflous. > Sure will do that. >> >> It would also be helpful to have some kerneldoc on amdgpu_hmm_range_alloc() >> and amdgpu_hmm_range_free(), but that is not a must have. > > I guess that will be in a new patch set. will see how it is done and if i can > use and existing one to update that.
Works for me. A lot of people also prefer to add kerneldoc as separate patch at the end of their patchset, so that you can see first how all puzzle pieces fall together before you add documentattion about it. Regards, Christian. > > Regards > Sunil Khatri > >> >> With that fixed the patch is Reviewed-by: Christian König >> <[email protected]> >> >> Regards, >> Christian. >> >>> + >>> + kfree(range); >>> } >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.h >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.h >>> index e85f912b8938..140bc9cd57b4 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.h >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.h >>> @@ -31,15 +31,20 @@ >>> #include <linux/interval_tree.h> >>> #include <linux/mmu_notifier.h> >>> +struct amdgpu_hmm_range { >>> + struct hmm_range hmm_range; >>> + struct amdgpu_bo *bo; >>> +}; >>> + >>> int amdgpu_hmm_range_get_pages(struct mmu_interval_notifier *notifier, >>> uint64_t start, uint64_t npages, bool readonly, >>> void *owner, >>> - struct hmm_range *hmm_range); >>> + struct amdgpu_hmm_range *range); >>> #if defined(CONFIG_HMM_MIRROR) >>> -bool amdgpu_hmm_range_valid(struct hmm_range *hmm_range); >>> -struct hmm_range *amdgpu_hmm_range_alloc(void); >>> -void amdgpu_hmm_range_free(struct hmm_range *hmm_range); >>> +bool amdgpu_hmm_range_valid(struct amdgpu_hmm_range *range); >>> +struct amdgpu_hmm_range *amdgpu_hmm_range_alloc(struct amdgpu_bo *bo); >>> +void amdgpu_hmm_range_free(struct amdgpu_hmm_range *range); >>> int amdgpu_hmm_register(struct amdgpu_bo *bo, unsigned long addr); >>> void amdgpu_hmm_unregister(struct amdgpu_bo *bo); >>> #else >>> @@ -52,17 +57,17 @@ static inline int amdgpu_hmm_register(struct amdgpu_bo >>> *bo, unsigned long addr) >>> static inline void amdgpu_hmm_unregister(struct amdgpu_bo *bo) {} >>> -static inline bool amdgpu_hmm_range_valid(struct hmm_range *hmm_range) >>> +static inline bool amdgpu_hmm_range_valid(struct amdgpu_hmm_range *range) >>> { >>> return false; >>> } >>> -static inline struct hmm_range *amdgpu_hmm_range_alloc(void) >>> +static inline struct amdgpu_hmm_range *amdgpu_hmm_range_alloc(struct >>> amdgpu_bo *bo) >>> { >>> return NULL; >>> } >>> -static inline void amdgpu_hmm_range_free(struct hmm_range *hmm_range) {} >>> +static inline void amdgpu_hmm_range_free(struct amdgpu_hmm_range *range) {} >>> #endif >>> #endif >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >>> index 96bd0185f936..fd00ec7c99a1 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >>> @@ -709,7 +709,7 @@ struct amdgpu_ttm_tt { >>> * that range is a valid memory and it is freed too. >>> */ >>> int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, >>> - struct hmm_range *range) >>> + struct amdgpu_hmm_range *range) >>> { >>> struct ttm_tt *ttm = bo->tbo.ttm; >>> struct amdgpu_ttm_tt *gtt = ttm_to_amdgpu_ttm_tt(ttm); >>> @@ -762,12 +762,12 @@ int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, >>> * that backs user memory and will ultimately be mapped into the device >>> * address space. >>> */ >>> -void amdgpu_ttm_tt_set_user_pages(struct ttm_tt *ttm, struct hmm_range >>> *range) >>> +void amdgpu_ttm_tt_set_user_pages(struct ttm_tt *ttm, struct >>> amdgpu_hmm_range *range) >>> { >>> unsigned long i; >>> for (i = 0; i < ttm->num_pages; ++i) >>> - ttm->pages[i] = range ? hmm_pfn_to_page(range->hmm_pfns[i]) : NULL; >>> + ttm->pages[i] = range ? >>> hmm_pfn_to_page(range->hmm_range.hmm_pfns[i]) : NULL; >>> } >>> /* >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h >>> index 99c46821b961..0ebb99e8d792 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h >>> @@ -28,6 +28,7 @@ >>> #include <drm/gpu_scheduler.h> >>> #include <drm/ttm/ttm_placement.h> >>> #include "amdgpu_vram_mgr.h" >>> +#include "amdgpu_hmm.h" >>> #define AMDGPU_PL_GDS (TTM_PL_PRIV + 0) >>> #define AMDGPU_PL_GWS (TTM_PL_PRIV + 1) >>> @@ -191,16 +192,16 @@ uint64_t amdgpu_ttm_domain_start(struct amdgpu_device >>> *adev, uint32_t type); >>> #if IS_ENABLED(CONFIG_DRM_AMDGPU_USERPTR) >>> int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, >>> - struct hmm_range *range); >>> + struct amdgpu_hmm_range *range); >>> #else >>> static inline int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, >>> - struct hmm_range *range) >>> + struct amdgpu_hmm_range *range) >>> { >>> return -EPERM; >>> } >>> #endif >>> -void amdgpu_ttm_tt_set_user_pages(struct ttm_tt *ttm, struct hmm_range >>> *range); >>> +void amdgpu_ttm_tt_set_user_pages(struct ttm_tt *ttm, struct >>> amdgpu_hmm_range *range); >>> int amdgpu_ttm_tt_get_userptr(const struct ttm_buffer_object *tbo, >>> uint64_t *user_addr); >>> int amdgpu_ttm_tt_set_userptr(struct ttm_buffer_object *bo, >>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c >>> b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c >>> index d10c6673f4de..3653c563ee9a 100644 >>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c >>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c >>> @@ -21,7 +21,6 @@ >>> * OTHER DEALINGS IN THE SOFTWARE. >>> */ >>> #include <linux/types.h> >>> -#include <linux/hmm.h> >>> #include <linux/dma-direction.h> >>> #include <linux/dma-mapping.h> >>> #include <linux/migrate.h> >>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.h >>> b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.h >>> index 2eebf67f9c2c..2b7fd442d29c 100644 >>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.h >>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.h >>> @@ -31,7 +31,6 @@ >>> #include <linux/list.h> >>> #include <linux/mutex.h> >>> #include <linux/sched/mm.h> >>> -#include <linux/hmm.h> >>> #include "kfd_priv.h" >>> #include "kfd_svm.h" >>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c >>> b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c >>> index 91609dd5730f..f041643308ca 100644 >>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c >>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c >>> @@ -1698,7 +1698,7 @@ static int svm_range_validate_and_map(struct >>> mm_struct *mm, >>> start = map_start << PAGE_SHIFT; >>> end = (map_last + 1) << PAGE_SHIFT; >>> for (addr = start; !r && addr < end; ) { >>> - struct hmm_range *hmm_range = NULL; >>> + struct amdgpu_hmm_range *range = NULL; >>> unsigned long map_start_vma; >>> unsigned long map_last_vma; >>> struct vm_area_struct *vma; >>> @@ -1737,13 +1737,13 @@ static int svm_range_validate_and_map(struct >>> mm_struct *mm, >>> } >>> WRITE_ONCE(p->svms.faulting_task, current); >>> - hmm_range = amdgpu_hmm_range_alloc(); >>> + range = amdgpu_hmm_range_alloc(NULL); >>> r = amdgpu_hmm_range_get_pages(&prange->notifier, addr, >>> npages, >>> readonly, owner, >>> - hmm_range); >>> + range); >>> WRITE_ONCE(p->svms.faulting_task, NULL); >>> if (r) { >>> - amdgpu_hmm_range_free(hmm_range); >>> + amdgpu_hmm_range_free(range); >>> pr_debug("failed %d to get svm range pages\n", r); >>> } >>> } else { >>> @@ -1753,7 +1753,7 @@ static int svm_range_validate_and_map(struct >>> mm_struct *mm, >>> if (!r) { >>> offset = (addr >> PAGE_SHIFT) - prange->start; >>> r = svm_range_dma_map(prange, ctx->bitmap, offset, npages, >>> - hmm_range->hmm_pfns); >>> + range->hmm_range.hmm_pfns); >>> if (r) >>> pr_debug("failed %d to dma map range\n", r); >>> } >>> @@ -1764,12 +1764,12 @@ static int svm_range_validate_and_map(struct >>> mm_struct *mm, >>> * Overrride return value to TRY AGAIN only if prior returns >>> * were successful >>> */ >>> - if (hmm_range && !amdgpu_hmm_range_valid(hmm_range) && !r) { >>> + if (range && !amdgpu_hmm_range_valid(range) && !r) { >>> pr_debug("hmm update the range, need validate again\n"); >>> r = -EAGAIN; >>> } >>> /* Free the hmm range */ >>> - amdgpu_hmm_range_free(hmm_range); >>> + amdgpu_hmm_range_free(range); >>> if (!r && !list_empty(&prange->child_list)) { >>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h >>> b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h >>> index 01c7a4877904..a63dfc95b602 100644 >>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h >>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h >>> @@ -31,7 +31,6 @@ >>> #include <linux/list.h> >>> #include <linux/mutex.h> >>> #include <linux/sched/mm.h> >>> -#include <linux/hmm.h> >>> #include "amdgpu.h" >>> #include "kfd_priv.h" >>>
