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 <[email protected]> > --- > 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. 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. 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);
