Re: [PATCH v4 10/25] 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] PT jobs on wedged, killed, or banned queues silently report success to the DRM scheduler, masking failures.
- [High] Memory leak of xe_pt_job_ops and dma_fence references if a PT job is aborted before execution.
--

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

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

This commit teaches the GuC submission backend to execute PT (Page Table) jobs.
PT jobs bypass GPU execution for the final step of a bind job, instead using
the CPU to program the required page tables.

Link: https://patch.msgid.link/20260228013501.106680-11-matthew.brost@intel.com

> 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
> @@ -1261,14 +1277,25 @@ guc_exec_queue_run_job(struct drm_sched_job *drm_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);
> +		} else {
> +			if (!exec_queue_registered(q))
> +				register_exec_queue(q, GUC_CONTEXT_NORMAL);
> +			if (!job->restore_replay)
> +				q->ring_ops->emit_job(job);
> +			submit_exec_queue(q, job);
> +		}
>  		job->restore_replay = false;
>  	}
>  
> +	if (is_pt_job(job)) {
> +		xe_pt_job_ops_put(job->pt_update[0].pt_job_ops);
> +		dma_fence_put(job->fence);	/* Drop ref from xe_sched_job_arm */
> +		return NULL;
> +	}

[Severity: High]
If the queue is wedged, killed, or banned, the execution block above is
skipped, but this block unconditionally returns NULL for PT jobs. Can this
cause PT jobs on failed queues to silently report success to the DRM
scheduler and mask failures?

[Severity: High]
This handles dropping the extra dma_fence and xe_pt_job_ops references during
normal execution, but what happens if the DRM scheduler aborts the job before
it executes (for example, if the queue is destroyed)?

Looking at xe_sched_job_destroy() in drivers/gpu/drm/xe/xe_sched_job.c:

> void xe_sched_job_destroy(struct kref *ref)
> {
>         struct xe_sched_job *job =
>                 container_of(ref, struct xe_sched_job, refcount);
> ...
>         xe_sched_job_free_fences(job);
>         dma_fence_put(job->fence);
>         drm_sched_job_cleanup(&job->drm);
>         job_free(job);
> ...
> }

It seems the destructor fails to call xe_pt_job_ops_put() for PT jobs and
only drops the primary fence reference. Does this code leak the xe_pt_job_ops
structure and the extra dma_fence reference taken during job arming if
execution is aborted?

>  run_job_out:
>  
>  	return job->fence;

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