On 10/14/26 11:02, Liang, Prike wrote:
> AMD General
>
> Regards,
> Prike
>
>> -----Original Message-----
>> From: Koenig, Christian <[email protected]>
>> Sent: Tuesday, October 13, 2026 5:26 PM
>> To: Liang, Prike <[email protected]>; [email protected]
>> Cc: Deucher, Alexander <[email protected]>; Lazar, Lijo
>> <[email protected]>; Prosyak, Vitaly <[email protected]>
>> 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 <[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);
>
