<div dir="ltr"><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Mon, Apr 14, 2025 at 7:33 AM Christian König <<a href="mailto:christian.koenig@amd.com">christian.koenig@amd.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Am 12.04.25 um 10:03 schrieb Arunpravin Paneer Selvam: > Add queue id support to the user queue wait IOCTL > drm_amdgpu_userq_wait structure. > > This is required to retrieve the wait user queue and maintain > the fence driver references in it so that the user queue in > the same context releases their reference to the fence drivers > at some point before queue destruction. > > Otherwise, we would gather those references until we > don't have any more space left and crash. > > v2: Modify the UAPI comment as per the mesa and libdrm UAPI comment. > > Libdrm MR: <a href="https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/408" rel="noreferrer" target="_blank">https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/408</a> > Mesa MR: <a href="https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34493" rel="noreferrer" target="_blank">https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34493</a> > > Signed-off-by: Arunpravin Paneer Selvam <<a href="mailto:Arunpravin.PaneerSelvam@amd.com" target="_blank">Arunpravin.PaneerSelvam@amd.com</a>> > Suggested-by: Christian König <<a href="mailto:christian.koenig@amd.com" target="_blank">christian.koenig@amd.com</a>> Reviewed-by: Christian König <<a href="mailto:christian.koenig@amd.com" target="_blank">christian.koenig@amd.com</a>> </blockquote><div> </div><div>Didn't you notice the ABI breakage?</div><div> </div><div>Marek</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> > --- > .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 20 +++++++++++-------- > .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.h | 1 - > drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c | 1 - > include/uapi/drm/amdgpu_drm.h | 6 ++++++ > 4 files changed, 18 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c > index a4953d668972..83bb2737c92e 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c > @@ -97,7 +97,6 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev, > spin_lock_init(&fence_drv->fence_list_lock); > > fence_drv->adev = adev; > - fence_drv->fence_drv_xa_ptr = &userq->fence_drv_xa; > fence_drv->context = dma_fence_context_alloc(1); > get_task_comm(fence_drv->timeline_name, current); > > @@ -591,6 +590,9 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data, > u32 num_syncobj, num_read_bo_handles, num_write_bo_handles; > struct drm_amdgpu_userq_fence_info *fence_info = NULL; > struct drm_amdgpu_userq_wait *wait_info = data; > + struct amdgpu_fpriv *fpriv = filp->driver_priv; > + struct amdgpu_userq_mgr *userq_mgr = &fpriv->userq_mgr; > + struct amdgpu_usermode_queue *waitq; > struct drm_gem_object **gobj_write; > struct drm_gem_object **gobj_read; > struct dma_fence **fences = NULL; > @@ -840,6 +842,10 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data, > fences[num_fences++] = fence; > } > > + waitq = idr_find(&userq_mgr->userq_idr, wait_info->waitq_id); > + if (!waitq) > + goto free_fences; > + > for (i = 0, cnt = 0; i < num_fences; i++) { > struct amdgpu_userq_fence_driver *fence_drv; > struct amdgpu_userq_fence *userq_fence; > @@ -868,14 +874,12 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data, > * Otherwise, we would gather those references until we don't > * have any more space left and crash. > */ > - if (fence_drv->fence_drv_xa_ptr) { > - r = xa_alloc(fence_drv->fence_drv_xa_ptr, &index, fence_drv, > - xa_limit_32b, GFP_KERNEL); > - if (r) > - goto free_fences; > + r = xa_alloc(&waitq->fence_drv_xa, &index, fence_drv, > + xa_limit_32b, GFP_KERNEL); > + if (r) > + goto free_fences; > > - amdgpu_userq_fence_driver_get(fence_drv); > - } > + amdgpu_userq_fence_driver_get(fence_drv); > > /* Store drm syncobj's gpu va address and value */ > fence_info[cnt].va = fence_drv->va; > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h > index f0a91cc02880..d5090a6bcdde 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h > @@ -55,7 +55,6 @@ struct amdgpu_userq_fence_driver { > spinlock_t fence_list_lock; > struct list_head fences; > struct amdgpu_device *adev; > - struct xarray *fence_drv_xa_ptr; > char timeline_name[TASK_COMM_LEN]; > }; > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c > index ecd49cf15b2a..7c754ba56cff 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c > @@ -73,7 +73,6 @@ amdgpu_userqueue_cleanup(struct amdgpu_userq_mgr *uq_mgr, > } > > uq_funcs->mqd_destroy(uq_mgr, queue); > - queue->fence_drv->fence_drv_xa_ptr = NULL; > amdgpu_userq_fence_driver_free(queue); > idr_remove(&uq_mgr->userq_idr, queue_id); > kfree(queue); > diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h > index ef97c0d78b8a..2195810ae42d 100644 > --- a/include/uapi/drm/amdgpu_drm.h > +++ b/include/uapi/drm/amdgpu_drm.h > @@ -501,6 +501,12 @@ struct drm_amdgpu_userq_fence_info { > }; > > struct drm_amdgpu_userq_wait { > + /** > + * @waitq_id: Queue handle used by the userq wait IOCTL to retrieve the > + * wait queue and maintain the fence driver references in it. > + */ > + __u32 waitq_id; > + __u32 pad; > /** > * @syncobj_handles: The list of syncobj handles submitted by the user queue > * job to get the va/value pairs. </blockquote></div></div>