Cleanup work in the preceding commit added locking to drm_sched_entity_pop_job(). This cleanup causes a slightly sub-optimal lock cycle with drm_sched_rq_pop_entity().
sched_entity also utilizes the lockless spsc_queue (partially already used simultaneously with locks), which was marked for removal in
commit 6e7eb171ac96 ("Documentation: drm: Add entry for removing spsc_queue to TODO list")
To remove the lock-cycle mentioned above, the unlock must be moved downwards, also locking the lockless queue.
Guard spsc_queue_pop() in drm_sched_entity_pop_job() with the lock and document why that is being done.
Signed-off-by: Philipp Stanner phasta@kernel.org --- drivers/gpu/drm/scheduler/sched_entity.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c index 91aec20611ad..5cf0af91faf2 100644 --- a/drivers/gpu/drm/scheduler/sched_entity.c +++ b/drivers/gpu/drm/scheduler/sched_entity.c @@ -529,9 +529,17 @@ struct drm_sched_job *drm_sched_entity_pop_job(struct drm_sched_entity *entity) spin_lock(&entity->lock); prev_last_scheduled = entity->last_scheduled; entity->last_scheduled = dma_fence_get(&sched_job->s_fence->finished); - spin_unlock(&entity->lock);
+ /* Preceding cleanup work made it necessary to add the spinlock + * to this function. spsc_queue, a lockless queue, is now + * counterintuitively guarded by the lock as well. spsc_queue is queued + * for removal (see DRM TODO list), so this somewhat serves as a + * preparational step. + * + * TODO: Replace spsc_queue completely with a locked (h)list. + */ spsc_queue_pop(&entity->job_queue); + spin_unlock(&entity->lock);
dma_fence_put(prev_last_scheduled); drm_sched_rq_pop_entity(entity);