AMD General
Regards,
Prike
> -----Original Message-----
> From: Koenig, Christian <[email protected]>
> Sent: Tuesday, October 13, 2026 5:31 PM
> To: Liang, Prike <[email protected]>; [email protected]
> Cc: Deucher, Alexander <[email protected]>; Lazar, Lijo
> <[email protected]>; Prosyak, Vitaly <[email protected]>
> Subject: Re: [PATCH 7/9] drm/amdgpu/userq: take reset_domain->sem before
> userq_mutex in destroy
>
> On 10/13/26 10:24, Prike Liang wrote:
> > amdgpu_userq_destroy() takes reset_domain->sem for read in
> > amdgpu_userq_detach_doorbell() while holding userq_mutex.
>
> Yeah that is correct behavior.
>
> > GPU recovery
> > holds reset_domain->sem for write and then takes clientlist_mutex in
> > drm_client_dev_suspend(),
>
> That is completely broken behavior and need to be fixed.
>
> The GPU reset can *NEVER* call drm_client_dev_suspend(), who came up with
> that nonsense?
The call to drm_client_dev_suspend() have been in the GPU recovery path for
some time, possibly leveraged incorrectly from the system suspend path. I’ll
clean it up in the GPU recovery path.
Regards,
Prike
> Regards,
> Christian.
>
> > which already depends on userq_mutex through the fbdev reservation and
> > userq restore paths. Lockdep reports:
> >
> > Chain exists of:
> > &dev->clientlist_mutex --> &userq_mgr->userq_mutex -->
> > &reset_domain->sem
> >
> > Keep waiting for the last fence under userq_mutex only, as the fence
> > of a hung queue is only force-completed by the reset, then take
> > reset_domain->sem before re-acquiring userq_mutex for the doorbell
> > detach, unmap and fence driver teardown. This also keeps the MES unmap
> > from racing with an in-progress GPU reset.
> >
> > Fixes: f18719ef4bb7 ("drm/amdgpu: Convert amdgpu userqueue management
> > from IDR to XArray")
> > Signed-off-by: Prike Liang <[email protected]>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 16 ++++++----------
> > 1 file changed, 6 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > index 5fef0cc7703c..a34ea8ecd700 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > @@ -415,15 +415,6 @@ static void amdgpu_userq_wait_for_last_fence(struct
> amdgpu_usermode_queue *queue
> > dma_fence_wait(f, false);
> > }
> >
> > -static void amdgpu_userq_detach_doorbell(struct amdgpu_usermode_queue
> > *queue) -{
> > - struct amdgpu_device *adev = queue->userq_mgr->adev;
> > -
> > - down_read(&adev->reset_domain->sem);
> > - xa_erase_irq(&adev->userq_doorbell_xa, queue->doorbell_index);
> > - up_read(&adev->reset_domain->sem);
> > -}
> > -
> > /**
> > * amdgpu_userq_ensure_ev_fence - ensure a valid, unsignaled eviction fence
> exists
> > * @uq_mgr: the usermode queue manager for this process @@ -547,8
> > +538,12 @@ amdgpu_userq_destroy(struct amdgpu_userq_mgr *uq_mgr,
> > struct amdgpu_usermode_que
> >
> > mutex_lock(&uq_mgr->userq_mutex);
> > amdgpu_userq_wait_for_last_fence(queue);
> > + mutex_unlock(&uq_mgr->userq_mutex);
> >
> > - amdgpu_userq_detach_doorbell(queue);
> > + /* GPU recovery takes userq_mutex under reset_domain->sem, keep that
> order */
> > + down_read(&adev->reset_domain->sem);
> > + mutex_lock(&uq_mgr->userq_mutex);
> > + xa_erase_irq(&adev->userq_doorbell_xa, queue->doorbell_index);
> >
> > #if defined(CONFIG_DEBUG_FS)
> > debugfs_remove_recursive(queue->debugfs_queue);
> > @@ -558,6 +553,7 @@ amdgpu_userq_destroy(struct amdgpu_userq_mgr
> *uq_mgr, struct amdgpu_usermode_que
> > amdgpu_userq_fence_driver_free(queue);
> > queue->fence_drv = NULL;
> > mutex_unlock(&uq_mgr->userq_mutex);
> > + up_read(&adev->reset_domain->sem);
> >
> > /*
> > * A failed unmap means MES could not remove the hung queue and is
> > now
