[PATCH] drm/amdgpu: Ignore KFD eviction fences invalidating preemptible DMABuf imports

[Eric Huang]

Hi Christian,

What do you think about Felix's explanation?

Regards,
Eric

On 2023-04-13 09:28, Felix Kuehling wrote:
> Am 2023-04-13 um 07:35 schrieb Christian König:
>> Am 13.04.23 um 03:01 schrieb Felix Kuehling:
>>> Am 2023-04-12 um 18:25 schrieb Eric Huang:
>>>> It is to avoid redundant eviction for KFD's DMAbuf import
>>>> bo when dmaunmapping DMAbuf. The DMAbuf import bo has
>>>> been set as AMDGPU_PL_PREEMPT in KFD when mapping.
>>>>
>>>> Signed-off-by: Eric Huang <jinhuieric.huang at amd.com>
>>>
>>> Reviewed-by: Felix Kuehling <Felix.Kuehling at amd.com>
>>>
>>> I'd like to get an Acked-by from Christian as well before submitting 
>>> this.
>>
>> I have to admit that I only partially followed the internal 
>> discussion, but in general you need a *really* good explanation for 
>> this.
>>
>> E.g. add code comment and explain in the commit message extensively 
>> why this is needed and why there are no alternatives.
>
> OK. I'll give it a shot:
>
>    This code path is used among other things when invalidating DMABuf
>    imports. These imports share a reservation object with the exported
>    BO. Waiting on all the fences in this reservation will trigger KFD
>    eviction fences unnecessarily, for example when a DMABuf import for
>    a DMA mapping on a secondary GPU is being unmapped explicitly. Only
>    moving the original exported BO requires stopping KFD user mode
>    queues. If the invalidation is triggered through a move notifier
>    from the exported BO, then moving the original BO already triggered
>    the eviction fence and we don't need to wait for it again on the 
> import.
>
>    We can identify DMABuf imports in KFD for secondary GPU DMA mappings
>    by the mem_type AMDGPU_PL_PREEMPT. In this case, use a wait
>    operation that ignores KFD eviction fences.
>
> How does this sound?
>
> Regards,
>   Felix
>
>
>>
>> Regards,
>> Christian.
>>
>>>
>>> Thanks,
>>>   Felix
>>>
>>>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 7 ++++++-
>>>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>> index 2430f3e9f3a7..64795fe9eecb 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>> @@ -526,7 +526,12 @@ static int amdgpu_bo_move(struct 
>>>> ttm_buffer_object *bo, bool evict,
>>>>       if ((old_mem->mem_type == TTM_PL_TT ||
>>>>            old_mem->mem_type == AMDGPU_PL_PREEMPT) &&
>>>>           new_mem->mem_type == TTM_PL_SYSTEM) {
>>>> -        r = ttm_bo_wait_ctx(bo, ctx);
>>>> +        if (old_mem->mem_type == AMDGPU_PL_PREEMPT)
>>>> +            r = amdgpu_bo_sync_wait(abo,
>>>> +                        AMDGPU_FENCE_OWNER_KFD,
>>>> +                        ctx->interruptible);
>>>> +        else
>>>> +            r = ttm_bo_wait_ctx(bo, ctx);
>>>>           if (r)
>>>>               return r;
>>