Re: [PATCH 5/9] drm/amdgpu: fix eviction fence and userq manager shutdown

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

On 2/2/26 23:37, Alex Deucher wrote:
> On Mon, Feb 2, 2026 at 7:51 AM Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>>
>> That is a really complicated dance and wasn't implemented fully correct.
> 
> I'd suggest elaborating on what was wrong with the previous flow.
> It's not entirely clear from the patch.
> 
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c            | 2 ++
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c | 8 +++++++-
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.h | 1 +
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c          | 7 +++++--
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h          | 1 +
>>  5 files changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> index 02abe85624a6..9cd44be45861 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> @@ -2976,6 +2976,8 @@ static int amdgpu_drm_release(struct inode *inode, struct file *filp)
>>
>>         if (fpriv && drm_dev_enter(dev, &idx)) {
>>                 amdgpu_evf_mgr_shutdown(&fpriv->evf_mgr);
>> +               amdgpu_userq_mgr_cancel_resume(&fpriv->userq_mgr);
>> +               amdgpu_evf_mgr_flush_suspend(&fpriv->evf_mgr);
>>                 amdgpu_userq_mgr_fini(&fpriv->userq_mgr);
>>                 amdgpu_evf_mgr_fini(&fpriv->evf_mgr);
>>                 drm_dev_exit(idx);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>> index 8fe9f91f9551..ef4da6f2e2a3 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>> @@ -146,13 +146,19 @@ void amdgpu_evf_mgr_init(struct amdgpu_eviction_fence_mgr *evf_mgr)
>>  void amdgpu_evf_mgr_shutdown(struct amdgpu_eviction_fence_mgr *evf_mgr)
>>  {
>>         evf_mgr->shutdown = true;
>> +       /* Make sure that the shutdown is visible to the suspend work */
>>         flush_work(&evf_mgr->suspend_work);
>>  }
>>
>> -void amdgpu_evf_mgr_fini(struct amdgpu_eviction_fence_mgr *evf_mgr)
>> +void amdgpu_evf_mgr_flush_suspend(struct amdgpu_eviction_fence_mgr *evf_mgr)
>>  {
>>         dma_fence_wait(rcu_dereference_protected(evf_mgr->ev_fence, true),
>>                        false);
>> +       /* Make sure that we are done with the last suspend work */
>>         flush_work(&evf_mgr->suspend_work);
> 
> Is it intended to flush the work here and in amdgpu_evf_mgr_shutdown()?

Yes, we need both. That's why I added the comments to explain why we have each.

Going to improve the commit message.

Christian.

> 
> Alex
> 
>> +}
>> +
>> +void amdgpu_evf_mgr_fini(struct amdgpu_eviction_fence_mgr *evf_mgr)
>> +{
>>         dma_fence_put(evf_mgr->ev_fence);
>>  }
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.h
>> index 527de3a23583..132a13a5dc1c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.h
>> @@ -66,6 +66,7 @@ void amdgpu_evf_mgr_detach_fence(struct amdgpu_eviction_fence_mgr *evf_mgr,
>>                                  struct amdgpu_bo *bo);
>>  void amdgpu_evf_mgr_init(struct amdgpu_eviction_fence_mgr *evf_mgr);
>>  void amdgpu_evf_mgr_shutdown(struct amdgpu_eviction_fence_mgr *evf_mgr);
>> +void amdgpu_evf_mgr_flush_suspend(struct amdgpu_eviction_fence_mgr *evf_mgr);
>>  void amdgpu_evf_mgr_fini(struct amdgpu_eviction_fence_mgr *evf_mgr);
>>
>>  #endif
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> index e007f10a6c1c..60e8a993557a 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> @@ -1350,13 +1350,16 @@ int amdgpu_userq_mgr_init(struct amdgpu_userq_mgr *userq_mgr, struct drm_file *f
>>         return 0;
>>  }
>>
>> +void amdgpu_userq_mgr_cancel_resume(struct amdgpu_userq_mgr *userq_mgr)
>> +{
>> +       cancel_delayed_work_sync(&userq_mgr->resume_work);
>> +}
>> +
>>  void amdgpu_userq_mgr_fini(struct amdgpu_userq_mgr *userq_mgr)
>>  {
>>         struct amdgpu_usermode_queue *queue;
>>         unsigned long queue_id;
>>
>> -       cancel_delayed_work_sync(&userq_mgr->resume_work);
>> -
>>         mutex_lock(&userq_mgr->userq_mutex);
>>         amdgpu_userq_detect_and_reset_queues(userq_mgr);
>>         xa_for_each(&userq_mgr->userq_xa, queue_id, queue) {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
>> index 095c09f3d96c..13f4e356728b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
>> @@ -117,6 +117,7 @@ int amdgpu_userq_ioctl(struct drm_device *dev, void *data, struct drm_file *filp
>>  int amdgpu_userq_mgr_init(struct amdgpu_userq_mgr *userq_mgr, struct drm_file *file_priv,
>>                           struct amdgpu_device *adev);
>>
>> +void amdgpu_userq_mgr_cancel_resume(struct amdgpu_userq_mgr *userq_mgr);
>>  void amdgpu_userq_mgr_fini(struct amdgpu_userq_mgr *userq_mgr);
>>
>>  int amdgpu_userq_create_object(struct amdgpu_userq_mgr *uq_mgr,
>> --
>> 2.43.0
>>