Re: [PATCH 2/2] drm/xe: Disable scheduling early on FD close to avoid CAT error cascade

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Concurrent execution of the eager KILL message and TDR paths causes a race condition in `disable_scheduling`, leading to a kernel crash via assertion failure.
--

commit d3677dc009f33b1e4d7baf0b32baa9087773e1dd
Author: Matthew Brost <matthew.brost@intel.com>

drm/xe: Disable scheduling early on FD close to avoid CAT error cascade

This commit mitigates CAT error cascades when an FD is closed with many
exec queues. It achieves this by eagerly sending a self-message to disable
scheduling outside of the slower, serialized TDR path.

> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index 5223f27825033..9b9279ab7930c 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
[ ... ]
> +static void __guc_exec_queue_process_msg_kill(struct xe_sched_msg *msg,
> +					      bool bound)
> +{
> +	struct xe_exec_queue *q = msg->private_data;
> +	struct xe_exec_queue *primary = xe_exec_queue_multi_queue_primary(q);
> +
> +	if (exec_queue_enabled(primary) && bound)
> +		disable_scheduling(primary, true);

[Severity: High]
Is it possible for this call to disable_scheduling() to race with the TDR path?

Calling disable_scheduling() from __guc_exec_queue_process_msg_kill() without
additional synchronization can race with the TDR's own call to
disable_scheduling(). 

If both paths execute concurrently, it will trigger the exclusive state
transition assertion in disable_scheduling() and crash the kernel:

	xe_gt_assert(guc_to_gt(guc), !exec_queue_pending_disable(q));

> +}
> +
[ ... ]
>  static void guc_exec_queue_kill(struct xe_exec_queue *q)
>  {
> +	struct xe_sched_msg *msg = q->guc->static_msgs + STATIC_MSG_KILL;
> +
>  	trace_xe_exec_queue_kill(q);
>  	set_exec_queue_killed(q);
>  	__suspend_fence_signal(q);
> +
> +	/*
> +	 * We eagerly send a message to ourselves to disable scheduling, as the
> +	 * TDR is serialized (i.e., only one exec queue is processed at a time).
> +	 * If an FD is closed with many exec queues, the TDR can be slower than
> +	 * the GuC scheduling contexts with invalid page tables, creating a
> +	 * cascade of CAT errors and engine resets, which is quite slow. Avoid
> +	 * this by immediately disabling scheduling outside of the TDR.
> +	 */
> +	if (kref_read(&q->refcount) && !exec_queue_wedged(q)) {
> +		struct xe_gpu_scheduler *sched = &q->guc->sched;
> +
> +		xe_sched_msg_lock(sched);
> +		guc_exec_queue_try_add_msg(q, msg, KILL);
> +		xe_sched_msg_unlock(sched);
> +	}
> +
>  	xe_guc_exec_queue_trigger_cleanup(q);

[Severity: High]
Could scheduling the KILL message alongside the TDR cleanup lead to concurrent
execution?

This sequence schedules the KILL message on the submit workqueue and then calls
xe_guc_exec_queue_trigger_cleanup() to schedule the TDR on the ordered
workqueue. 

Because of a potential data race, the KILL message's worker could be queued
after the TDR has called cancel_work_sync(), leading directly to the concurrent
calls to disable_scheduling() discussed above.

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901011712.2617466-1-matthew.brost@intel.com?part=2