Re: [PATCH v8 10/24] drm/xe: Update GuC submission backend to run PT jobs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/xe: Memory leak of pt_job_ops and fence refs on aborted PT jobs
- [High] drm/xe: guc_exec_queue_run_job() drops error status for aborted PT jobs
--

commit 9cf187bc8ca7b690be92bb4415979a7f9395643d
Author: Matthew Brost <matthew.brost@intel.com>
Date:   Fri Sep 25 20:15:12 2026 +0000

drm/xe: Update GuC submission backend to run PT jobs

PT jobs bypass GPU execution for the final step of a bind job, using the
CPU to program the required page tables. Teach the GuC submission backend
how to execute these jobs.

> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
[ ... ]
>  static struct dma_fence *
>  guc_exec_queue_run_job(struct drm_sched_job *drm_job)
>  {
>  	struct xe_sched_job *job = to_xe_sched_job(drm_job);
>  	struct xe_exec_queue *q = job->q;
>  	struct xe_guc *guc = exec_queue_to_guc(q);
> -	bool killed_or_banned_or_wedged =
> -		exec_queue_killed_or_banned_or_wedged(q);
> +	bool killed_or_banned_or_wedged_or_error  =
> +		exec_queue_killed_or_banned_or_wedged(q) ||
> +		xe_sched_job_is_error(job);
>  
>  	xe_gt_assert(guc_to_gt(guc), !(exec_queue_destroyed(q) || exec_queue_pending_disable(q)) ||
>  		     exec_queue_banned(q) || exec_queue_suspended(q));
>  
>  	trace_xe_sched_job_run(job);
>  
> -	if (!killed_or_banned_or_wedged && !xe_sched_job_is_error(job)) {
> +	if (is_pt_job(job)) {
> +		xe_gt_assert(guc_to_gt(guc), !exec_queue_registered(q));
> +		run_pt_job(job, killed_or_banned_or_wedged_or_error);
> +		xe_pt_job_ops_put(job->pt_update[0].pt_job_ops);
> +		dma_fence_put(job->fence);	/* Drop ref from xe_sched_job_arm */

[Severity: High]
If a PT job is aborted by the DRM scheduler before guc_exec_queue_run_job()
is called, do these references get properly released?

When jobs are aborted (e.g., due to failing dependencies or context
teardown), the cleanup path typically goes through xe_sched_job_destroy().

Since these references are dropped exclusively on this execution path, it
seems they might permanently leak xe_pt_job_ops and dma_fence_stub if the
job never makes it here.

> +
> +		return NULL;

[Severity: High]
Could unconditionally returning NULL here hide error states from the DRM
scheduler?

When killed_or_banned_or_wedged_or_error is true, run_pt_job() is passed
true for force_clear, which correctly clears the page tables. 

However, returning NULL tells the DRM scheduler that the job completed
successfully without error.

This false success signal might cause dependent jobs on other queues to
execute on unbound memory and trigger GPU page faults.

> +	} else if (!killed_or_banned_or_wedged_or_error) {
>  		if (xe_exec_queue_is_multi_queue_secondary(q)) {

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