Re: [PATCH 8/9] drm/sched: Replace completion with a flush

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/sched: cross-entity deadlock via flush_work() in drm_sched_entity_kill()
--

commit 3786a3d89362d00183a97f456e4eaeb1dff144f4
Author: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>

drm/sched: Replace completion with a flush

This commit removes the per-entity completion used to synchronize entity
teardown, replacing it with a direct flush of the scheduler's run job
worker. The goal is to achieve the same synchronization guarantee without
requiring the scheduler to signal a completion when an entity goes idle.

> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
> --- a/drivers/gpu/drm/scheduler/sched_entity.c
> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
[ ... ]
> @@ -304,16 +299,23 @@ static void drm_sched_entity_kill_jobs_cb(struct dma_fence *f,
>  void drm_sched_entity_kill(struct drm_sched_entity *entity)
>  {
[ ... ]
> -	/* Make sure this entity is not used by the scheduler at the moment */
> -	wait_for_completion(&entity->entity_idle);
> +	/*
> +	 * Make sure this entity is not used by the scheduler at the moment.
> +	 *
> +	 * Scheduler is guaranteed to be stable after the entity was stopped and
> +	 * removed from the run-queue.
> +	 */
> +	if (sched)
> +		drm_sched_flush_run_work(sched);

[Severity: High]
Could this introduce a cross-entity deadlock regression during context teardown?

Because drm_sched_flush_run_work calls flush_work on the scheduler's shared
worker, it synchronously waits for the worker to finish its current execution.
This might cause it to wait on a job belonging to a completely different
entity.

If a process destroys its device context and triggers drm_sched_entity_kill
while holding a driver-specific lock, and the shared worker is concurrently
running drm_sched_run_job_work for an unrelated entity, that driver-provided
run_job callback might attempt to acquire the same driver lock.

The original code used a per-entity completion (entity_idle) which explicitly
avoided waiting on other entities. Does this flush create a new cross-entity
dependency that could lead to a deadlock?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002154713.77591-1-tvrtko.ursulin@igalia.com?part=8