Re: [PATCH v6 2/3] drm/xe/guc: Protect timeout handling from device unplug

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/xe: user-space hang and job leak on unplug due to aborted TDR cleanup
--

commit 1871c63913c38876d863255c92ead2457412e562
Author: Satyanarayana K V P <satyanarayana.k.v.p@intel.com>

drm/xe/guc: Protect timeout handling from device unplug

This commit adds drm_dev_enter() and drm_dev_exit() guards to the GuC scheduler
timeout callback. If the device unplug has already started, it sets an -ENODEV
error on the timed-out job and returns early.

> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index 7ce73489c35fc..68fd9785a8da4 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -1599,6 +1599,12 @@ guc_exec_queue_timedout_job(struct drm_sched_job *drm_job)
>  	int err = -ETIME;
>  	pid_t pid = -1;
>  	bool wedged = false, wedge_device = false, skip_timeout_check;
> +	int idx;
> +
> +	if (!drm_dev_enter(&xe->drm, &idx)) {
> +		xe_sched_job_set_error(job, -ENODEV);
> +		return DRM_GPU_SCHED_STAT_NO_HANG;
> +	}

[Severity: High]
Does this early return skip setting the error on all other pending jobs in the
queue?

By returning early here, we skip the drm_sched_for_each_pending_job() loop
later in this function that sets -ECANCELED for all other pending jobs.

Later during hardware teardown, xe_gt_fini() calls xe_hw_fence_irq_finish()
to forcibly signal remaining hardware fences. However, xe_hw_fence_irq_finish()
only signals if xe_hw_fence_signaled() returns true:

drivers/gpu/drm/xe/xe_hw_fence.c:xe_hw_fence_signaled() {
    ...
	return dma_fence->error ||
		!__dma_fence_is_later(dma_fence, dma_fence->seqno, seqno);
}

Because the device is unplugged, MMIO reads of the sequence number typically
return 0xFFFFFFFF, making the chronological check evaluate to false for
pending jobs.

Without an error set on the fence from the skipped loop, xe_hw_fence_signaled()
evaluates to false.

This could leave the fences permanently orphaned in xe_hw_fence_irq_finish():

drivers/gpu/drm/xe/xe_hw_fence.c:xe_hw_fence_irq_finish() {
    ...
		list_for_each_entry_safe(fence, next, &irq->pending, irq_link) {
			list_del_init(&fence->irq_link);
			XE_WARN_ON(dma_fence_check_and_signal_locked(&fence->dma));
			dma_fence_put(&fence->dma);
		}
    ...
}

If the fences are removed from the pending list but not signaled, can this
cause any user-space process waiting on them to hang indefinitely?

Furthermore, since the xe driver does not implement the cancel_job DRM
scheduler backend op, it seems drm_sched_fini() will not clean up the
pending_list, which could leak the unsignaled xe_sched_job structures.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012093053.2103743-5-satyanarayana.k.v.p@intel.com?part=2