[PATCH 3/9] drm/amdgpu: wait for the user objects done before destroying

[Liang, Prike]

[Public]

> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig at amd.com>
> Sent: Monday, June 2, 2025 7:05 PM
> To: Liang, Prike <Prike.Liang at amd.com>; amd-gfx at lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; Lazar, Lijo
> <Lijo.Lazar at amd.com>
> Subject: Re: [PATCH 3/9] drm/amdgpu: wait for the user objects done before
> destroying
>
> On 5/30/25 09:54, Prike Liang wrote:
> > The userq buffer should complete its attached work before being
> > destroyed.
>
> That doesn't make sense at all. We should wait for the requested signal fences, but
> apart from that it is the responsibility for userspace to make sure that queues are
> only destroyed after they are idle.
>
> If the kernel does this it should *never* wait for anything ongoing, it should only
> forcefully unmap the queue and destroy it.
[Prike] Yeah, generally, the userq destroy IOCTL request should be performed after Mesa tears the window context down by executing amdgpu_winsys_destroy().
But if there's an improper case where kernel tries to destroy the userq BOs attached fences somehow haven't signaled, then it's safe to force destroy the userq buffer object with ongoing work?

> Regards,
> Christian.
>
> >
> > Signed-off-by: Prike Liang <Prike.Liang at amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c  | 3 ++-
> > drivers/gpu/drm/amd/amdgpu/mes_userqueue.c | 3 +++
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > index 8eea0e1e1b6a..f45585bd5872 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > @@ -217,6 +217,8 @@ int amdgpu_userq_create_object(struct
> > amdgpu_userq_mgr *uq_mgr,  void amdgpu_userq_destroy_object(struct
> amdgpu_userq_mgr *uq_mgr,
> >                              struct amdgpu_userq_obj *userq_obj)  {
> > +   amdgpu_bo_sync_wait(userq_obj->obj,
> AMDGPU_FENCE_OWNER_UNDEFINED,
> > +                   false);
> >     amdgpu_bo_kunmap(userq_obj->obj);
> >     amdgpu_bo_unref(&userq_obj->obj);
> >  }
> > @@ -317,7 +319,6 @@ amdgpu_userq_destroy(struct drm_file *filp, int queue_id)
> >             amdgpu_bo_unpin(queue->db_obj.obj);
> >             amdgpu_bo_unreserve(queue->db_obj.obj);
> >     }
> > -   amdgpu_bo_unref(&queue->db_obj.obj);
> >     r = amdgpu_userq_unmap_helper(uq_mgr, queue);
> >     if (r != AMDGPU_USERQ_STATE_UNMAPPED) {
> >             drm_dbg_driver(adev_to_drm(uq_mgr->adev), "Can't unmap the
> queue
> > for destroying.\n"); diff --git
> > a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> > b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> > index 1457fb49a794..b46e67b179fc 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> > @@ -336,6 +336,9 @@ mes_userq_mqd_destroy(struct amdgpu_userq_mgr
> *uq_mgr,
> >                   struct amdgpu_usermode_queue *queue)  {
> >     amdgpu_userq_destroy_object(uq_mgr, &queue->fw_obj);
> > +   if (queue->db_obj.obj->tbo.pin_count)
> > +           amdgpu_bo_unpin(queue->db_obj.obj);
> > +   amdgpu_userq_destroy_object(uq_mgr, &queue->db_obj);
> >     kfree(queue->userq_prop);
> >     amdgpu_userq_destroy_object(uq_mgr, &queue->mqd);  }