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.

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) {

Reply via email to