All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] drm/xe/guc: Fix race around q->guc->suspend_pending access
@ 2026-08-18 18:38 Jagmeet Randhawa
  2026-08-18 18:45 ` ✓ CI.KUnit: success for drm/xe/guc: Fix race around q->guc->suspend_pending access (rev2) Patchwork
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Jagmeet Randhawa @ 2026-08-18 18:38 UTC (permalink / raw)
  To: intel-xe; +Cc: niranjana.vishwanathapura, Jagmeet Randhawa

q->guc->suspend_pending is accessed without any common lock.
__suspend_fence_signal(), called from guc_exec_queue_kill() and the
suspend-timeout ban path, clears the flag asynchronously. Meanwhile
handle_sched_done(), guc_exec_queue_stop() and
__guc_exec_queue_process_msg_suspend() check the flag and then call
suspend_fence_signal(), which asserts that it is still set.

As the check and suspend_fence_signal() are not atomic, the clear can
land in between and trip the xe_gt_assert(q->guc->suspend_pending).

The flag is already set and read under the per-queue msg_lock
(xe_sched_msg_lock()) on the suspend and resume paths. Extend that same
lock to the clear paths (kill and ban) and to the three check-then-act
sites so the check and the signal are atomic with respect to the clear.
In __guc_exec_queue_process_msg_suspend() only the non-sleeping branch is
wrapped, since the other branch waits. In handle_sched_done() the flag is
snapshotted under the lock and deregister_exec_queue() is kept outside it.

v2: Document that sched->msg_lock also protects
    guc->suspend_pending, which indicates a suspend message is in
    flight, in addition to the sched->msgs list (Niranjana)

Signed-off-by: Jagmeet Randhawa <jagmeet.randhawa@intel.com>
---
 drivers/gpu/drm/xe/xe_gpu_scheduler_types.h  |  5 +++-
 drivers/gpu/drm/xe/xe_guc_exec_queue_types.h |  5 +++-
 drivers/gpu/drm/xe/xe_guc_submit.c           | 29 ++++++++++++++++----
 3 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_gpu_scheduler_types.h b/drivers/gpu/drm/xe/xe_gpu_scheduler_types.h
index 63d9bf92583c..78ef2e8ded4f 100644
--- a/drivers/gpu/drm/xe/xe_gpu_scheduler_types.h
+++ b/drivers/gpu/drm/xe/xe_gpu_scheduler_types.h
@@ -47,7 +47,10 @@ struct xe_gpu_scheduler {
 	const struct xe_sched_backend_ops	*ops;
 	/** @msgs: list of messages to be processed in @work_process_msg */
 	struct list_head			msgs;
-	/** @msg_lock: Message lock */
+	/**
+	 * @msg_lock: Protects @msgs and guc->suspend_pending (indicates a
+	 * suspend message is in flight) of exec queues on this scheduler.
+	 */
 	spinlock_t				msg_lock;
 	/** @work_process_msg: processes messages */
 	struct work_struct		work_process_msg;
diff --git a/drivers/gpu/drm/xe/xe_guc_exec_queue_types.h b/drivers/gpu/drm/xe/xe_guc_exec_queue_types.h
index d27826b36649..74b711abe257 100644
--- a/drivers/gpu/drm/xe/xe_guc_exec_queue_types.h
+++ b/drivers/gpu/drm/xe/xe_guc_exec_queue_types.h
@@ -52,7 +52,10 @@ struct xe_guc_exec_queue {
 	u16 id;
 	/** @suspend_wait: wait queue used to wait on pending suspends */
 	wait_queue_head_t suspend_wait;
-	/** @suspend_pending: a suspend of the exec_queue is pending */
+	/**
+	 * @suspend_pending: a suspend of the exec_queue is pending.
+	 * Protected by @sched.msg_lock.
+	 */
 	bool suspend_pending;
 	/**
 	 * @suspend_count: Reference count of active suspend requests. The
diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
index 9036f89dff7d..c565c1d32d3a 100644
--- a/drivers/gpu/drm/xe/xe_guc_submit.c
+++ b/drivers/gpu/drm/xe/xe_guc_submit.c
@@ -1928,9 +1928,13 @@ static void __guc_exec_queue_process_msg_suspend(struct xe_sched_msg *msg)
 			set_exec_queue_suspended(q);
 			disable_scheduling(q, false);
 		}
-	} else if (q->guc->suspend_pending) {
-		set_exec_queue_suspended(q);
-		suspend_fence_signal(q);
+	} else {
+		xe_sched_msg_lock(&q->guc->sched);
+		if (q->guc->suspend_pending) {
+			set_exec_queue_suspended(q);
+			suspend_fence_signal(q);
+		}
+		xe_sched_msg_unlock(&q->guc->sched);
 	}
 }
 
@@ -2130,7 +2134,9 @@ static void guc_exec_queue_kill(struct xe_exec_queue *q)
 {
 	trace_xe_exec_queue_kill(q);
 	set_exec_queue_killed(q);
+	xe_sched_msg_lock(&q->guc->sched);
 	__suspend_fence_signal(q);
+	xe_sched_msg_unlock(&q->guc->sched);
 	xe_guc_exec_queue_trigger_cleanup(q);
 }
 
@@ -2392,11 +2398,15 @@ static void guc_exec_queue_suspend_timeout_ban(struct xe_exec_queue *q)
 	 */
 	if (xe_exec_queue_is_multi_queue(q)) {
 		set_exec_queue_group_banned(q);
+		xe_sched_msg_lock(&q->guc->sched);
 		__suspend_fence_signal(q);
+		xe_sched_msg_unlock(&q->guc->sched);
 		xe_guc_exec_queue_group_trigger_cleanup(q);
 	} else {
 		set_exec_queue_banned(q);
+		xe_sched_msg_lock(&q->guc->sched);
 		__suspend_fence_signal(q);
+		xe_sched_msg_unlock(&q->guc->sched);
 		xe_guc_exec_queue_trigger_cleanup(q);
 	}
 }
@@ -2614,10 +2624,12 @@ static void guc_exec_queue_stop(struct xe_guc *guc, struct xe_exec_queue *q)
 		if (exec_queue_destroyed(q))
 			do_destroy = true;
 	}
+	xe_sched_msg_lock(sched);
 	if (q->guc->suspend_pending) {
 		set_exec_queue_suspended(q);
 		suspend_fence_signal(q);
 	}
+	xe_sched_msg_unlock(sched);
 	atomic_and(EXEC_QUEUE_STATE_WEDGED | EXEC_QUEUE_STATE_BANNED |
 		   EXEC_QUEUE_STATE_KILLED | EXEC_QUEUE_STATE_DESTROYED |
 		   EXEC_QUEUE_STATE_SUSPENDED,
@@ -3222,13 +3234,20 @@ static void handle_sched_done(struct xe_guc *guc, struct xe_exec_queue *q,
 		smp_wmb();
 		wake_up_all(&guc->ct.wq);
 	} else {
+		bool was_pending;
+
 		xe_gt_assert(guc_to_gt(guc), runnable_state == 0);
 		xe_gt_assert(guc_to_gt(guc), exec_queue_pending_disable(q));
 
-		if (q->guc->suspend_pending) {
+		xe_sched_msg_lock(&q->guc->sched);
+		was_pending = q->guc->suspend_pending;
+		if (was_pending) {
 			clear_exec_queue_pending_disable(q);
 			suspend_fence_signal(q);
-		} else {
+		}
+		xe_sched_msg_unlock(&q->guc->sched);
+
+		if (!was_pending) {
 			if (exec_queue_banned(q)) {
 				smp_wmb();
 				wake_up_all(&guc->ct.wq);
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-08-19  3:51 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-18 18:38 [PATCH v2] drm/xe/guc: Fix race around q->guc->suspend_pending access Jagmeet Randhawa
2026-08-18 18:45 ` ✓ CI.KUnit: success for drm/xe/guc: Fix race around q->guc->suspend_pending access (rev2) Patchwork
2026-08-18 18:51 ` [PATCH v2] drm/xe/guc: Fix race around q->guc->suspend_pending access sashiko-bot
2026-08-18 20:54   ` Niranjana Vishwanathapura
2026-08-18 19:32 ` ✓ Xe.CI.BAT: success for drm/xe/guc: Fix race around q->guc->suspend_pending access (rev2) Patchwork
2026-08-18 20:53 ` [PATCH v2] drm/xe/guc: Fix race around q->guc->suspend_pending access Niranjana Vishwanathapura
2026-08-18 21:06   ` Matthew Brost
2026-08-18 21:05 ` Matthew Brost
2026-08-18 23:27   ` Niranjana Vishwanathapura
2026-08-19  1:01     ` Matthew Brost
2026-08-19  3:51       ` Niranjana Vishwanathapura
2026-08-18 22:08 ` ✓ Xe.CI.FULL: success for drm/xe/guc: Fix race around q->guc->suspend_pending access (rev2) Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.