Re: [PATCH v4 20/25] drm/xe: Add ULLS migration job support to migration layer

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

On Fri, Sep 04, 2026 at 12:27:20AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Waiting on a dma_fence while holding a reclaim-tainted mutex causes deadlocks and lockdep splats.
> - [Medium] ULLS mode exit delay is implemented as 20ms instead of the intended 5us.
> --
> 
> commit 68ec8c46273b3023414fee593faab1cdb6c24eca
> Author: Matthew Brost <matthew.brost@intel.com>
> 
> drm/xe: Add ULLS migration job support to migration layer
> 
> This commit adds a function to enter ULLS mode for migration jobs and a
> delayed worker to exit it for power savings. The ULLS mode exit delay is
> intended to be 5us. It also updates migration job flags so the submission
> backend properly submits in ULLS mode.
> 
> Link: https://patch.msgid.link/20260228013501.106680-21-matthew.brost@intel.com
> 
> > 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
> > @@ -77,6 +80,14 @@ struct xe_migrate {
> >  	struct dma_fence *fence;
> >  	/** @min_chunk_size: For dgfx, Minimum chunk size */
> >  	u64 min_chunk_size;
> > +	/** @ulls: ULLS support */
> > +	struct {
> > +		/** @ulls.enabled: ULLS is enabled */
> > +		bool enabled;
> > +#define ULLS_EXIT_JIFFIES	(HZ / 50)
> 
> [Severity: Medium]
> The commit message states "ULLS mode exit delay is currently set to 5us", but
> does this macro actually result in 5us? 
> 
> Using (HZ / 50) yields 20 milliseconds on a 1000Hz kernel, and 20 milliseconds
> on a 100Hz kernel (since 100/50 is 2 jiffies). Both evaluate to 20,000us
> instead of 5us.
> 
> Additionally, delayed_work uses jiffy-resolution timers which cannot achieve
> microsecond precision. Will this cause the engine to remain awake and consume
> power for 4000x longer than intended?
> 

Code and commit message are wrong, intent is 5ms. Will fix both.

> > +		/** @ulls.exit_work: ULLS exit worker */
> > +		struct delayed_work exit_work;
> > +	} ulls;
> >  };
> 
> [ ... ]
> 
> > +static void xe_migrate_ulls_exit(struct work_struct *work)
> > +{
> 
> [ ... ]
> 
> > +	mutex_lock(&m->job_mutex);
> > +
> > +	if (!xe_exec_queue_is_idle(m->q, 1))
> > +		goto unlock_exit;
> > +
> > +	xe_sched_job_get(job);
> > +	xe_sched_job_arm(job);
> > +	job->is_ulls = true;
> > +	job->is_ulls_last = true;
> > +	fence = dma_fence_get(&job->drm.s_fence->finished);
> > +	xe_sched_job_push(job);
> > +
> > +	/* Serialize force wake put */
> > +	dma_fence_wait(fence, false);
> 
> [Severity: High]
> Can calling dma_fence_wait() here while holding m->job_mutex cause a deadlock?
> 
> The job_mutex is initialized with fs_reclaim_acquire(), which marks it as
> participating in memory reclaim paths. Waiting on a dma_fence while holding
> a reclaim-tainted lock is generally unsafe because fence signaling paths
> cannot safely allocate memory without risking a forward-progress dependency
> inversion.
> 
> If a page fault blocks on job_mutex (e.g. via xe_migrate_ulls_enter) while
> a GPU reset that requires memory reclaim is pending, could this deadlock the
> system and trigger a lockdep splat?
> 

No correct. A fence always signals without allocating more memory so no
deadlock risk.

Matt

> > +	dma_fence_put(fence);
> > +
> > +	m->ulls.enabled = false;
> > +unlock_exit:
> > +	if (job)
> > +		xe_sched_job_put(job);
> 
> [ ... ]
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260903235842.3401722-1-matthew.brost@intel.com?part=20