Re: [PATCH] amdgpu: amdgpu_userq_restore_all return errors on failed bo reservation

[Khatri, Sunil]

On 18-06-2026 06:12 pm, Christian König wrote:
On 6/18/26 12:51, Zhu Lingshan wrote:
In amdgpu_userq_restore_all(), when failed to reserve
a bo, it should return a meaningful error code other than
"false" that means SUCCESS, which is wrong.
The caller should not ignore the return code of
amdgpu_userq_restore_all as well

Signed-off-by: Zhu Lingshan<lingshan....@amd.com>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 10 ++++++----
  1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index 95b680fc88c5..8b14870afbf5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -894,9 +894,9 @@ amdgpu_userq_restore_all(struct amdgpu_userq_mgr *uq_mgr)
        unsigned long queue_id;
        int ret = 0, r;
  
-
-       if (amdgpu_bo_reserve(vm->root.bo, false))
-               return false;
+       r = amdgpu_bo_reserve(vm->root.bo, false);
+       if (r)
+               return r;
Good catch, but that amdgpu_bo_reserve() is called here is a bug in the first 
place.
I think i probably missed that. There is no return value check for 
amdgpu_userq_restore_all and this cant fail. We should wait 
uninterruptible here i.e change false->true and drop the if condition.

The call to amdgpu_userq_vm_validate() must be moved into 
amdgpu_userq_vm_validate(), right before we call drm_exec_fini() and the manual 
call to amdgpu_bo_reserve() here dropped.

Otherwise we have a small windows where we drop the BO locks before starting 
the queues which could make the VM invalid again and cause all kind of issues.

we need to maintain order of locking First reserve root bo and then take 
mutex else we had mutex deadlocks.... this is why we have added root bo 
locking first before taking mutex in next line.

Regards

Sunil Khatri


Regards,
Christian.

  
  	mutex_lock(&uq_mgr->userq_mutex);
        /* Resume all the queues for this process */
@@ -1133,7 +1133,9 @@ static void amdgpu_userq_restore_worker(struct 
work_struct *work)
                goto put_fence;
        }
  
-	amdgpu_userq_restore_all(uq_mgr);
+       ret = amdgpu_userq_restore_all(uq_mgr);
+       if (ret)
+               drm_file_err(uq_mgr->file, "Failed to restore user queues, 
ret=%d\n", ret);
  
  put_fence:
        dma_fence_put(ev_fence);