drm/amdgpu: fix hang and race in userq destroy

When a queue is hung, the hang_detect_work is the
only way to recover it. However in amdgpu_userq_destroy(),
the hang_detect_work is cancelled too early,
resulting in amdgpu_userq_wait_for_last_fence()
may never return, leaving an uninterruptible dma_fence_wait()
hang there.

To fix this problem, this commit moves the cancelling of
hang_detect_work after amdgpu_userq_wait_for_last_fence(), and it has
to be before the unmap helper, because hang_detect_work resets the
queue, so it races with amdgpu_userq_unmap_helper() for MES operations
and queue state.

This commit splits amdgpu_userq_cleanup() into two parts:

1) amdgpu_userq_detach_doorbell(), which detaches the queue from
userq_doorbell_xa. This has to be called before the cancel, otherwise
the IRQ handlers (for example amdgpu_userq_process_fence_irq)
can re-schedule the hang_detect_work and the cancel is not final.

2) amdgpu_userq_fence_driver_free(), this has to be called after the
unmap helper, because it can release the seq64 slot that the GPU
writes fence values to.

Only one cancel_delayed_work_sync(&queue->hang_detect_work) is needed,
so other redundancies are removed.

Signed-off-by: Zhu Lingshan <lingshan.zhu@amd.com>
Acked-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index 17cc48d..24adad7 100644 (file)
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -418,19 +418,12 @@ static void amdgpu_userq_wait_for_last_fence(struct amdgpu_usermode_queue *queue
        dma_fence_wait(f, false);
 }
 
-static void amdgpu_userq_cleanup(struct amdgpu_usermode_queue *queue)
+static void amdgpu_userq_detach_doorbell(struct amdgpu_usermode_queue *queue)
 {
-       struct amdgpu_userq_mgr *uq_mgr = queue->userq_mgr;
-       struct amdgpu_device *adev = uq_mgr->adev;
+       struct amdgpu_device *adev = queue->userq_mgr->adev;
 
-       /* Wait for mode-1 reset to complete */
        down_read(&adev->reset_domain->sem);
-
-       /* Use interrupt-safe locking since IRQ handlers may access these XArrays */
        xa_erase_irq(&adev->userq_doorbell_xa, queue->doorbell_index);
-       amdgpu_userq_fence_driver_free(queue);
-       queue->fence_drv = NULL;
-
        up_read(&adev->reset_domain->sem);
 }
 
@@ -551,18 +544,19 @@ amdgpu_userq_destroy(struct amdgpu_userq_mgr *uq_mgr, struct amdgpu_usermode_que
 
        cancel_delayed_work_sync(&uq_mgr->resume_work);
 
-       /* Cancel any pending hang detection work and cleanup */
-       cancel_delayed_work_sync(&queue->hang_detect_work);
-
        mutex_lock(&uq_mgr->userq_mutex);
        amdgpu_userq_wait_for_last_fence(queue);
 
+       amdgpu_userq_detach_doorbell(queue);
+       cancel_delayed_work_sync(&queue->hang_detect_work);
+
 #if defined(CONFIG_DEBUG_FS)
        debugfs_remove_recursive(queue->debugfs_queue);
 #endif
        r = amdgpu_userq_unmap_helper(queue);
        atomic_dec(&uq_mgr->userq_count[queue->queue_type]);
-       amdgpu_userq_cleanup(queue);
+       amdgpu_userq_fence_driver_free(queue);
+       queue->fence_drv = NULL;
        mutex_unlock(&uq_mgr->userq_mutex);
 
        /*
@@ -574,7 +568,6 @@ amdgpu_userq_destroy(struct amdgpu_userq_mgr *uq_mgr, struct amdgpu_usermode_que
        if (r)
                queue_work(adev->reset_domain->wq, &uq_mgr->reset_work);
 
-       cancel_delayed_work_sync(&queue->hang_detect_work);
        uq_funcs->mqd_destroy(queue);
        queue->userq_mgr = NULL;
 
@@ -748,7 +741,7 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
            ((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_cleanup().
+                * unresponsive during recovery), matching amdgpu_userq_detach_doorbell().
                 */
                down_read(&adev->reset_domain->sem);
                r = amdgpu_userq_map_helper(queue);