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

[Khatri, Sunil]

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.koe...@amd.com>
Signed-off-by: Sunil Khatri <sunil.kha...@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,