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

[Matthew Brost <matthew.brost@intel.com>]

On Fri, Sep 04, 2026 at 12:36:49AM +0000, sashiko-bot@kernel.org wrote:
> 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?
> 

The intent to is drop the job on sliently in this case, so code is correct.

> [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?

Every job put into the scheduler reaches run_job, so a non-issue.

Matt

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