Re: [Patch v4 1/4] drm/amdgpu/userq: dont check return value in amdgpu_userq_evict

["Khatri, Sunil" <sukhatri@amd.com>]

On 31-03-2026 05:12 pm, Christian König wrote:
>
> On 3/31/26 09:49, Sunil Khatri wrote:
>> In function amdgpu_userq_evict we do not need to check
>> for return values and print errors as we are already
>> print error in all the functions of amdgpu_userq_evict.
>>
>> a. amdgpu_userq_wait_for_signal: Could timeout and we print
>>     error message in the function already
>> b. amdgpu_userq_evict_all: We unmap all the queues here and
>>     in case of unmap failure we already print unmap error.
>>
>> Suggested-by: Christian König <christian.koenig@amd.com>
>> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 26 +++++++++--------------
>>   1 file changed, 10 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> index fdae8c411aaa..79ee2f6e09da 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> @@ -1258,7 +1258,8 @@ amdgpu_userq_evict_all(struct amdgpu_userq_mgr *uq_mgr)
>>   	}
>>   
>>   	if (ret)
>> -		drm_file_err(uq_mgr->file, "Couldn't unmap all the queues\n");
>> +		drm_file_err(uq_mgr->file,
>> +			     "Couldn't unmap all the queues, eviction failed ret=%d\n", ret);
>>   	return ret;
>>   }
>>   
>> @@ -1289,13 +1290,14 @@ amdgpu_userq_wait_for_signal(struct amdgpu_userq_mgr *uq_mgr)
>>   	xa_for_each(&uq_mgr->userq_xa, queue_id, queue) {
>>   		struct dma_fence *f = queue->last_fence;
>>   
>> -		if (!f || dma_fence_is_signaled(f))
>> +		if (!f)
>>   			continue;
>>   
>> -		ret = dma_fence_wait_timeout(f, true, msecs_to_jiffies(100));
>> +		ret = dma_fence_wait(f, false);
>>   		if (ret <= 0) {
>> -			drm_file_err(uq_mgr->file, "Timed out waiting for fence=%llu:%llu\n",
>> -				     f->context, f->seqno);
>> +			drm_file_err(uq_mgr->file,
>> +				     "Timed out in wait_for_signal fence=%llu:%llu ret=%d\n",
>> +				     f->context, f->seqno, ret);
>>   
>>   			return -ETIMEDOUT;
>>   		}
> You can completely drop this. dma_fence_wait() will never return any error when used like this.
>
> Apart from that the patch looks correct to me.

Got it

Regards
Sunil

>
> Regards,
> Christian.
>
>
>> @@ -1307,18 +1309,10 @@ amdgpu_userq_wait_for_signal(struct amdgpu_userq_mgr *uq_mgr)
>>   void
>>   amdgpu_userq_evict(struct amdgpu_userq_mgr *uq_mgr)
>>   {
>> -	struct amdgpu_device *adev = uq_mgr->adev;
>> -	int ret;
>> -
>>   	/* Wait for any pending userqueue fence work to finish */
>> -	ret = amdgpu_userq_wait_for_signal(uq_mgr);
>> -	if (ret)
>> -		dev_err(adev->dev, "Not evicting userqueue, timeout waiting for work\n");
>> -
>> -	ret = amdgpu_userq_evict_all(uq_mgr);
>> -	if (ret)
>> -		dev_err(adev->dev, "Failed to evict userqueue\n");
>> -
>> +	amdgpu_userq_wait_for_signal(uq_mgr);
>> +	/* unmaps all the queues */
>> +	amdgpu_userq_evict_all(uq_mgr);
>>   }
>>   
>>   int amdgpu_userq_mgr_init(struct amdgpu_userq_mgr *userq_mgr, struct drm_file *file_priv,