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 <[email protected]>
> Signed-off-by: Sunil Khatri <[email protected]>
> ---
> 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,
