Re: [PATCH v7 13/24] drm/xe: Enable CPU binds for jobs

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

On Fri, Sep 25, 2026 at 06:03:25AM +0000, sashiko-bot@kernel.org wrote:
> 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 dma_fence when PT jobs are canceled
> - [High] drm/xe: GPU hang and memory leak due to missing CPU bind support in Execlist backend
> --
> 
> commit 6471c15e2149c2517458e891e7dceef8c5573f97
> Author: Matthew Brost <matthew.brost@intel.com>
> 
> drm/xe: Enable CPU binds for jobs
> 
> This commit updates the migration layer to formulate page table jobs that
> issue CPU binds in the submission backend. All code related to GPU-based
> binding is removed to lower latency and avoid PTE migration limits.
> 
> > diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
> > --- a/drivers/gpu/drm/xe/xe_migrate.c
> > +++ b/drivers/gpu/drm/xe/xe_migrate.c
> [ ... ]
> > @@ -1937,162 +1866,18 @@ __xe_migrate_update_pgtables(struct xe_migrate *m,
> >  {
> >  	const struct xe_migrate_pt_update_ops *ops = pt_update->ops;
> >  	struct xe_tile *tile = m->tile;
> >  	struct xe_sched_job *job;
> >  	struct dma_fence *fence;
> >  	bool is_migrate = is_migrate_queue(m, pt_update_ops->q);
> > +	int err;
> >  
> > -	job = xe_bb_create_migration_job(pt_update_ops->q, bb,
> > -					 xe_migrate_batch_base(m, usm),
> > -					 update_idx);
> > +	job = xe_sched_job_create(pt_update_ops->q, NULL);
> 
> [Severity: High]
> Does the Execlist backend properly handle these new PT jobs?
> 

Execlist backend is dead. Non-issue.

> Since xe_sched_job_create() is now called with a NULL batch address, these
> jobs rely on backend interception. However, looking at execlist_run_job() in
> the Execlist backend:
> 
> drivers/gpu/drm/xe/xe_execlist.c:execlist_run_job() {
>     ...
>     q->ring_ops->emit_job(job);
>     xe_execlist_make_active(exl);
> 
>     return job->fence;
> }
> 
> If GuC submission is disabled, it appears the backend receives this job and
> directly emits it to the ring without an is_pt_job() check. Could this result
> in pushing an uninitialized batch address to the hardware, causing a GPU hang
> and leaking the pt_job_ops structures?
> 
> >  	if (IS_ERR(job)) {
> >  		err = PTR_ERR(job);
> > -		goto err_sa;
> > +		goto err_out;
> >  	}
> >  
> > -	xe_sched_job_add_migrate_flush(job, MI_INVALIDATE_TLB);
> > +	xe_tile_assert(tile, job->is_pt_job);
> >  
> >  	if (ops->pre_commit) {
> >  		pt_update->job = job;
> > @@ -2103,6 +1888,12 @@ __xe_migrate_update_pgtables(struct xe_migrate *m,
> >  	if (is_migrate)
> >  		mutex_lock(&m->job_mutex);
> >  
> > +	job->pt_update[0].vm = pt_update->vops->vm;
> > +	job->pt_update[0].tile = tile;
> > +	job->pt_update[0].ops = ops;
> > +	job->pt_update[0].pt_job_ops =
> > +		xe_pt_job_ops_get(pt_update_ops->pt_job_ops);
> > +
> 
> [Severity: High]
> Is there a memory leak if this PT job gets canceled before execution?
> 

Again non-issue - _arm() always equals run_job() call. Non-issue.

Matt

> We acquire a reference to pt_job_ops here, and xe_sched_job_arm() takes an
> extra dma_fence reference. If the job is canceled (for example, from an
> execution queue kill or GPU reset), the DRM scheduler calls the free_job
> callback which ultimately calls xe_sched_job_destroy():
> 
> drivers/gpu/drm/xe/xe_sched_job.c:xe_sched_job_destroy() {
>     ...
>     xe_sched_job_free_fences(job);
>     dma_fence_put(job->fence);
>     drm_sched_job_cleanup(&job->drm);
>     job_free(job);
> }
> 
> Since xe_sched_job_destroy() doesn't check job->is_pt_job, it seems we never
> drop the pt_job_ops or the extra dma_fence reference. This would permanently
> leak the deferred actions and their associated page table BO references. Does
> the job destruction path need updating to clean up canceled PT jobs?
> 
> >  	xe_sched_job_arm(job);
> >  	fence = dma_fence_get(&job->drm.s_fence->finished);
> >  	xe_sched_job_push(job);
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260925045320.1325860-1-matthew.brost@intel.com?part=13