AMD General
Regards,
Prike
> -----Original Message-----
> From: Prosyak, Vitaly <[email protected]>
> Sent: Tuesday, September 29, 2026 6:49 AM
> To: Liang, Prike <[email protected]>; [email protected]
> Cc: Deucher, Alexander <[email protected]>; Koenig, Christian
> <[email protected]>; Prosyak, Vitaly <[email protected]>
> Subject: Re: [PATCH 12/18] drm/amdgpu: don't block wait gpu reset whthin userq
> lock
>
> Reviewed-by: Vitaly Prosyak <[email protected]>
>
> I have tested this patch on nv31. It successfully resolves the lockdep
> deadlock
> warning triggered during the reverse
>
> ordering of &reset_domain->sem and &userq_mgr->userq_mutex. Please note that
> this patch requires a rebase to apply
>
> cleanly onto the current target branch. Once rebased, the lock inversion
> issue is
> gone, and the trylock-and-retry logic
>
> works cleanly to unblock recovery execution path interactions. We can safely
> enable
> this on the CI.
>
Thank you for the review and verification, I will send out the new version with
a clean up and rebase.
> On 2026-09-02 08:49, Prike Liang wrote:
> > The offending edge was code that did a blocking
> > down_read(&adev->reset_domain->sem) as following, while holding
> > userq_mutex. Since GPU recovery takes reset_domain
> > ->sem for write and then transitively acquires userq_mutex,
> > the reverse ordering could deadlock.
> >
> > .569196]
> > other info that might help us debug this:
> >
> > [ 307.569516] Chain exists of:
> > &adev->firmware.mutex --> &userq_mgr->userq_mutex -->
> > &reset_domain->sem
> >
> > [ 307.570011] Possible unsafe locking scenario:
> >
> > [ 307.570250] CPU0 CPU1
> > [ 307.570438] ---- ----
> > [ 307.570624] lock(&reset_domain->sem);
> > [ 307.570785] lock(&userq_mgr->userq_mutex);
> > [ 307.571061] lock(&reset_domain->sem);
> > [ 307.571320] lock(&adev->firmware.mutex);
> > [ 307.571491]
> > *** DEADLOCK ***
> >
> > Signed-off-by: Prike Liang <[email protected]>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 51
> > ++++++++++++++++++++---
> > 1 file changed, 45 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > index 2534e4a1a530..a7d5ca741a3b 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > @@ -422,9 +422,12 @@ 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);
> > + /*
> > + * The caller serializes doorbell removal against an in-progress GPU
> > + * reset by holding adev->reset_domain->sem for read.
> > + */
> > + lockdep_assert_held_read(&adev->reset_domain->sem);
> > xa_erase_irq(&adev->userq_doorbell_xa, queue->doorbell_index);
> > - up_read(&adev->reset_domain->sem);
> > }
> >
> > /**
> > @@ -544,11 +547,34 @@ amdgpu_userq_destroy(struct amdgpu_userq_mgr
> > *uq_mgr, struct amdgpu_usermode_que
> >
> > cancel_delayed_work_sync(&uq_mgr->resume_work);
> >
> > + /*
> > + * Cancel hang detection before serializing against a GPU reset. Hang
> > + * detection triggers recovery, which takes reset_domain->sem for write,
> > + * so it must not be canceled while that semaphore is held for read.
> > + * A reset IRQ can restart hang detection, so this is repeated on retry.
> > + */
> > + cancel_delayed_work_sync(&queue->hang_detect_work);
> > +retry:
> > mutex_lock(&uq_mgr->userq_mutex);
> > amdgpu_userq_wait_for_last_fence(queue);
> >
> > + /*
> > + * Serialize queue teardown (doorbell detach and MES unmap) against an
> > + * in-progress GPU reset. Do not block on the reset semaphore while
> > + * holding userq_mutex: recovery takes the semaphore for write and then
> > + * (transitively) userq_mutex, so blocking here would invert that order
> > + * and deadlock. If the trylock fails, drop userq_mutex, wait for
> > + * recovery to finish, and retry.
> > + */
> > + if (!down_read_trylock(&adev->reset_domain->sem)) {
> > + mutex_unlock(&uq_mgr->userq_mutex);
> > +
> > + down_read(&adev->reset_domain->sem);
> > + up_read(&adev->reset_domain->sem);
> > + goto retry;
> > + }
> > +
> > amdgpu_userq_detach_doorbell(queue);
> > - cancel_delayed_work_sync(&queue->hang_detect_work);
> >
> > #if defined(CONFIG_DEBUG_FS)
> > debugfs_remove_recursive(queue->debugfs_queue);
> > @@ -557,6 +583,7 @@ amdgpu_userq_destroy(struct amdgpu_userq_mgr
> *uq_mgr, struct amdgpu_usermode_que
> > atomic_dec(&uq_mgr->userq_count[queue->queue_type]);
> > amdgpu_userq_fence_driver_free(queue);
> > queue->fence_drv = NULL;
> > + up_read(&adev->reset_domain->sem);
> > mutex_unlock(&uq_mgr->userq_mutex);
> >
> > /*
> > @@ -734,16 +761,28 @@ amdgpu_userq_create(struct drm_file *filp, union
> drm_amdgpu_userq *args)
> > if (r)
> > goto clean_mqd;
> >
> > +map_retry:
> > amdgpu_userq_ensure_ev_fence(&fpriv->userq_mgr, &fpriv->evf_mgr);
> >
> > /* don't map the queue if scheduling is halted */
> > if (!adev->userq_halt_for_enforce_isolation ||
> > ((queue->queue_type != AMDGPU_HW_IP_GFX) &&
> > (queue->queue_type != AMDGPU_HW_IP_COMPUTE))) {
> > - /* Serialize the map against an in-progress GPU reset (MES is
> > - * unresponsive during recovery), matching
> amdgpu_userq_detach_doorbell().
> > + /*
> > + * Serialize the map against an in-progress GPU reset (MES is
> > + * unresponsive during recovery). Do not block on the reset
> > + * semaphore while holding userq_mutex: recovery takes the
> > + * semaphore for write and then (transitively) userq_mutex, so
> > + * blocking here would invert that order and deadlock. If the
> > + * trylock fails, drop userq_mutex, wait for recovery, and
> > retry.
> > */
> > - down_read(&adev->reset_domain->sem);
> > + if (!down_read_trylock(&adev->reset_domain->sem)) {
> > + mutex_unlock(&uq_mgr->userq_mutex);
> > +
> > + down_read(&adev->reset_domain->sem);
> > + up_read(&adev->reset_domain->sem);
> > + goto map_retry;
> > + }
> > r = amdgpu_userq_map_helper(queue);
> > up_read(&adev->reset_domain->sem);
> > if (r) {
