On Mon, 2026-09-07 at 14:05 +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.
I think I don't fully get the bug from that message. The bug (is it even one?) you address is the performance bottleneck for CFS under high load, right? Anyways, I think I got a better idea: > > We do this by pulling the locked sections out to encompass both the queue > push/pop and corresponding rbtree management. So basically you move for locking the spsc-queue, the lockless queue which is usually used with the locks next to it anyways. That's cool stuff. I've suggested this for a while already https://docs.kernel.org/gpu/todo.html#replace-the-lockless-queue-with-a-locked-list So from a maintainability perspective, it would be far cooler if you go down this road one step further and kill the spsc_queue for good, replacing it with a fully locked list. AFAIR Christian and Danilo also have agreed to this. […] > @@ -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); btw. I think it *reads* a bit safer if the rq initalization happens below the lockdep assertion. Also might make the diff nicer. P.
