On Fri, Aug 14, 2026 at 08:57:38AM +0100, Tvrtko Ursulin wrote: > Prevent pushing a new job to an entity seeing it being the first in the > queue, and hence entering the drm_sched_rq_add_entity() path, if the pop > side in drm_sched_entity_pop_job() has de-queued the job but not yet > updated the saved virtual time. > > We do this by pulling the locked sections out to encompass both the queue > push/pop and corresponding rbtree management. > > Signed-off-by: Tvrtko Ursulin <[email protected]> > Fixes: 2fa4d8e2c109 ("drm/sched: Add fair scheduling policy") > Suggested-by: [email protected] # via Claude Opus > Tested-by: [email protected] > Cc: Christian König <[email protected]> > Cc: Danilo Krummrich <[email protected]> > Cc: Philipp Stanner <[email protected]> > Cc: Pierre-Eric Pelloux-Prayer <[email protected]> > Cc: Matthew Brost <[email protected]>
Ugh. Idiom: "How much longer are we going to keep polishing this turd?" The revert of FAIR IMO shows it basically time for a rewrite, cough - DRM dep. If we can agree, I'll prioritize, if not we'll be here seemingly forever. Matt > Cc: Vitaly Prosyak <[email protected]> > --- > drivers/gpu/drm/scheduler/sched_entity.c | 8 +++++++- > drivers/gpu/drm/scheduler/sched_rq.c | 20 +++++++++----------- > 2 files changed, 16 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/scheduler/sched_entity.c > b/drivers/gpu/drm/scheduler/sched_entity.c > index a4a7efdbf229..673ca9cbf362 100644 > --- a/drivers/gpu/drm/scheduler/sched_entity.c > +++ b/drivers/gpu/drm/scheduler/sched_entity.c > @@ -559,9 +559,10 @@ struct drm_sched_job *drm_sched_entity_pop_job(struct > drm_sched_entity *entity) > */ > smp_wmb(); > > + spin_lock(&entity->lock); > spsc_queue_pop(&entity->job_queue); > - > drm_sched_rq_pop_entity(entity); > + spin_unlock(&entity->lock); > > /* Jobs and entities might have different lifecycles. Since we're > * removing the job from the entities queue, set the jobs entity pointer > @@ -647,6 +648,9 @@ void drm_sched_entity_push_job(struct drm_sched_job > *sched_job) > * Make sure to set the submit_ts first, to avoid a race. > */ > sched_job->submit_ts = submit_ts = ktime_get(); > + > + spin_lock(&entity->lock); > + > first = spsc_queue_push(&entity->job_queue, &sched_job->queue_node); > > /* first job wakes up scheduler */ > @@ -657,5 +661,7 @@ void drm_sched_entity_push_job(struct drm_sched_job > *sched_job) > if (sched) > drm_sched_wakeup(sched); > } > + > + spin_unlock(&entity->lock); > } > EXPORT_SYMBOL(drm_sched_entity_push_job); > diff --git a/drivers/gpu/drm/scheduler/sched_rq.c > b/drivers/gpu/drm/scheduler/sched_rq.c > index 0464d324d98d..23f46ec610e7 100644 > --- a/drivers/gpu/drm/scheduler/sched_rq.c > +++ b/drivers/gpu/drm/scheduler/sched_rq.c > @@ -257,19 +257,17 @@ static ktime_t drm_sched_entity_get_job_ts(struct > drm_sched_entity *entity) > struct drm_gpu_scheduler * > drm_sched_rq_add_entity(struct drm_sched_entity *entity, ktime_t ts) > { > + struct drm_sched_rq *rq = entity->rq; > struct drm_gpu_scheduler *sched; > - struct drm_sched_rq *rq; > > /* Add the entity to the run queue */ > - spin_lock(&entity->lock); > + lockdep_assert_held(&entity->lock); > + > if (entity->stopped) { > - spin_unlock(&entity->lock); > - > DRM_ERROR("Trying to push to a killed entity\n"); > return NULL; > } > > - rq = entity->rq; > spin_lock(&rq->lock); > sched = rq->sched; > > @@ -289,7 +287,6 @@ drm_sched_rq_add_entity(struct drm_sched_entity *entity, > ktime_t ts) > drm_sched_rq_update_fifo_locked(entity, rq, ts); > > spin_unlock(&rq->lock); > - spin_unlock(&entity->lock); > > return sched; > } > @@ -343,16 +340,17 @@ drm_sched_rq_next_rr_ts(struct drm_sched_rq *rq, > */ > void drm_sched_rq_pop_entity(struct drm_sched_entity *entity) > { > + struct drm_sched_rq *rq = entity->rq; > struct drm_sched_job *next_job; > - struct drm_sched_rq *rq; > + > + lockdep_assert_held(&entity->lock); > + > + spin_lock(&rq->lock); > > /* > * Update the entity's location in the min heap according to > * the timestamp of the next job, if any. > */ > - spin_lock(&entity->lock); > - rq = entity->rq; > - spin_lock(&rq->lock); > next_job = drm_sched_entity_queue_peek(entity); > if (next_job) { > ktime_t ts; > @@ -375,8 +373,8 @@ void drm_sched_rq_pop_entity(struct drm_sched_entity > *entity) > drm_sched_entity_save_vruntime(entity, min_vruntime); > } > } > + > spin_unlock(&rq->lock); > - spin_unlock(&entity->lock); > } > > /** > -- > 2.54.0 >
