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