[PATCH 3/5] drm/amdgpu/userq: bound the eviction fence rearm retry loop
Junrui Luo via B4 Relay <[email protected]>
| Newsgroups | org.kernel.vger.linux-media,org.freedesktop.lists.amd-gfx,org.freedesktop.lists.dri-devel,org.kernel.feeds.b4-sent,org.kernel.vger.linux-kernel,org.kernel.vger.stable |
|---|---|
| Message-ID | <[email protected]> |
From: Junrui Luo <[email protected]> amdgpu_userq_ensure_ev_fence() loops until the eviction fence is both present and unsignaled. The only producer of such a fence is amdgpu_evf_mgr_rearm(), which runs as the very last step of amdgpu_userq_vm_validate(). Every failure point ahead of it - the kzalloc() in the rearm itself, amdgpu_hmm_range_alloc(), the ttm_bo_validate() calls, the GART binding of the wptr BOs - makes amdgpu_userq_restore_worker() give up with only a drm_file_err(). Nothing propagates that back, so the waiting thread reschedules the worker and flushes it again, forever. Both flush_delayed_work() and mutex_lock() sleep in TASK_UNINTERRUPTIBLE, so the looping task cannot be killed and the OOM killer cannot reclaim it. An unprivileged render node client reaches this from both AMDGPU_USERQ and AMDGPU_USERQ_SIGNAL. The eviction fence sequence number is already bumped by every successful rearm, so use it as the loop's progress condition: if a completed flush of the restore worker did not move it then no rearm happened and retrying cannot help. Return -ENOMEM in that case and let both callers report it to userspace. Fixes: a242a3e4b5be ("drm/amdgpu: simplify eviction fence suspend/resume") Reported-by: Yuhao Jiang <[email protected]> Assisted-by: Claude:claude-opus-5 Cc: [email protected] Signed-off-by: Junrui Luo <[email protected]> --- drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 21 +++++++++++++++++++-- drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h | 4 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 10 +++++++++- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c index bec107216811..208b53ae5bd1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c @@ -448,12 +448,16 @@ static void amdgpu_userq_cleanup(struct amdgpu_usermode_queue *queue) * Ensures that a valid and not yet signaled eviction fence is attached to the * usermode queue before any queue operations proceed. If it is signalled, then * rearm a new eviction fence. + * + * Returns 0 with @uq_mgr->userq_mutex held, or -ENOMEM with the mutex released + * when the restore worker could not rearm the fence. */ -void +int amdgpu_userq_ensure_ev_fence(struct amdgpu_userq_mgr *uq_mgr, struct amdgpu_eviction_fence_mgr *evf_mgr) { struct dma_fence *ev_fence; + int seq, prev_seq = -1; retry: /* Flush any pending resume work to create ev_fence */ @@ -463,7 +467,16 @@ amdgpu_userq_ensure_ev_fence(struct amdgpu_userq_mgr *uq_mgr, ev_fence = amdgpu_evf_mgr_get_fence(evf_mgr); if (dma_fence_is_signaled(ev_fence)) { dma_fence_put(ev_fence); + seq = atomic_read(&evf_mgr->ev_fence_seq); mutex_unlock(&uq_mgr->userq_mutex); + /* + * The sequence number is only bumped by a successful rearm, so + * if the flush above ran the worker without moving it then the + * restore failed and looping again would never terminate. + */ + if (seq == prev_seq) + return -ENOMEM; + prev_seq = seq; /* * Looks like there was no pending resume work, * add one now to create a valid eviction fence @@ -472,6 +485,8 @@ amdgpu_userq_ensure_ev_fence(struct amdgpu_userq_mgr *uq_mgr, goto retry; } dma_fence_put(ev_fence); + + return 0; } @@ -747,7 +762,9 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args) if (r) goto clean_mqd; - amdgpu_userq_ensure_ev_fence(&fpriv->userq_mgr, &fpriv->evf_mgr); + r = amdgpu_userq_ensure_ev_fence(&fpriv->userq_mgr, &fpriv->evf_mgr); + if (r) + goto erase_doorbell; /* don't map the queue if scheduling is halted */ if (!adev->userq_halt_for_enforce_isolation || diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h index 6412a7f7b6ef..c35909bf7ceb 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h @@ -164,8 +164,8 @@ void amdgpu_userq_mgr_fini(struct amdgpu_userq_mgr *userq_mgr); void amdgpu_userq_evict(struct amdgpu_userq_mgr *uq_mgr); -void amdgpu_userq_ensure_ev_fence(struct amdgpu_userq_mgr *userq_mgr, - struct amdgpu_eviction_fence_mgr *evf_mgr); +int amdgpu_userq_ensure_ev_fence(struct amdgpu_userq_mgr *userq_mgr, + struct amdgpu_eviction_fence_mgr *evf_mgr); u32 amdgpu_userq_get_supported_ip_mask(struct amdgpu_device *adev); bool amdgpu_userq_enabled(struct drm_device *dev); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c index 7e80442ec3e5..1c287ce59736 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c @@ -523,7 +523,15 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data, goto put_queue; /* We are here means UQ is active, make sure the eviction fence is valid */ - amdgpu_userq_ensure_ev_fence(&fpriv->userq_mgr, &fpriv->evf_mgr); + r = amdgpu_userq_ensure_ev_fence(&fpriv->userq_mgr, &fpriv->evf_mgr); + if (r) { + /* The fence is not initialized yet, so unwind it by hand */ + amdgpu_userq_fence_put_fence_drv_array(fence); + amdgpu_userq_fence_driver_put(fence->fence_drv); + kvfree(fence->fence_drv_array); + kfree(fence); + goto put_queue; + } /* Create the new fence */ amdgpu_userq_fence_init(queue, fence, wptr); -- 2.51.2