Re: [PATCH 1/9] drm/amdgpu: defer GPU recovery after userq eviction failure

["Christian König" <christian.koenig@amd.com>]

On 10/14/26 11:02, Liang, Prike wrote:
> AMD General
> 
> Regards,
>       Prike
> 
>> -----Original Message-----
>> From: Koenig, Christian <Christian.Koenig@amd.com>
>> Sent: Tuesday, October 13, 2026 5:26 PM
>> To: Liang, Prike <Prike.Liang@amd.com>; amd-gfx@lists.freedesktop.org
>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Lazar, Lijo
>> <Lijo.Lazar@amd.com>; Prosyak, Vitaly <Vitaly.Prosyak@amd.com>
>> Subject: Re: [PATCH 1/9] drm/amdgpu: defer GPU recovery after userq eviction
>> failure
>>
>> 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.
> 
> You're right, the current lock ordering userq mutex-> reset semaphore in the userq eviction signaling path does not need to change,
> as it matches the order declared in amdgpu_lockdep_init(). On further check, the VCN/UVD IB test may perform a memory
> allocation with GFP_KERNEL that triggers reclaim during GPU recovery. We may need a follow-up patch to address the resulting potential deadlock.

Yeah that is a really good catch. That needs to use GFP_ATOMIC so that we can use the emergency reserves.

That should probably be ok since the memory is immediately freed up again.

Regards,
Christian.

> 
> Regards,
> Prike
> 
> Regards,
>> Christian.
>>
>>>
>>> Fixes: c8ed2de0f2ee ("drm/amdgpu: rework userq reset work handling")
>>> Signed-off-by: Prike Liang <Prike.Liang@amd.com>
>>> ---
>>>  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);
>