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

[Christian König]

Am 25.04.23 um 16:11 schrieb Eric Huang:
> Hi Christian,
>
> What do you think about Felix's explanation?

That's unfortunately not something we can do here.

>
> 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?

To be honest like quite a bad idea. Why in the world are imported BOs 
moved from GTT to SYSTEM in the first place?

The only reason for this I can think of is that the DMA mappings become 
invalid for some reasons and in this case waiting for the KFD fence is 
actually the absolutely right thing to do.

Regards,
Christian.

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