Sorry to be late to the party, but absolute clear NAK to that. The userq mutex is part of the eviction handling and that eviction handling in turn depends on resets to complete.
So it's absolutely forbidden to take the userq mutex inside the reset handler! That is like the third time I have to fix this. Regards, Christian. On 9/29/26 00:49, vitaly prosyak wrote: > 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) {
