[PATCH V6 3/5] drm/amdgpu: use drm_file_err in fence timeouts

[Khatri, Sunil]

On 4/17/2025 7:11 PM, Tvrtko Ursulin wrote:
>
> On 17/04/2025 13:31, Sunil Khatri wrote:
>> use drm_file_err instead of DRM_ERROR which adds
>> process and pid information in the userqueue error
>> logging.
>>
>> Sample log:
>> [   42.444297] [drm:amdgpu_userqueue_wait_for_signal [amdgpu]] 
>> *ERROR* Timed out waiting for fence f=000000001c74d978 for 
>> comm:Xwayland pid:3427
>> [   42.444669] [drm:amdgpu_userqueue_suspend [amdgpu]] *ERROR* Not 
>> suspending userqueue, timeout waiting for comm:Xwayland pid:3427
>> [   42.824729] [drm:amdgpu_userqueue_wait_for_signal [amdgpu]] 
>> *ERROR* Timed out waiting for fence f=0000000074407d3e for 
>> comm:systemd-logind pid:1058
>> [   42.825082] [drm:amdgpu_userqueue_suspend [amdgpu]] *ERROR* Not 
>> suspending userqueue, timeout waiting for comm:systemd-logind pid:1058
>
> If you have some oomph left after this many revisions in a short time 
> it would be good to update the examples to latest format.

Will push the changes in new patch set

regards

Sunil

>> Signed-off-by: Sunil Khatri <sunil.khatri at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c | 21 ++++++++++++-------
>>   1 file changed, 13 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
>> index cd9dc0018ee6..613a3a63301b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
>> @@ -43,8 +43,9 @@ amdgpu_userqueue_cleanup(struct amdgpu_userq_mgr 
>> *uq_mgr,
>>       if (f && !dma_fence_is_signaled(f)) {
>>           ret = dma_fence_wait_timeout(f, true, msecs_to_jiffies(100));
>>           if (ret <= 0) {
>> -            DRM_ERROR("Timed out waiting for fence=%llu:%llu\n",
>> -                  f->context, f->seqno);
>> +            drm_file_err(uq_mgr->file,
>> +                     "Timed out waiting for fence: context=%llu 
>> seqno:%llu\n",
>> +                     f->context, f->seqno);
>
> I would just go "...fence=%llu:%llu" for consistency with tracepoints 
> but up to you.
>
>>               return;
>>           }
>>       }
>> @@ -456,7 +457,8 @@ amdgpu_userqueue_resume_all(struct 
>> amdgpu_userq_mgr *uq_mgr)
>>       }
>>         if (ret)
>> -        DRM_ERROR("Failed to map all the queues\n");
>> +        drm_file_err(uq_mgr->file, "Failed to map all the queues\n");
>> +
>>       return ret;
>>   }
>>   @@ -614,7 +616,8 @@ amdgpu_userqueue_suspend_all(struct 
>> amdgpu_userq_mgr *uq_mgr)
>>       }
>>         if (ret)
>> -        DRM_ERROR("Couldn't unmap all the queues\n");
>> +        drm_file_err(uq_mgr->file, "Couldn't unmap all the queues\n");
>> +
>>       return ret;
>>   }
>>   @@ -631,8 +634,10 @@ amdgpu_userqueue_wait_for_signal(struct 
>> amdgpu_userq_mgr *uq_mgr)
>>               continue;
>>           ret = dma_fence_wait_timeout(f, true, msecs_to_jiffies(100));
>>           if (ret <= 0) {
>> -            DRM_ERROR("Timed out waiting for fence=%llu:%llu\n",
>> -                  f->context, f->seqno);
>> +            drm_file_err(uq_mgr->file,
>> +                     "Timed out waiting for fence: context=%llu 
>> seqno:%llu\n",
>> +                     f->context, f->seqno);
>> +
>>               return -ETIMEDOUT;
>>           }
>>       }
>> @@ -651,13 +656,13 @@ amdgpu_userqueue_suspend(struct 
>> amdgpu_userq_mgr *uq_mgr,
>>       /* Wait for any pending userqueue fence work to finish */
>>       ret = amdgpu_userqueue_wait_for_signal(uq_mgr);
>>       if (ret) {
>> -        DRM_ERROR("Not suspending userqueue, timeout waiting for 
>> work\n");
>> +        drm_file_err(uq_mgr->file, "Not suspending userqueue, 
>> timeout waiting\n");
>
> ...work?
>
> Anyway, we can tidy it later.
>
> With or without fence=%llu:%llu:
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at igalia.com>
>
> Regards,
>
> Tvrtko
>
>>           return;
>>       }
>>         ret = amdgpu_userqueue_suspend_all(uq_mgr);
>>       if (ret) {
>> -        DRM_ERROR("Failed to evict userqueue\n");
>> +        drm_file_err(uq_mgr->file, "Failed to evict userqueue\n");
>>           return;
>>       }
>