Re: [PATCH] drm/amdgpu: fix hang and race in userq destroy

On Fri, Aug 14, 2026 at 6:55 AM Zhu Lingshan <[email protected]> wrote:
>
> 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 <[email protected]>

Acked-by: Alex Deucher <[email protected]>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 23 ++++++++---------------
>  1 file changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index 17cc48d87c4d..24adad7be251 100644
> --- 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);
> --
> 2.53.0
>