Re: [PATCH 13/16] drm/xe/multi_queue: Support active group after primary is destroyed

[Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>]

On Tue, Nov 04, 2025 at 09:24:19AM -0800, Niranjana Vishwanathapura wrote:
>On Mon, Nov 03, 2025 at 02:05:53PM -0800, Matthew Brost wrote:
>>On Fri, Oct 31, 2025 at 11:29:33AM -0700, Niranjana Vishwanathapura wrote:
>>>Add support to keep the group active after the primary queue is
>>>destroyed. Instead of killing the primary queue during exec_queue
>>>destroy ioctl, kill it when all the secondary queues of the group
>>>are killed.
>>>
>>>Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
>>>---
>>> drivers/gpu/drm/xe/xe_device.c           |  7 ++-
>>> drivers/gpu/drm/xe/xe_exec_queue.c       | 55 +++++++++++++++++++++++-
>>> drivers/gpu/drm/xe/xe_exec_queue.h       |  2 +
>>> drivers/gpu/drm/xe/xe_exec_queue_types.h |  4 ++
>>> include/uapi/drm/xe_drm.h                |  5 +++
>>> 5 files changed, 70 insertions(+), 3 deletions(-)
>>>
>>>diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
>>>index 0b496676527a..708a17c357e6 100644
>>>--- a/drivers/gpu/drm/xe/xe_device.c
>>>+++ b/drivers/gpu/drm/xe/xe_device.c
>>>@@ -176,7 +176,12 @@ static void xe_file_close(struct drm_device *dev, struct drm_file *file)
>>> 	xa_for_each(&xef->exec_queue.xa, idx, q) {
>>> 		if (q->vm && q->hwe->hw_engine_group)
>>> 			xe_hw_engine_group_del_exec_queue(q->hwe->hw_engine_group, q);
>>>-		xe_exec_queue_kill(q);
>>>+
>>>+		if (xe_exec_queue_is_multi_queue_primary(q))
>>>+			xe_exec_queue_group_kill_put(q->multi_queue.group);
>>>+		else
>>>+			xe_exec_queue_kill(q);
>>>+
>>> 		xe_exec_queue_put(q);
>>> 	}
>>> 	xa_for_each(&xef->vm.xa, idx, vm)
>>>diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
>>>index 3c1bb4f10fd5..d7b0173691c1 100644
>>>--- a/drivers/gpu/drm/xe/xe_exec_queue.c
>>>+++ b/drivers/gpu/drm/xe/xe_exec_queue.c
>>>@@ -405,6 +405,26 @@ struct xe_exec_queue *xe_exec_queue_create_bind(struct xe_device *xe,
>>> }
>>> ALLOW_ERROR_INJECTION(xe_exec_queue_create_bind, ERRNO);
>>>
>>>+static void xe_exec_queue_group_kill(struct kref *ref)
>>>+{
>>>+	struct xe_exec_queue_group *group = container_of(ref, struct xe_exec_queue_group,
>>>+							 kill_refcount);
>>>+	xe_exec_queue_kill(group->primary);
>>>+}
>>>+
>>>+static inline void xe_exec_queue_group_kill_get(struct xe_exec_queue_group *group)
>>>+{
>>>+	kref_get(&group->kill_refcount);
>>>+}
>>>+
>>>+void xe_exec_queue_group_kill_put(struct xe_exec_queue_group *group)
>>>+{
>>>+	if (!group)
>>>+		return;
>>>+
>>>+	kref_put(&group->kill_refcount, xe_exec_queue_group_kill);
>>>+}
>>>+
>>> void xe_exec_queue_destroy(struct kref *ref)
>>> {
>>> 	struct xe_exec_queue *q = container_of(ref, struct xe_exec_queue, refcount);
>>>@@ -607,6 +627,7 @@ static int xe_exec_queue_group_init(struct xe_device *xe, struct xe_exec_queue *
>>> 	group->primary = q;
>>> 	group->cgp_bo = bo;
>>> 	INIT_LIST_HEAD(&group->list);
>>>+	kref_init(&group->kill_refcount);
>>> 	xa_init_flags(&group->xa, XA_FLAGS_ALLOC1);
>>> 	mutex_init(&group->lock);
>>> 	mutex_init(&group->list_lock);
>>>@@ -675,6 +696,11 @@ static int xe_exec_queue_group_add(struct xe_device *xe, struct xe_exec_queue *q
>>> 	q->multi_queue.pos = pos;
>>> 	mutex_unlock(&group->lock);
>>>
>>>+	if (group->primary->multi_queue.keep_active) {
>>>+		xe_exec_queue_group_kill_get(group);
>>>+		q->multi_queue.keep_active = true;
>>>+	}
>>>+
>>> 	return 0;
>>> }
>>>
>>>@@ -691,6 +717,11 @@ static void xe_exec_queue_group_delete(struct xe_exec_queue *q)
>>> 	if (lrc)
>>> 		xe_lrc_put(lrc);
>>> 	mutex_unlock(&group->lock);
>>>+
>>>+	if (q->multi_queue.keep_active) {
>>>+		xe_exec_queue_group_kill_put(group);
>>>+		q->multi_queue.keep_active = false;
>>>+	}
>>> }
>>>
>>> static int exec_queue_set_multi_group(struct xe_device *xe, struct xe_exec_queue *q,
>>>@@ -709,12 +740,24 @@ static int exec_queue_set_multi_group(struct xe_device *xe, struct xe_exec_queue
>>> 		return -EINVAL;
>>>
>>> 	if (value & DRM_XE_MULTI_GROUP_CREATE) {
>>>-		if (XE_IOCTL_DBG(xe, value & ~DRM_XE_MULTI_GROUP_CREATE))
>>>+		if (XE_IOCTL_DBG(xe, value & ~(DRM_XE_MULTI_GROUP_CREATE |
>>>+					       DRM_XE_MULTI_GROUP_KEEP_ACTIVE)))
>>>+			return -EINVAL;
>>>+
>>>+		/*
>>>+		 * KEEP_ACTIVE is not supported in preempt fence mode as in that mode,
>>>+		 * VM_DESTROY ioctl expects all exec queues of that VM are already killed.
>>>+		 */
>>>+		if (XE_IOCTL_DBG(xe, (value & DRM_XE_MULTI_GROUP_KEEP_ACTIVE) &&
>>>+				 xe_vm_in_preempt_fence_mode(q->vm)))
>>> 			return -EINVAL;
>>>
>>> 		q->multi_queue.valid = true;
>>> 		q->multi_queue.is_primary = true;
>>> 		q->multi_queue.pos = 0;
>>>+		if (value & DRM_XE_MULTI_GROUP_KEEP_ACTIVE)
>>>+			q->multi_queue.keep_active = true;
>>>+
>>> 		return 0;
>>> 	}
>>>
>>>@@ -1254,6 +1297,11 @@ void xe_exec_queue_kill(struct xe_exec_queue *q)
>>>
>>> 	q->ops->kill(q);
>>> 	xe_vm_remove_compute_exec_queue(q->vm, q);
>>>+
>>>+	if (!xe_exec_queue_is_multi_queue_primary(q) && q->multi_queue.keep_active) {
>>>+		xe_exec_queue_group_kill_put(q->multi_queue.group);
>>>+		q->multi_queue.keep_active = false;
>>>+	}
>>
>>This looks a little odd. Either you don't need to clear
>>multi_queue.keep_active as xe_exec_queue_kill can be called at most once
>>(IIRC it can be called multiple times) or you need some locking around
>>multi_queue.keep_active or it needs to be an atomic to prevent multiple
>>threads from calling xe_exec_queue_group_kill_put twice.
>>
>
>It looks like xe_exec_queue_kill() will only get called once.
>We really don't need to reset the keep_active flag here by setting it
>to 'false'. I just added that for completeness.
>Ok, Will remove.
>

Or may be just keep it as it looks more readable to me. ie., for the
secondary queues, we set keep_active while calling group_kill_get() and
we reset it while calling group_kill_put().

Niranjana

>Niranjana
>
>>Matt
>>
>>> }
>>>
>>> int xe_exec_queue_destroy_ioctl(struct drm_device *dev, void *data,
>>>@@ -1280,7 +1328,10 @@ int xe_exec_queue_destroy_ioctl(struct drm_device *dev, void *data,
>>> 	if (q->vm && q->hwe->hw_engine_group)
>>> 		xe_hw_engine_group_del_exec_queue(q->hwe->hw_engine_group, q);
>>>
>>>-	xe_exec_queue_kill(q);
>>>+	if (xe_exec_queue_is_multi_queue_primary(q))
>>>+		xe_exec_queue_group_kill_put(q->multi_queue.group);
>>>+	else
>>>+		xe_exec_queue_kill(q);
>>>
>>> 	trace_xe_exec_queue_close(q);
>>> 	xe_exec_queue_put(q);
>>>diff --git a/drivers/gpu/drm/xe/xe_exec_queue.h b/drivers/gpu/drm/xe/xe_exec_queue.h
>>>index 61478b2e883b..b642341f1ede 100644
>>>--- a/drivers/gpu/drm/xe/xe_exec_queue.h
>>>+++ b/drivers/gpu/drm/xe/xe_exec_queue.h
>>>@@ -109,6 +109,8 @@ static inline struct xe_exec_queue *xe_exec_queue_multi_queue_primary(struct xe_
>>> 	return xe_exec_queue_is_multi_queue(q) ? q->multi_queue.group->primary : q;
>>> }
>>>
>>>+void xe_exec_queue_group_kill_put(struct xe_exec_queue_group *group);
>>>+
>>> bool xe_exec_queue_is_lr(struct xe_exec_queue *q);
>>>
>>> bool xe_exec_queue_is_idle(struct xe_exec_queue *q);
>>>diff --git a/drivers/gpu/drm/xe/xe_exec_queue_types.h b/drivers/gpu/drm/xe/xe_exec_queue_types.h
>>>index e64b6588923e..cdca3afe838c 100644
>>>--- a/drivers/gpu/drm/xe/xe_exec_queue_types.h
>>>+++ b/drivers/gpu/drm/xe/xe_exec_queue_types.h
>>>@@ -55,6 +55,8 @@ struct xe_exec_queue_group {
>>> 	struct list_head list;
>>> 	/** @list_lock: Secondary queue list lock */
>>> 	struct mutex list_lock;
>>>+	/** @kill_refcount: ref count to kill primary queue */
>>>+	struct kref kill_refcount;
>>> 	/** @sync_pending: CGP_SYNC_DONE g2h response pending */
>>> 	bool sync_pending;
>>> };
>>>@@ -152,6 +154,8 @@ struct xe_exec_queue {
>>> 		u8 valid:1;
>>> 		/** @multi_queue.is_primary: Is primary queue (Q0) of the group */
>>> 		u8 is_primary:1;
>>>+		/** @multi_queue.keep_active: Keep the group active after primary is destroyed */
>>>+		u8 keep_active:1;
>>> 	} multi_queue;
>>>
>>> 	/** @sched_props: scheduling properties */
>>>diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
>>>index d72151163e77..333fb38b3404 100644
>>>--- a/include/uapi/drm/xe_drm.h
>>>+++ b/include/uapi/drm/xe_drm.h
>>>@@ -1260,6 +1260,10 @@ struct drm_xe_vm_bind {
>>>  *    then a new multi-queue group is created with this queue as the primary queue
>>>  *    (Q0). Otherwise, the queue gets added to the multi-queue group whose primary
>>>  *    queue id is specified in the 'value' field.
>>>+ *    If the extension's 'value' field has %DRM_XE_MULTI_GROUP_KEEP_ACTIVE flag
>>>+ *    set, then the multi-queue group is kept active after the primary queue is
>>>+ *    destroyed.
>>>+ *
>>>  *  - %DRM_XE_EXEC_QUEUE_SET_PROPERTY_MULTI_QUEUE_PRIORITY - Set the queue
>>>  *    priority within the multi-queue group.
>>>  *
>>>@@ -1304,6 +1308,7 @@ struct drm_xe_exec_queue_create {
>>> #define   DRM_XE_EXEC_QUEUE_SET_PROPERTY_PXP_TYPE		2
>>> #define   DRM_XE_EXEC_QUEUE_SET_PROPERTY_MULTI_GROUP		3
>>> #define     DRM_XE_MULTI_GROUP_CREATE				(1ull << 63)
>>>+#define     DRM_XE_MULTI_GROUP_KEEP_ACTIVE			(1ull << 62)
>>> #define   DRM_XE_EXEC_QUEUE_SET_PROPERTY_MULTI_QUEUE_PRIORITY	4
>>> 	/** @extensions: Pointer to the first extension struct, if any */
>>> 	__u64 extensions;
>>>--
>>>2.43.0
>>>