[PATCH v6 1/8] drm/amdgpu/userq: Fix doorbell object cleanup of queue

[PATCH v6 1/8] drm/amdgpu/userq: Fix doorbell object cleanup of queue

[Sunil Khatri]

Unpin and unref the door bell obj if queue creation fails before
initialization is complete.

Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index 154742bdd5a6..eedea84c5e0f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -787,7 +787,7 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
 	r = uq_funcs->mqd_create(queue, &args->in);
 	if (r) {
 		drm_file_err(uq_mgr->file, "Failed to create Queue\n");
-		goto clean_mapping;
+		goto clean_doorbell_bo;
 	}
 
 	/* Update VM owner at userq submit-time for page-fault attribution. */
@@ -808,7 +808,7 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
 		if (r) {
 			drm_file_err(uq_mgr->file, "Failed to map Queue\n");
 			mutex_unlock(&uq_mgr->userq_mutex);
-			goto clean_doorbell;
+			goto erase_doorbell;
 		}
 	}
 
@@ -831,10 +831,15 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
 	args->out.queue_id = qid;
 	return 0;
 
-clean_doorbell:
+erase_doorbell:
 	xa_erase_irq(&adev->userq_doorbell_xa, index);
 clean_mqd:
 	uq_funcs->mqd_destroy(queue);
+clean_doorbell_bo:
+	amdgpu_bo_reserve(queue->db_obj.obj, true);
+	amdgpu_bo_unpin(queue->db_obj.obj);
+	amdgpu_bo_unreserve(queue->db_obj.obj);
+	amdgpu_bo_unref(&queue->db_obj.obj);
 clean_mapping:
 	amdgpu_bo_reserve(fpriv->vm.root.bo, true);
 	amdgpu_userq_buffer_vas_list_cleanup(adev, queue);
-- 
2.34.1

[PATCH v6 2/8] drm/amdgpu/userq: Fix the mutex_init cleanup for fence_drv_lock

[Sunil Khatri]

mutex fence_drv_lock is destroyed in amdgpu_userq_fence_driver_free
also in one of the jump condition mutex_destroy is also called leading
to double mutex_destroy.

So rearranging the code so amdgpu_userq_fence_driver_free takes care
of the clean up along with mutex_destroy.

Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index eedea84c5e0f..3bfb9ae2cb3a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -748,12 +748,12 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
 	INIT_DELAYED_WORK(&queue->hang_detect_work,
 			  amdgpu_userq_hang_detect_work);
 
-	mutex_init(&queue->fence_drv_lock);
-	xa_init_flags(&queue->fence_drv_xa, XA_FLAGS_ALLOC);
 	r = amdgpu_userq_fence_driver_alloc(adev, &queue->fence_drv);
 	if (r)
 		goto free_queue;
 
+	xa_init_flags(&queue->fence_drv_xa, XA_FLAGS_ALLOC);
+	mutex_init(&queue->fence_drv_lock);
 	/* Make sure the queue can actually run with those virtual addresses. */
 	r = amdgpu_bo_reserve(fpriv->vm.root.bo, false);
 	if (r)
@@ -844,7 +844,6 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
 	amdgpu_bo_reserve(fpriv->vm.root.bo, true);
 	amdgpu_userq_buffer_vas_list_cleanup(adev, queue);
 	amdgpu_bo_unreserve(fpriv->vm.root.bo);
-	mutex_destroy(&queue->fence_drv_lock);
 free_fence_drv:
 	amdgpu_userq_fence_driver_free(queue);
 free_queue:
-- 
2.34.1

[PATCH v6 3/8] drm/amdgpu: simplify return value in amdgpu_userq_get_doorbell_index

[Sunil Khatri]

amdgpu_userq_get_doorbell_index returns a uint64 type index
as well as a int type failure values. Simplifying this and
using a int type return value and getting the index in input pointer
of type uint64 type.

Also since it's used at once place making it static would be better.

Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 19 ++++++++++---------
 drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h |  4 ----
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index 3bfb9ae2cb3a..0737636fac43 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -536,12 +536,13 @@ void amdgpu_userq_destroy_object(struct amdgpu_userq_mgr *uq_mgr,
 	amdgpu_bo_unref(&userq_obj->obj);
 }
 
-uint64_t
+static int
 amdgpu_userq_get_doorbell_index(struct amdgpu_userq_mgr *uq_mgr,
 				struct amdgpu_db_info *db_info,
-				struct drm_file *filp)
+				struct drm_file *filp,
+				uint64_t *index)
 {
-	uint64_t index;
+	uint64_t doorbell_index;
 	struct drm_gem_object *gobj;
 	struct amdgpu_userq_obj *db_obj = db_info->db_obj;
 	int r, db_size;
@@ -588,12 +589,13 @@ amdgpu_userq_get_doorbell_index(struct amdgpu_userq_mgr *uq_mgr,
 		goto unpin_bo;
 	}
 
-	index = amdgpu_doorbell_index_on_bar(uq_mgr->adev, db_obj->obj,
+	doorbell_index = amdgpu_doorbell_index_on_bar(uq_mgr->adev, db_obj->obj,
 					     db_info->doorbell_offset, db_size);
 	drm_dbg_driver(adev_to_drm(uq_mgr->adev),
-		       "[Usermode queues] doorbell index=%lld\n", index);
+		       "[Usermode queues] doorbell index=%lld\n", doorbell_index);
 	amdgpu_bo_unreserve(db_obj->obj);
-	return index;
+	*index = doorbell_index;
+	return 0;
 
 unpin_bo:
 	amdgpu_bo_unpin(db_obj->obj);
@@ -776,10 +778,9 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
 	db_info.doorbell_handle = queue->doorbell_handle;
 	db_info.db_obj = &queue->db_obj;
 	db_info.doorbell_offset = args->in.doorbell_offset;
-	index = amdgpu_userq_get_doorbell_index(uq_mgr, &db_info, filp);
-	if (index == (uint64_t)-EINVAL) {
+	r = amdgpu_userq_get_doorbell_index(uq_mgr, &db_info, filp, &index);
+	if (r) {
 		drm_file_err(uq_mgr->file, "Failed to get doorbell for queue\n");
-		r = -EINVAL;
 		goto clean_mapping;
 	}
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
index 49b33e2d6932..033b8a0de6b1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
@@ -163,10 +163,6 @@ void 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);
 
-uint64_t amdgpu_userq_get_doorbell_index(struct amdgpu_userq_mgr *uq_mgr,
-					 struct amdgpu_db_info *db_info,
-					     struct drm_file *filp);
-
 u32 amdgpu_userq_get_supported_ip_mask(struct amdgpu_device *adev);
 bool amdgpu_userq_enabled(struct drm_device *dev);
 
-- 
2.34.1

[PATCH v6 4/8] drm/amdgpu/userq: clean up wptr_obj along with mqd_destroy

[Sunil Khatri]

During queue creation failure, when we clean up mqd via
mqd_destroy we arent doing the wptr_obj cleanup and hence
adding that clean up.

Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index 0737636fac43..47a38fefad89 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -835,6 +835,10 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
 erase_doorbell:
 	xa_erase_irq(&adev->userq_doorbell_xa, index);
 clean_mqd:
+	amdgpu_bo_reserve(queue->wptr_obj.obj, true);
+	amdgpu_bo_unpin(queue->wptr_obj.obj);
+	amdgpu_bo_unreserve(queue->wptr_obj.obj);
+	amdgpu_bo_unref(&queue->wptr_obj.obj);
 	uq_funcs->mqd_destroy(queue);
 clean_doorbell_bo:
 	amdgpu_bo_reserve(queue->db_obj.obj, true);
-- 
2.34.1

[PATCH v6 5/8] drm/amdgpu/userq: add amdgpu_bo_unpin when amdgpu_ttm_alloc_gart fails

[Sunil Khatri]

Unping the wptr_obj->obj when amdgpu_ttm_alloc_gart fails.

Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/mes_userqueue.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
index 14db2124ff81..2d95203ec58e 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
@@ -81,7 +81,7 @@ mes_userq_create_wptr_mapping(struct amdgpu_device *adev,
 	ret = amdgpu_ttm_alloc_gart(&wptr_obj->obj->tbo);
 	if (ret) {
 		DRM_ERROR("Failed to bind bo to GART. ret %d\n", ret);
-		goto fail_map;
+		goto fail_alloc_gart;
 	}
 
 	queue->wptr_obj.gpu_addr = amdgpu_bo_gpu_offset(wptr_obj->obj);
@@ -89,6 +89,8 @@ mes_userq_create_wptr_mapping(struct amdgpu_device *adev,
 	drm_exec_fini(&exec);
 	return 0;
 
+fail_alloc_gart:
+	amdgpu_bo_unpin(wptr_obj->obj);
 fail_map:
 	amdgpu_bo_unref(&wptr_obj->obj);
 fail_lock:
-- 
2.34.1

[PATCH v6 6/8] drm/amdgpu/userq: reserve root bo without interruption

[Sunil Khatri]

Fix the code to make it an uninterruptible reservation
for root bo.

Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index 47a38fefad89..c8f7bb23e2c3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -620,11 +620,7 @@ amdgpu_userq_destroy(struct amdgpu_userq_mgr *uq_mgr, struct amdgpu_usermode_que
 	/* Cancel any pending hang detection work and cleanup */
 	cancel_delayed_work_sync(&queue->hang_detect_work);
 
-	r = amdgpu_bo_reserve(vm->root.bo, false);
-	if (r) {
-		drm_file_err(uq_mgr->file, "Failed to reserve root bo during userqueue destroy\n");
-		return r;
-	}
+	amdgpu_bo_reserve(vm->root.bo, true);
 	amdgpu_userq_buffer_vas_list_cleanup(adev, queue);
 	amdgpu_bo_unreserve(vm->root.bo);
 
-- 
2.34.1

[PATCH v6 7/8] drm/amdgpu/userq: make sure queue is valid in the hang_detect_work

[Sunil Khatri]

Thread 1: Running amdgpu_userq_destroy which eventually remove
the queue from door bell and set userq_mgr = NULL.

Thread2: An interrupt might have scheduled the hang_detect_work
which still need userq_mgr to be valid but could get an NULL
ptrs.

To fix that make sure we cancel the hang_detect_work again before
setting userq_mgr to NULL.

Along with that we also need all the queue va to remain valid till
we could be running anything on the queue and hence moving the
userq_va post hang_detect handler is cancelled.

Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index c8f7bb23e2c3..7354c51ae83d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -427,8 +427,6 @@ static void amdgpu_userq_cleanup(struct amdgpu_usermode_queue *queue)
 	xa_erase_irq(&adev->userq_doorbell_xa, queue->doorbell_index);
 	amdgpu_userq_fence_driver_free(queue);
 	queue->fence_drv = NULL;
-	queue->userq_mgr = NULL;
-	list_del(&queue->userq_va_list);
 
 	up_read(&adev->reset_domain->sem);
 }
@@ -619,11 +617,6 @@ amdgpu_userq_destroy(struct amdgpu_userq_mgr *uq_mgr, struct amdgpu_usermode_que
 
 	/* Cancel any pending hang detection work and cleanup */
 	cancel_delayed_work_sync(&queue->hang_detect_work);
-
-	amdgpu_bo_reserve(vm->root.bo, true);
-	amdgpu_userq_buffer_vas_list_cleanup(adev, queue);
-	amdgpu_bo_unreserve(vm->root.bo);
-
 	mutex_lock(&uq_mgr->userq_mutex);
 	amdgpu_userq_wait_for_last_fence(queue);
 
@@ -635,6 +628,14 @@ amdgpu_userq_destroy(struct amdgpu_userq_mgr *uq_mgr, struct amdgpu_usermode_que
 	amdgpu_userq_cleanup(queue);
 	mutex_unlock(&uq_mgr->userq_mutex);
 
+	/* This is case an interrupt was fired and a hang detection work is pending */
+	cancel_delayed_work_sync(&queue->hang_detect_work);
+	amdgpu_bo_reserve(vm->root.bo, true);
+	amdgpu_userq_buffer_vas_list_cleanup(adev, queue);
+	amdgpu_bo_unreserve(vm->root.bo);
+	list_del(&queue->userq_va_list);
+	queue->userq_mgr = NULL;
+
 	amdgpu_bo_reserve(queue->db_obj.obj, true);
 	amdgpu_bo_unpin(queue->db_obj.obj);
 	amdgpu_bo_unreserve(queue->db_obj.obj);
-- 
2.34.1

[PATCH v6 8/8] drm/amdgpu/userq: user array to store userq vas

[Sunil Khatri]

Add per queue array to store userq vas and keep
size to accommodate vas of all types of queues
i.e gfx, compute and sdma.

Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 36 ++++++++---------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h |  4 ++-
 2 files changed, 16 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index 7354c51ae83d..9ac7f18c903f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -218,18 +218,11 @@ void amdgpu_userq_process_fence_irq(struct amdgpu_device *adev, u32 doorbell)
 static int amdgpu_userq_buffer_va_list_add(struct amdgpu_usermode_queue *queue,
 					   struct amdgpu_bo_va_mapping *va_map, u64 addr)
 {
-	struct amdgpu_userq_va_cursor *va_cursor;
-	struct userq_va_list;
-
-	va_cursor = kzalloc(sizeof(*va_cursor), GFP_KERNEL);
-	if (!va_cursor)
+	if (queue->userq_va_count >= ARRAY_SIZE(queue->userq_va))
 		return -ENOMEM;
 
-	INIT_LIST_HEAD(&va_cursor->list);
-	va_cursor->gpu_addr = addr;
+	queue->userq_va[queue->userq_va_count++] = addr;
 	va_map->bo_va->userq_va_mapped = true;
-	list_add(&va_cursor->list, &queue->userq_va_list);
-
 	return 0;
 }
 
@@ -284,14 +277,13 @@ static bool amdgpu_userq_buffer_va_mapped(struct amdgpu_vm *vm, u64 addr)
 
 static bool amdgpu_userq_buffer_vas_mapped(struct amdgpu_usermode_queue *queue)
 {
-	struct amdgpu_userq_va_cursor *va_cursor, *tmp;
-	int r = 0;
+	int i, r = 0;
 
-	list_for_each_entry_safe(va_cursor, tmp, &queue->userq_va_list, list) {
-		r += amdgpu_userq_buffer_va_mapped(queue->vm, va_cursor->gpu_addr);
+	for (i = 0; i < queue->userq_va_count; i++) {
+		r += amdgpu_userq_buffer_va_mapped(queue->vm, queue->userq_va[i]);
 		dev_dbg(queue->userq_mgr->adev->dev,
 			"validate the userq mapping:%p va:%llx r:%d\n",
-			queue, va_cursor->gpu_addr, r);
+			queue, queue->userq_va[i], r);
 	}
 
 	if (r != 0)
@@ -303,19 +295,19 @@ static bool amdgpu_userq_buffer_vas_mapped(struct amdgpu_usermode_queue *queue)
 static void amdgpu_userq_buffer_vas_list_cleanup(struct amdgpu_device *adev,
 						 struct amdgpu_usermode_queue *queue)
 {
-	struct amdgpu_userq_va_cursor *va_cursor, *tmp;
 	struct amdgpu_bo_va_mapping *mapping;
+	int i;
 
 	/* Caller must hold vm->root.bo reservation */
 	dma_resv_assert_held(queue->vm->root.bo->tbo.base.resv);
 
-	list_for_each_entry_safe(va_cursor, tmp, &queue->userq_va_list, list) {
-		mapping = amdgpu_vm_bo_lookup_mapping(queue->vm, va_cursor->gpu_addr);
-		if (mapping)
+	for (i = 0; i < queue->userq_va_count; i++) {
+		mapping = amdgpu_vm_bo_lookup_mapping(queue->vm, queue->userq_va[i]);
+		if (mapping) {
+			mapping->bo_va->userq_va_mapped = false;
 			dev_dbg(adev->dev, "delete the userq:%p va:%llx\n",
-				queue, va_cursor->gpu_addr);
-		list_del(&va_cursor->list);
-		kfree(va_cursor);
+				queue, queue->userq_va[i]);
+			}
 	}
 }
 
@@ -633,7 +625,6 @@ amdgpu_userq_destroy(struct amdgpu_userq_mgr *uq_mgr, struct amdgpu_usermode_que
 	amdgpu_bo_reserve(vm->root.bo, true);
 	amdgpu_userq_buffer_vas_list_cleanup(adev, queue);
 	amdgpu_bo_unreserve(vm->root.bo);
-	list_del(&queue->userq_va_list);
 	queue->userq_mgr = NULL;
 
 	amdgpu_bo_reserve(queue->db_obj.obj, true);
@@ -738,7 +729,6 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
 	}
 
 	kref_init(&queue->refcount);
-	INIT_LIST_HEAD(&queue->userq_va_list);
 	queue->doorbell_handle = args->in.doorbell_handle;
 	queue->queue_type = args->in.ip_type;
 	queue->vm = &fpriv->vm;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
index 033b8a0de6b1..fdf4d878c894 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
@@ -93,7 +93,9 @@ struct amdgpu_usermode_queue {
 	struct delayed_work	hang_detect_work;
 	struct kref		refcount;
 
-	struct list_head	userq_va_list;
+	/* User to store core bo's va addresses */
+	u64			userq_va[5];
+	int			userq_va_count;
 };
 
 struct amdgpu_userq_funcs {
-- 
2.34.1

Re: [PATCH v6 1/8] drm/amdgpu/userq: Fix doorbell object cleanup of queue

[Christian König]

On 5/19/26 13:17, Sunil Khatri wrote:
> Unpin and unref the door bell obj if queue creation fails before
> initialization is complete.
> 
> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index 154742bdd5a6..eedea84c5e0f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -787,7 +787,7 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
>  	r = uq_funcs->mqd_create(queue, &args->in);
>  	if (r) {
>  		drm_file_err(uq_mgr->file, "Failed to create Queue\n");
> -		goto clean_mapping;
> +		goto clean_doorbell_bo;
>  	}
>  
>  	/* Update VM owner at userq submit-time for page-fault attribution. */
> @@ -808,7 +808,7 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
>  		if (r) {
>  			drm_file_err(uq_mgr->file, "Failed to map Queue\n");
>  			mutex_unlock(&uq_mgr->userq_mutex);
> -			goto clean_doorbell;
> +			goto erase_doorbell;
>  		}
>  	}
>  
> @@ -831,10 +831,15 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
>  	args->out.queue_id = qid;
>  	return 0;
>  
> -clean_doorbell:
> +erase_doorbell:
>  	xa_erase_irq(&adev->userq_doorbell_xa, index);
>  clean_mqd:
>  	uq_funcs->mqd_destroy(queue);
> +clean_doorbell_bo:
> +	amdgpu_bo_reserve(queue->db_obj.obj, true);
> +	amdgpu_bo_unpin(queue->db_obj.obj);
> +	amdgpu_bo_unreserve(queue->db_obj.obj);
> +	amdgpu_bo_unref(&queue->db_obj.obj);
>  clean_mapping:
>  	amdgpu_bo_reserve(fpriv->vm.root.bo, true);
>  	amdgpu_userq_buffer_vas_list_cleanup(adev, queue);

Re: [PATCH v6 2/8] drm/amdgpu/userq: Fix the mutex_init cleanup for fence_drv_lock

[Christian König]

On 5/19/26 13:17, Sunil Khatri wrote:
> mutex fence_drv_lock is destroyed in amdgpu_userq_fence_driver_free
> also in one of the jump condition mutex_destroy is also called leading
> to double mutex_destroy.
> 
> So rearranging the code so amdgpu_userq_fence_driver_free takes care
> of the clean up along with mutex_destroy.

Please also move amdgpu_userq_fence_driver_free() into amdgpu_userq.c or eventually completely drop it.

The cleanup done in there is actually on the queue and not the fence driver.

Regards,
Christian.

> 
> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index eedea84c5e0f..3bfb9ae2cb3a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -748,12 +748,12 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
>  	INIT_DELAYED_WORK(&queue->hang_detect_work,
>  			  amdgpu_userq_hang_detect_work);
>  
> -	mutex_init(&queue->fence_drv_lock);
> -	xa_init_flags(&queue->fence_drv_xa, XA_FLAGS_ALLOC);
>  	r = amdgpu_userq_fence_driver_alloc(adev, &queue->fence_drv);
>  	if (r)
>  		goto free_queue;
>  
> +	xa_init_flags(&queue->fence_drv_xa, XA_FLAGS_ALLOC);
> +	mutex_init(&queue->fence_drv_lock);
>  	/* Make sure the queue can actually run with those virtual addresses. */
>  	r = amdgpu_bo_reserve(fpriv->vm.root.bo, false);
>  	if (r)
> @@ -844,7 +844,6 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
>  	amdgpu_bo_reserve(fpriv->vm.root.bo, true);
>  	amdgpu_userq_buffer_vas_list_cleanup(adev, queue);
>  	amdgpu_bo_unreserve(fpriv->vm.root.bo);
> -	mutex_destroy(&queue->fence_drv_lock);
>  free_fence_drv:
>  	amdgpu_userq_fence_driver_free(queue);
>  free_queue:

Re: [PATCH v6 3/8] drm/amdgpu: simplify return value in amdgpu_userq_get_doorbell_index

[Christian König]

On 5/19/26 13:17, Sunil Khatri wrote:
> amdgpu_userq_get_doorbell_index returns a uint64 type index
> as well as a int type failure values. Simplifying this and
> using a int type return value and getting the index in input pointer
> of type uint64 type.
> 
> Also since it's used at once place making it static would be better.
> 
> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 19 ++++++++++---------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h |  4 ----
>  2 files changed, 10 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index 3bfb9ae2cb3a..0737636fac43 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -536,12 +536,13 @@ void amdgpu_userq_destroy_object(struct amdgpu_userq_mgr *uq_mgr,
>  	amdgpu_bo_unref(&userq_obj->obj);
>  }
>  
> -uint64_t
> +static int
>  amdgpu_userq_get_doorbell_index(struct amdgpu_userq_mgr *uq_mgr,
>  				struct amdgpu_db_info *db_info,
> -				struct drm_file *filp)
> +				struct drm_file *filp,
> +				uint64_t *index)
>  {
> -	uint64_t index;
> +	uint64_t doorbell_index;
>  	struct drm_gem_object *gobj;
>  	struct amdgpu_userq_obj *db_obj = db_info->db_obj;
>  	int r, db_size;
> @@ -588,12 +589,13 @@ amdgpu_userq_get_doorbell_index(struct amdgpu_userq_mgr *uq_mgr,
>  		goto unpin_bo;
>  	}
>  
> -	index = amdgpu_doorbell_index_on_bar(uq_mgr->adev, db_obj->obj,
> +	doorbell_index = amdgpu_doorbell_index_on_bar(uq_mgr->adev, db_obj->obj,
>  					     db_info->doorbell_offset, db_size);
>  	drm_dbg_driver(adev_to_drm(uq_mgr->adev),
> -		       "[Usermode queues] doorbell index=%lld\n", index);
> +		       "[Usermode queues] doorbell index=%lld\n", doorbell_index);
>  	amdgpu_bo_unreserve(db_obj->obj);
> -	return index;
> +	*index = doorbell_index;
> +	return 0;
>  
>  unpin_bo:
>  	amdgpu_bo_unpin(db_obj->obj);
> @@ -776,10 +778,9 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
>  	db_info.doorbell_handle = queue->doorbell_handle;
>  	db_info.db_obj = &queue->db_obj;
>  	db_info.doorbell_offset = args->in.doorbell_offset;
> -	index = amdgpu_userq_get_doorbell_index(uq_mgr, &db_info, filp);
> -	if (index == (uint64_t)-EINVAL) {
> +	r = amdgpu_userq_get_doorbell_index(uq_mgr, &db_info, filp, &index);
> +	if (r) {
>  		drm_file_err(uq_mgr->file, "Failed to get doorbell for queue\n");
> -		r = -EINVAL;
>  		goto clean_mapping;
>  	}
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> index 49b33e2d6932..033b8a0de6b1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> @@ -163,10 +163,6 @@ void 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);
>  
> -uint64_t amdgpu_userq_get_doorbell_index(struct amdgpu_userq_mgr *uq_mgr,
> -					 struct amdgpu_db_info *db_info,
> -					     struct drm_file *filp);
> -
>  u32 amdgpu_userq_get_supported_ip_mask(struct amdgpu_device *adev);
>  bool amdgpu_userq_enabled(struct drm_device *dev);
>  

Re: [PATCH v6 4/8] drm/amdgpu/userq: clean up wptr_obj along with mqd_destroy

[Christian König]

On 5/19/26 13:17, Sunil Khatri wrote:
> During queue creation failure, when we clean up mqd via
> mqd_destroy we arent doing the wptr_obj cleanup and hence
> adding that clean up.
> 
> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index 0737636fac43..47a38fefad89 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -835,6 +835,10 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
>  erase_doorbell:
>  	xa_erase_irq(&adev->userq_doorbell_xa, index);
>  clean_mqd:
> +	amdgpu_bo_reserve(queue->wptr_obj.obj, true);
> +	amdgpu_bo_unpin(queue->wptr_obj.obj);
> +	amdgpu_bo_unreserve(queue->wptr_obj.obj);
> +	amdgpu_bo_unref(&queue->wptr_obj.obj);

That should probably be a function in mes_userqueue.c instead of here.

Regards,
Christian.

>  	uq_funcs->mqd_destroy(queue);
>  clean_doorbell_bo:
>  	amdgpu_bo_reserve(queue->db_obj.obj, true);

Re: [PATCH v6 2/8] drm/amdgpu/userq: Fix the mutex_init cleanup for fence_drv_lock

[Khatri, Sunil]

On 19-05-2026 06:08 pm, Christian König wrote:
> On 5/19/26 13:17, Sunil Khatri wrote:
>> mutex fence_drv_lock is destroyed in amdgpu_userq_fence_driver_free
>> also in one of the jump condition mutex_destroy is also called leading
>> to double mutex_destroy.
>>
>> So rearranging the code so amdgpu_userq_fence_driver_free takes care
>> of the clean up along with mutex_destroy.
> Please also move amdgpu_userq_fence_driver_free() into amdgpu_userq.c or eventually completely drop it.
>
> The cleanup done in there is actually on the queue and not the fence driver

There is no clear demarcation here, we are doing 
amdgpu_userq_walk_and_drop_fence_drv and amdgpu_userq_fence_driver_put 
in the clean up function.  If it's ok we could pick that up later for code

organization as its mixed use case right now and all the function called 
from clean up also needs to be pulled in.

Regards
Sunil Khatri

>
> Regards,
> Christian.
>
>> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> index eedea84c5e0f..3bfb9ae2cb3a 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> @@ -748,12 +748,12 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
>>   	INIT_DELAYED_WORK(&queue->hang_detect_work,
>>   			  amdgpu_userq_hang_detect_work);
>>   
>> -	mutex_init(&queue->fence_drv_lock);
>> -	xa_init_flags(&queue->fence_drv_xa, XA_FLAGS_ALLOC);
>>   	r = amdgpu_userq_fence_driver_alloc(adev, &queue->fence_drv);
>>   	if (r)
>>   		goto free_queue;
>>   
>> +	xa_init_flags(&queue->fence_drv_xa, XA_FLAGS_ALLOC);
>> +	mutex_init(&queue->fence_drv_lock);
>>   	/* Make sure the queue can actually run with those virtual addresses. */
>>   	r = amdgpu_bo_reserve(fpriv->vm.root.bo, false);
>>   	if (r)
>> @@ -844,7 +844,6 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
>>   	amdgpu_bo_reserve(fpriv->vm.root.bo, true);
>>   	amdgpu_userq_buffer_vas_list_cleanup(adev, queue);
>>   	amdgpu_bo_unreserve(fpriv->vm.root.bo);
>> -	mutex_destroy(&queue->fence_drv_lock);
>>   free_fence_drv:
>>   	amdgpu_userq_fence_driver_free(queue);
>>   free_queue:

Re: [PATCH v6 4/8] drm/amdgpu/userq: clean up wptr_obj along with mqd_destroy

[Khatri, Sunil]

On 19-05-2026 06:13 pm, Christian König wrote:
>
> On 5/19/26 13:17, Sunil Khatri wrote:
>> During queue creation failure, when we clean up mqd via
>> mqd_destroy we arent doing the wptr_obj cleanup and hence
>> adding that clean up.
>>
>> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> index 0737636fac43..47a38fefad89 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> @@ -835,6 +835,10 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
>>   erase_doorbell:
>>   	xa_erase_irq(&adev->userq_doorbell_xa, index);
>>   clean_mqd:
>> +	amdgpu_bo_reserve(queue->wptr_obj.obj, true);
>> +	amdgpu_bo_unpin(queue->wptr_obj.obj);
>> +	amdgpu_bo_unreserve(queue->wptr_obj.obj);
>> +	amdgpu_bo_unref(&queue->wptr_obj.obj);
> That should probably be a function in mes_userqueue.c instead of here.

Yeah even i thought so about it, sure will move in MES layer in the new 
patch set

Regards
Sunil khatri

>
> Regards,
> Christian.
>
>>   	uq_funcs->mqd_destroy(queue);
>>   clean_doorbell_bo:
>>   	amdgpu_bo_reserve(queue->db_obj.obj, true);

Re: [PATCH v6 2/8] drm/amdgpu/userq: Fix the mutex_init cleanup for fence_drv_lock

[Christian König]

On 5/19/26 15:09, Khatri, Sunil wrote:
> 
> On 19-05-2026 06:08 pm, Christian König wrote:
>> On 5/19/26 13:17, Sunil Khatri wrote:
>>> mutex fence_drv_lock is destroyed in amdgpu_userq_fence_driver_free
>>> also in one of the jump condition mutex_destroy is also called leading
>>> to double mutex_destroy.
>>>
>>> So rearranging the code so amdgpu_userq_fence_driver_free takes care
>>> of the clean up along with mutex_destroy.
>> Please also move amdgpu_userq_fence_driver_free() into amdgpu_userq.c or eventually completely drop it.
>>
>> The cleanup done in there is actually on the queue and not the fence driver
> 
> There is no clear demarcation here, we are doing amdgpu_userq_walk_and_drop_fence_drv and amdgpu_userq_fence_driver_put in the clean up function.  If it's ok we could pick that up later for code
> 
> organization as its mixed use case right now and all the function called from clean up also needs to be pulled in.

Yeah all of that looks pretty mixed up. Maybe we should move the handling more into amdgpu_userq_fence.c, I don't really know what would be cleaner.

Anyway Reviewed-by: Christian König <christian.koenig@amd.com> for this patch at the moment since it is clearly fixing a bug.

Thanks,
Christian.

> 
> Regards
> Sunil Khatri
> 
>>
>> Regards,
>> Christian.
>>
>>> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 5 ++---
>>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>>> index eedea84c5e0f..3bfb9ae2cb3a 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>>> @@ -748,12 +748,12 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
>>>       INIT_DELAYED_WORK(&queue->hang_detect_work,
>>>                 amdgpu_userq_hang_detect_work);
>>>   -    mutex_init(&queue->fence_drv_lock);
>>> -    xa_init_flags(&queue->fence_drv_xa, XA_FLAGS_ALLOC);
>>>       r = amdgpu_userq_fence_driver_alloc(adev, &queue->fence_drv);
>>>       if (r)
>>>           goto free_queue;
>>>   +    xa_init_flags(&queue->fence_drv_xa, XA_FLAGS_ALLOC);
>>> +    mutex_init(&queue->fence_drv_lock);
>>>       /* Make sure the queue can actually run with those virtual addresses. */
>>>       r = amdgpu_bo_reserve(fpriv->vm.root.bo, false);
>>>       if (r)
>>> @@ -844,7 +844,6 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
>>>       amdgpu_bo_reserve(fpriv->vm.root.bo, true);
>>>       amdgpu_userq_buffer_vas_list_cleanup(adev, queue);
>>>       amdgpu_bo_unreserve(fpriv->vm.root.bo);
>>> -    mutex_destroy(&queue->fence_drv_lock);
>>>   free_fence_drv:
>>>       amdgpu_userq_fence_driver_free(queue);
>>>   free_queue:

Re: [PATCH v6 5/8] drm/amdgpu/userq: add amdgpu_bo_unpin when amdgpu_ttm_alloc_gart fails

[Christian König]

On 5/19/26 13:17, Sunil Khatri wrote:
> Unping the wptr_obj->obj when amdgpu_ttm_alloc_gart fails.
> 
> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/mes_userqueue.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> index 14db2124ff81..2d95203ec58e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> @@ -81,7 +81,7 @@ mes_userq_create_wptr_mapping(struct amdgpu_device *adev,
>  	ret = amdgpu_ttm_alloc_gart(&wptr_obj->obj->tbo);
>  	if (ret) {
>  		DRM_ERROR("Failed to bind bo to GART. ret %d\n", ret);
> -		goto fail_map;
> +		goto fail_alloc_gart;
>  	}
>  
>  	queue->wptr_obj.gpu_addr = amdgpu_bo_gpu_offset(wptr_obj->obj);
> @@ -89,6 +89,8 @@ mes_userq_create_wptr_mapping(struct amdgpu_device *adev,
>  	drm_exec_fini(&exec);
>  	return 0;
>  
> +fail_alloc_gart:
> +	amdgpu_bo_unpin(wptr_obj->obj);
>  fail_map:
>  	amdgpu_bo_unref(&wptr_obj->obj);
>  fail_lock:

Re: [PATCH v6 6/8] drm/amdgpu/userq: reserve root bo without interruption

[Christian König]

On 5/19/26 13:17, Sunil Khatri wrote:
> Fix the code to make it an uninterruptible reservation
> for root bo.
> 
> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index 47a38fefad89..c8f7bb23e2c3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -620,11 +620,7 @@ amdgpu_userq_destroy(struct amdgpu_userq_mgr *uq_mgr, struct amdgpu_usermode_que
>  	/* Cancel any pending hang detection work and cleanup */
>  	cancel_delayed_work_sync(&queue->hang_detect_work);
>  
> -	r = amdgpu_bo_reserve(vm->root.bo, false);
> -	if (r) {
> -		drm_file_err(uq_mgr->file, "Failed to reserve root bo during userqueue destroy\n");
> -		return r;
> -	}
> +	amdgpu_bo_reserve(vm->root.bo, true);
>  	amdgpu_userq_buffer_vas_list_cleanup(adev, queue);
>  	amdgpu_bo_unreserve(vm->root.bo);
>  

Re: [PATCH v6 7/8] drm/amdgpu/userq: make sure queue is valid in the hang_detect_work

[Christian König]

On 5/19/26 13:18, Sunil Khatri wrote:
> Thread 1: Running amdgpu_userq_destroy which eventually remove
> the queue from door bell and set userq_mgr = NULL.
> 
> Thread2: An interrupt might have scheduled the hang_detect_work
> which still need userq_mgr to be valid but could get an NULL
> ptrs.
> 
> To fix that make sure we cancel the hang_detect_work again before
> setting userq_mgr to NULL.
> 
> Along with that we also need all the queue va to remain valid till
> we could be running anything on the queue and hence moving the
> userq_va post hang_detect handler is cancelled.
> 
> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index c8f7bb23e2c3..7354c51ae83d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -427,8 +427,6 @@ static void amdgpu_userq_cleanup(struct amdgpu_usermode_queue *queue)
>  	xa_erase_irq(&adev->userq_doorbell_xa, queue->doorbell_index);
>  	amdgpu_userq_fence_driver_free(queue);
>  	queue->fence_drv = NULL;
> -	queue->userq_mgr = NULL;
> -	list_del(&queue->userq_va_list);
>  
>  	up_read(&adev->reset_domain->sem);
>  }
> @@ -619,11 +617,6 @@ amdgpu_userq_destroy(struct amdgpu_userq_mgr *uq_mgr, struct amdgpu_usermode_que
>  
>  	/* Cancel any pending hang detection work and cleanup */
>  	cancel_delayed_work_sync(&queue->hang_detect_work);
> -
> -	amdgpu_bo_reserve(vm->root.bo, true);
> -	amdgpu_userq_buffer_vas_list_cleanup(adev, queue);
> -	amdgpu_bo_unreserve(vm->root.bo);
> -
>  	mutex_lock(&uq_mgr->userq_mutex);
>  	amdgpu_userq_wait_for_last_fence(queue);
>  
> @@ -635,6 +628,14 @@ amdgpu_userq_destroy(struct amdgpu_userq_mgr *uq_mgr, struct amdgpu_usermode_que
>  	amdgpu_userq_cleanup(queue);
>  	mutex_unlock(&uq_mgr->userq_mutex);
>  
> +	/* This is case an interrupt was fired and a hang detection work is pending */

Typo in comment, but I think you can drop it complete. Comments should explain why and not what.

Apart from that Reviewed-by: Christian König <christian.koenig@amd.com>

Regards,
Christian.

> +	cancel_delayed_work_sync(&queue->hang_detect_work);
> +	amdgpu_bo_reserve(vm->root.bo, true);
> +	amdgpu_userq_buffer_vas_list_cleanup(adev, queue);
> +	amdgpu_bo_unreserve(vm->root.bo);
> +	list_del(&queue->userq_va_list);
> +	queue->userq_mgr = NULL;
> +
>  	amdgpu_bo_reserve(queue->db_obj.obj, true);
>  	amdgpu_bo_unpin(queue->db_obj.obj);
>  	amdgpu_bo_unreserve(queue->db_obj.obj);

Re: [PATCH v6 8/8] drm/amdgpu/userq: user array to store userq vas

[Christian König]

On 5/19/26 13:18, Sunil Khatri wrote:
> Add per queue array to store userq vas and keep
> size to accommodate vas of all types of queues
> i.e gfx, compute and sdma.

Yeah that's a start, but I think we should go a bit further.

> 
> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 36 ++++++++---------------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h |  4 ++-
>  2 files changed, 16 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index 7354c51ae83d..9ac7f18c903f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -218,18 +218,11 @@ void amdgpu_userq_process_fence_irq(struct amdgpu_device *adev, u32 doorbell)
>  static int amdgpu_userq_buffer_va_list_add(struct amdgpu_usermode_queue *queue,
>  					   struct amdgpu_bo_va_mapping *va_map, u64 addr)
>  {

Completely nuke that function and it's caller.

> -	struct amdgpu_userq_va_cursor *va_cursor;
> -	struct userq_va_list;
> -
> -	va_cursor = kzalloc(sizeof(*va_cursor), GFP_KERNEL);
> -	if (!va_cursor)
> +	if (queue->userq_va_count >= ARRAY_SIZE(queue->userq_va))
>  		return -ENOMEM;
>  
> -	INIT_LIST_HEAD(&va_cursor->list);
> -	va_cursor->gpu_addr = addr;
> +	queue->userq_va[queue->userq_va_count++] = addr;
>  	va_map->bo_va->userq_va_mapped = true;
> -	list_add(&va_cursor->list, &queue->userq_va_list);
> -
>  	return 0;
>  }
>  
> @@ -284,14 +277,13 @@ static bool amdgpu_userq_buffer_va_mapped(struct amdgpu_vm *vm, u64 addr)
>  
>  static bool amdgpu_userq_buffer_vas_mapped(struct amdgpu_usermode_queue *queue)
>  {
> -	struct amdgpu_userq_va_cursor *va_cursor, *tmp;
> -	int r = 0;
> +	int i, r = 0;
>  
> -	list_for_each_entry_safe(va_cursor, tmp, &queue->userq_va_list, list) {
> -		r += amdgpu_userq_buffer_va_mapped(queue->vm, va_cursor->gpu_addr);
> +	for (i = 0; i < queue->userq_va_count; i++) {
> +		r += amdgpu_userq_buffer_va_mapped(queue->vm, queue->userq_va[i]);
>  		dev_dbg(queue->userq_mgr->adev->dev,
>  			"validate the userq mapping:%p va:%llx r:%d\n",
> -			queue, va_cursor->gpu_addr, r);
> +			queue, queue->userq_va[i], r);
>  	}
>  
>  	if (r != 0)
> @@ -303,19 +295,19 @@ static bool amdgpu_userq_buffer_vas_mapped(struct amdgpu_usermode_queue *queue)
>  static void amdgpu_userq_buffer_vas_list_cleanup(struct amdgpu_device *adev,
>  						 struct amdgpu_usermode_queue *queue)

Drop that whole function.

>  {
> -	struct amdgpu_userq_va_cursor *va_cursor, *tmp;
>  	struct amdgpu_bo_va_mapping *mapping;
> +	int i;
>  
>  	/* Caller must hold vm->root.bo reservation */
>  	dma_resv_assert_held(queue->vm->root.bo->tbo.base.resv);
>  
> -	list_for_each_entry_safe(va_cursor, tmp, &queue->userq_va_list, list) {
> -		mapping = amdgpu_vm_bo_lookup_mapping(queue->vm, va_cursor->gpu_addr);
> -		if (mapping)
> +	for (i = 0; i < queue->userq_va_count; i++) {
> +		mapping = amdgpu_vm_bo_lookup_mapping(queue->vm, queue->userq_va[i]);
> +		if (mapping) {
> +			mapping->bo_va->userq_va_mapped = false;
>  			dev_dbg(adev->dev, "delete the userq:%p va:%llx\n",
> -				queue, va_cursor->gpu_addr);
> -		list_del(&va_cursor->list);
> -		kfree(va_cursor);
> +				queue, queue->userq_va[i]);
> +			}
>  	}
>  }
>  
> @@ -633,7 +625,6 @@ amdgpu_userq_destroy(struct amdgpu_userq_mgr *uq_mgr, struct amdgpu_usermode_que
>  	amdgpu_bo_reserve(vm->root.bo, true);
>  	amdgpu_userq_buffer_vas_list_cleanup(adev, queue);
>  	amdgpu_bo_unreserve(vm->root.bo);
> -	list_del(&queue->userq_va_list);
>  	queue->userq_mgr = NULL;
>  
>  	amdgpu_bo_reserve(queue->db_obj.obj, true);
> @@ -738,7 +729,6 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
>  	}
>  
>  	kref_init(&queue->refcount);
> -	INIT_LIST_HEAD(&queue->userq_va_list);
>  	queue->doorbell_handle = args->in.doorbell_handle;
>  	queue->queue_type = args->in.ip_type;
>  	queue->vm = &fpriv->vm;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> index 033b8a0de6b1..fdf4d878c894 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> @@ -93,7 +93,9 @@ struct amdgpu_usermode_queue {
>  	struct delayed_work	hang_detect_work;
>  	struct kref		refcount;
>  
> -	struct list_head	userq_va_list;
> +	/* User to store core bo's va addresses */
> +	u64			userq_va[5];
> +	int			userq_va_count;

Make that look like this:

union {
	struct {
		u64 queue_rb;
		u64 wptr;
		u64 rptr;
		....
	} va_names;
	u64 va_array[];
};

Unused entries should simply be zero.

After amdgpu_userq_create() fills in the different VAs we just call amdgpu_userq_buffer_vas_mapped() to double check that they are valid before mapping the queue.

Regards,
Christian.

>  };
>  
>  struct amdgpu_userq_funcs {