Re: [PATCH v5 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 3 potential issue(s) to consider:
- [High] ULLS_EXIT_JIFFIES evaluates to 0 on HZ=100 configurations, causing a tight spin loop.
- [High] Deadlock caused by synchronous dma_fence_wait while holding reclaim-tainted job_mutex.
- [Medium] Asynchronous runtime PM put used during device teardown path.
--

commit 1b246ef0dc00023e2aba23868ed86c7ff7c9257a
Author: Matthew Brost <matthew.brost@intel.com>

drm/xe: Add ULLS migration job support to migration layer

Add function to enter ULLS mode for migration job and delayed worker to
exit (power saving). ULLS mode expected to entered upon page fault or
SVM prefetch. ULLS mode exit delay is currently set to 5ms.

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
> index 471ae5741836e..588e1c9085f31 100644
> --- 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 / 200)	/* 5ms */

[Severity: High]
Does this calculation truncate to 0 when HZ=100?

If HZ < 200, ULLS_EXIT_JIFFIES evaluates to 0. When xe_migrate_ulls_exit
executes while the queue is not idle, it reschedules itself with a delay
of 0. This results in an immediate re-execution of the worker, creating a
tight spin loop and causing a soft lockup with 100% CPU utilization until
the queue becomes idle.

> +		/** @ulls.exit_work: ULLS exit worker */
> +		struct delayed_work exit_work;
> +	} ulls;
>  };
>  
>  #define MAX_PREEMPTDISABLE_TRANSFER SZ_8M /* Around 1ms. */
> @@ -98,6 +109,16 @@ struct xe_migrate {
>  static void xe_migrate_fini(void *arg)
>  {
>  	struct xe_migrate *m = arg;
> +	struct xe_device *xe = tile_to_xe(m->tile);
> +
> +	disable_delayed_work_sync(&m->ulls.exit_work);
> +	mutex_lock(&m->job_mutex);
> +	if (m->ulls.enabled) {
> +		xe_force_wake_put(gt_to_fw(m->q->hwe->gt), m->q->hwe->domain);
> +		xe_pm_runtime_put(xe);

[Severity: Medium]
Will this asynchronous put call in the teardown path be canceled by the
subsequent PM disable?

xe_migrate_fini acts as the devm cleanup action. It calls xe_pm_runtime_put
which queues an asynchronous suspend. However, during driver teardown,
pm_runtime_disable will cancel this pending async work before it executes,
leaving the hardware incorrectly in an active power state.

Should this be a synchronous pm_runtime_put_sync instead?

> +		m->ulls.enabled = false;
> +	}
> +	mutex_unlock(&m->job_mutex);
>  
>  	xe_vm_lock(m->q->vm, false);
>  	xe_bo_unpin(m->pt_bo);

[ ... ]

> +static void xe_migrate_ulls_exit(struct work_struct *work)
> +{
> +	struct xe_migrate *m = container_of(work, struct xe_migrate,
> +					    ulls.exit_work.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]
Does waiting on this dma-fence while holding the reclaim-tainted job_mutex
create a deadlock?

m->job_mutex is explicitly annotated as being part of the memory reclaim path
via fs_reclaim_acquire/release in xe_migrate_init. By synchronously
waiting on a dma-fence with dma_fence_wait while holding this mutex, a
circular dependency is created with the memory allocator.

If the system is under memory pressure and triggers reclaim while signaling
the fence, or if a GPU reset (which must allocate memory) is needed to signal
the fence, the reclaim path will attempt to acquire job_mutex and block
forever.

> +	dma_fence_put(fence);
> +
> +	m->ulls.enabled = false;
> +unlock_exit:

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