On 03/10/2026 02:33, [email protected] wrote:
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 <[email protected]>
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?
I don't see that anyone flushes entities under some driver lock, nor
that anyone would ever want to do that. So I am trending towards saying
this is not a concern.
Regards,
Tvrtko
