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

[Felix Kuehling]

On 2023-01-16 17:12, Errabolu, Ramesh wrote:
> [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.

Done. Thanks for pointing this out.

Regards,
   Felix


>
>> +     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);