Re: [PATCH 3/8] drm/amdgpu: overwrite signaled fence in amdgpu_sync

["Christian König" <ckoenig.leichtzumerken@gmail.com>]

Am 12.03.25 um 16:06 schrieb SRINIVASAN SHANMUGAM:
>
> On 3/7/2025 7:18 PM, Christian König wrote:
>> This allows using amdgpu_sync even without peeking into the fences for a
>> long time.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 13 +++++++++----
>>   1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
>> index 86c17a8946f5..bfe12164d27d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
>> @@ -135,11 +135,16 @@ static bool amdgpu_sync_add_later(struct amdgpu_sync *sync, struct dma_fence *f)
>>       struct amdgpu_sync_entry *e;
>>         hash_for_each_possible(sync->fences, e, node, f->context) {
>> -        if (unlikely(e->fence->context != f->context))
>> -            continue;
>> +        if (dma_fence_is_signaled(e->fence)) {
>> +            dma_fence_put(e->fence);
>> +            e->fence = dma_fence_get(f);
>> +            return true;
>> +        }
>>   -        amdgpu_sync_keep_later(&e->fence, f);
>> -        return true;
>> +        if (likely(e->fence->context == f->context)) {
>> +            amdgpu_sync_keep_later(&e->fence, f);
> --> The call to amdgpu_sync_keep_later(&e->fence, f); ensures that the new fence is tracked for future synchronization., ie., so If the driver only replaced the old fence without keeping a reference to the new one of the next job or second job for example, it could lead to situations where the synchronization state is lost. This could cause race conditions where one job might proceed before another job has completed, leading to errors. , so this is  " amdgpu_sync_keep_later(&e->fence, f);" done, for tracking purpose of next job/second job, if we have multiple jobs in gang submissions of same context/client, is my understanding is correct here pls?

Your questions is not easy to understand, but I think your understanding is correct.

Basically if you have submissions A,B,C to the same ring buffer and the HW guarantees that they execute in order you only need to keep a reference to C to wait for A and B as well.

Regards,
Christian.

>> +            return true;
>> +        }
>>       }
>>       return false;
>>   }