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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Memory leak of `xe_pt_job_ops` and `job->fence` when PT jobs are aborted by the DRM scheduler.
--

commit 5533668320d4e1611a5ff8ddda82df8a65a1b27b
Author: Matthew Brost <matthew.brost@intel.com>

drm/xe: Enable CPU binds for jobs
    
This patch removes GPU-based binding and updates the migration layer to
formulate a PT job which will issue CPU bind in the submission backend. This
lowers latency and avoids running out of migration PTEs.

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

> diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
> index ba8e195afcc89..e5c46e0fa9607 100644
> --- a/drivers/gpu/drm/xe/xe_migrate.c
> +++ b/drivers/gpu/drm/xe/xe_migrate.c
[ ... ]
> @@ -2104,6 +1889,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);
> +
>  	xe_sched_job_arm(job);
>  	fence = dma_fence_get(&job->drm.s_fence->finished);
>  	xe_sched_job_push(job);

[Severity: Medium]
Are these references correctly released if the DRM scheduler aborts the job?

When a PT job is created and armed, pt_job_ops gets a reference here, and the
job's fence receives an extra reference. Normally, guc_exec_queue_run_job()
drops both of these references:

drivers/gpu/drm/xe/xe_guc_submit.c:guc_exec_queue_run_job() {
	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;
	}
...
}

However, if the job is aborted (e.g., queue closed, wedging), run_job() is
bypassed and the scheduler calls xe_sched_job_destroy() instead. Looking at
xe_sched_job_destroy(), it only puts job->fence once and lacks a call to
xe_pt_job_ops_put() entirely:

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);
...
}

Does this permanently leak the pt_job_ops structure and the second fence
reference when PT jobs are aborted during error paths?

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