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

["Christian König" <christian.koenig@amd.com>]

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.

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,