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

[sashiko-bot@kernel.org]

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?

> +		/** @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?

> +	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