On 10/13/26 10:24, Prike Liang wrote:
> On userq eviction failure, the suspend worker waits for reset_work
> while holding userq_mutex and inside a DMA-fence signaling section.
> This makes eviction-fence completion depend on GPU recovery finishing,
> although recovery can itself require memory allocation and eviction.
Absolutely clear NAK to that!
GPU recovery is to unlock memory allocation and eviction, so it can *NEVER*
allocate memory nor depend on eviction to complete.
This patch here is just utterly nonsense. What we need to avoid is to touch the
userq mutex and/or any eviction fence from the GPU recovery.
Regards,
Christian.
>
> Fixes: c8ed2de0f2ee ("drm/amdgpu: rework userq reset work handling")
> Signed-off-by: Prike Liang <[email protected]>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c | 12 +++++++++---
> drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 10 +++-------
> drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h | 2 +-
> 3 files changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
> index 082d045edf45..c2d1bc7decba 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
> @@ -65,6 +65,7 @@ amdgpu_eviction_fence_suspend_worker(struct work_struct
> *work)
> struct amdgpu_userq_mgr *uq_mgr = &fpriv->userq_mgr;
> struct dma_fence *ev_fence;
> bool cookie;
> + int r;
>
> mutex_lock(&uq_mgr->userq_mutex);
>
> @@ -79,21 +80,26 @@ amdgpu_eviction_fence_suspend_worker(struct work_struct
> *work)
> cookie = dma_fence_begin_signalling();
>
> ev_fence = amdgpu_evf_mgr_get_fence(evf_mgr);
> - amdgpu_userq_evict(uq_mgr);
> + r = amdgpu_userq_evict(uq_mgr);
>
> /*
> * Signaling the eviction fence must be done while holding the
> * userq_mutex. Otherwise we won't resume the queues before issuing the
> * next fence.
> */
> - dma_fence_signal(ev_fence);
> + if (!r)
> + dma_fence_signal(ev_fence);
> dma_fence_end_signalling(cookie);
> dma_fence_put(ev_fence);
>
> - if (!evf_mgr->shutdown)
> + if (!r && !evf_mgr->shutdown)
> schedule_delayed_work(&uq_mgr->resume_work, 0);
>
> mutex_unlock(&uq_mgr->userq_mutex);
> +
> + if (r)
> + amdgpu_reset_domain_schedule(uq_mgr->adev->reset_domain,
> + &uq_mgr->reset_work);
> }
>
> int amdgpu_evf_mgr_attach_fence(struct amdgpu_eviction_fence_mgr *evf_mgr,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index 3fe91fdafe55..5fef0cc7703c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -1308,13 +1308,9 @@ amdgpu_userq_evict_all(struct amdgpu_userq_mgr *uq_mgr)
> ret = r;
> }
>
> - if (ret) {
> + if (ret)
> drm_file_err(uq_mgr->file,
> "Couldn't unmap all the queues, eviction failed
> ret=%d\n", ret);
> - amdgpu_reset_domain_schedule(uq_mgr->adev->reset_domain,
> - &uq_mgr->reset_work);
> - flush_work(&uq_mgr->reset_work);
> - }
> return ret;
> }
>
> @@ -1334,10 +1330,10 @@ amdgpu_userq_wait_for_signal(struct amdgpu_userq_mgr
> *uq_mgr)
> }
> }
>
> -void
> +int
> amdgpu_userq_evict(struct amdgpu_userq_mgr *uq_mgr)
> {
> - amdgpu_userq_evict_all(uq_mgr);
> + return amdgpu_userq_evict_all(uq_mgr);
> }
>
> int amdgpu_userq_mgr_init(struct amdgpu_userq_mgr *userq_mgr, struct
> drm_file *file_priv,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> index 488dc21d7c81..4dcf6151de6a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> @@ -163,7 +163,7 @@ void amdgpu_userq_mgr_cancel_resume(struct
> amdgpu_userq_mgr *userq_mgr);
> void amdgpu_userq_mgr_fini(struct amdgpu_userq_mgr *userq_mgr);
>
> void amdgpu_userq_wait_for_signal(struct amdgpu_userq_mgr *uq_mgr);
> -void amdgpu_userq_evict(struct amdgpu_userq_mgr *uq_mgr);
> +int 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);
