[PATCH 6/6] drm/amdgpu: Do bo_va ref counting for KFD BOs

[Errabolu, Ramesh]

[AMD Official Use Only - General]

Comment inline.

Regards,
Ramesh

-----Original Message-----
From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of Chen, Xiaogang
Sent: Saturday, January 14, 2023 4:28 AM
To: Kuehling, Felix <Felix.Kuehling at amd.com>; amd-gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org
Cc: Koenig, Christian <Christian.Koenig at amd.com>
Subject: Re: [PATCH 6/6] drm/amdgpu: Do bo_va ref counting for KFD BOs

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


Reviewed-by: Xiaogang Chen <Xiaoganng.Chen at amd.com>

Regards

Xiaogang

On 1/11/2023 7:31 PM, Felix Kuehling wrote:
> This is needed to correctly handle BOs imported into the GEM API, 
> which would otherwise get added twice to the same VM.
>
> Signed-off-by: Felix Kuehling <Felix.Kuehling at amd.com>
> ---
>   .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  | 28 +++++++++++++++----
>   1 file changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index df08e84f01d7..8b5ba2e04a79 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -370,9 +370,17 @@ static int amdgpu_amdkfd_bo_validate_and_fence(struct amdgpu_bo *bo,
>               return ret;
>
>       ret = amdgpu_amdkfd_bo_validate(bo, domain, true);
> -     if (!ret)
> -             dma_resv_add_fence(bo->tbo.base.resv, fence,
> -                                DMA_RESV_USAGE_BOOKKEEP);
> +     if (ret)
> +             goto unreserve_out;
> +
> +     ret = dma_resv_reserve_fences(bo->tbo.base.resv, 1);
Ramesh: Could this stmt to reserve space for fence be applied in patch 4.

> +     if (ret)
> +             goto unreserve_out;
> +
> +     dma_resv_add_fence(bo->tbo.base.resv, fence,
> +                        DMA_RESV_USAGE_BOOKKEEP);
> +
> +unreserve_out:
>       amdgpu_bo_unreserve(bo);
>
>       return ret;
> @@ -785,6 +793,7 @@ static int kfd_mem_attach(struct amdgpu_device *adev, struct kgd_mem *mem,
>       uint64_t va = mem->va;
>       struct kfd_mem_attachment *attachment[2] = {NULL, NULL};
>       struct amdgpu_bo *bo[2] = {NULL, NULL};
> +     struct amdgpu_bo_va *bo_va;
>       bool same_hive = false;
>       int i, ret;
>
> @@ -871,7 +880,12 @@ static int kfd_mem_attach(struct amdgpu_device *adev, struct kgd_mem *mem,
>                       pr_debug("Unable to reserve BO during memory attach");
>                       goto unwind;
>               }
> -             attachment[i]->bo_va = amdgpu_vm_bo_add(adev, vm, bo[i]);
> +             bo_va = amdgpu_vm_bo_find(vm, bo[i]);
> +             if (!bo_va)
> +                     bo_va = amdgpu_vm_bo_add(adev, vm, bo[i]);
> +             else
> +                     ++bo_va->ref_count;
> +             attachment[i]->bo_va = bo_va;
>               amdgpu_bo_unreserve(bo[i]);
>               if (unlikely(!attachment[i]->bo_va)) {
>                       ret = -ENOMEM;
> @@ -895,7 +909,8 @@ static int kfd_mem_attach(struct amdgpu_device *adev, struct kgd_mem *mem,
>                       continue;
>               if (attachment[i]->bo_va) {
>                       amdgpu_bo_reserve(bo[i], true);
> -                     amdgpu_vm_bo_del(adev, attachment[i]->bo_va);
> +                     if (--attachment[i]->bo_va->ref_count == 0)
> +                             amdgpu_vm_bo_del(adev, 
> + attachment[i]->bo_va);
>                       amdgpu_bo_unreserve(bo[i]);
>                       list_del(&attachment[i]->list);
>               }
> @@ -912,7 +927,8 @@ static void kfd_mem_detach(struct 
> kfd_mem_attachment *attachment)
>
>       pr_debug("\t remove VA 0x%llx in entry %p\n",
>                       attachment->va, attachment);
> -     amdgpu_vm_bo_del(attachment->adev, attachment->bo_va);
> +     if (--attachment->bo_va->ref_count == 0)
> +             amdgpu_vm_bo_del(attachment->adev, attachment->bo_va);
>       drm_gem_object_put(&bo->tbo.base);
>       list_del(&attachment->list);
>       kfree(attachment);