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.
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.
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"
>
