[Public]
> From: Koenig, Christian <christian.koe...@amd.com>
> Sent: Thursday, June 5, 2025 9:38 PM
> To: Liang, Prike <prike.li...@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <alexander.deuc...@amd.com>; Lazar, Lijo
> <lijo.la...@amd.com>
> Subject: Re: [PATCH 3/9] drm/amdgpu: wait for the user objects done before
> destroying
>
> On 6/5/25 15:15, Liang, Prike wrote:
> > [Public]
> >
> >> -----Original Message-----
> >> From: Koenig, Christian <christian.koe...@amd.com>
> >> Sent: Monday, June 2, 2025 7:05 PM
> >> To: Liang, Prike <prike.li...@amd.com>; amd-gfx@lists.freedesktop.org
> >> Cc: Deucher, Alexander <alexander.deuc...@amd.com>; Lazar, Lijo
> >> <lijo.la...@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?
>
> What do you mean with attached fences? We need to wait for the userq fence
> before
> destroying the queue, but apart from that it shouldn't matter what fences are
> attached
> where.
[Prike] The userq destroy function already waits for its last fence using
amdgpu_userq_wait_for_last_fence().
I mean, is it necessary to also wait for the reservation fences of the userq's
MQD, doorbell, and WPTR buffer
objects to be signaled by using amdgpu_bo_sync_wait() before destroying such
BOs?
> Regards,
> Christian.
>
> >
> >> Regards,
> >> Christian.
> >>
> >>>
> >>> Signed-off-by: Prike Liang <prike.li...@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); }
> >
