On 3/26/26 14:23, 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 | 18 ++++++------------
> 1 file changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index aa0e6eea9436..6e6b1cae15ce 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -1258,7 +1258,7 @@ 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 queue,
> eviction failed\n");
I think it would be good to include ret in the error message.
> return ret;
> }
>
> @@ -1289,10 +1289,10 @@ 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_timeout(f, false, MAX_SCHEDULE_TIMEOUT);
With those parameters dma_fence_wait_timeout() can't fail any more, so you
don't really need to handle the return value.
There is also the dma_fence_wait() function as shortcut for that.
Regards,
Christian.
> if (ret <= 0) {
> drm_file_err(uq_mgr->file, "Timed out waiting for
> fence=%llu:%llu\n",
> f->context, f->seqno);
> @@ -1308,17 +1308,11 @@ 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,
