Looks good, in the 10 minutes I spent this morning doing a quick test, I wasn't able to reproduce the failure. I will do extended testing tonight.
I will report back later with more details, but given the issue was typically reproducible on-demand, this is looking very promising. Thanks for all the hard work everyone! On Thursday, August 13th, 2026 at 12:58 AM, Tvrtko Ursulin <[email protected]> wrote: > > On 13/08/2026 07:42, [email protected] wrote: > > Ok, after a lot of A/B testing, it appears that trying your > > "drm-intel/drm-sched-fair-fixed" branch, the stuttering is still there. > > It's definitely paced differently though, I will attach a video. It's > > "choppier" when it happens. I tested in a few different areas / times of > > day, in case you notice the scenery isn't the same; this is representative > > of what I experienced in other places. I will note, that I tried playing 4 > > times, 3/4 times, shortly after boot. 1/4 times I left my desktop to idle > > for about 30 minutes while I did something else, and I didn't encounter the > > issue at all in 10 minutes. Not sure how significant that is, or if it's > > just noise. > > > > https://www.youtube.com/watch?v=GWyIVEuhooM > > > > To make sure I wasn't insane, I went back to the patch Claude gave me, > > applied against a clean 7.2.0-rc7. At least again in a few sessions after > > boot, no issue. I'll post that exact code below. > > Yes, I had a brain fart yesterday and had only pulled the lock out on > the pop side. I have now pushed the updated branch, with both the add > and pop side made symmetric in lock taking aspect. > > If you could pull and re-test once more that would be great. > > Regards, > > Tvrtko > > > If you need any other info from me let me know. > > > > > > --- a/drivers/gpu/drm/scheduler/sched_rq.c > > +++ b/drivers/gpu/drm/scheduler/sched_rq.c > > @@ -2,6 +2,7 @@ > > /* Copyright 2015 Advanced Micro Devices, Inc. */ > > /* Copyright (c) 2025 Valve Corporation */ > > > > +#include <linux/moduleparam.h> > > #include <linux/rbtree.h> > > > > #include <drm/drm_print.h> > > @@ -9,6 +10,32 @@ > > > > #include "sched_internal.h" > > > > +/* > > + * Diagnostic counters. Not for submission. > > + * > > + * dbg_pop_stayed - pops where the entity had another job queued and so > > stayed > > + * in the tree. No save/restore of vruntime occurs. > > + * dbg_pop_left - pops where the entity queue drained, so it left the > > tree > > + * and drm_sched_entity_save_vruntime() ran. Only this > > path > > + * arms a later restore. > > + * dbg_add_restore - calls to drm_sched_rq_add_entity(), i.e. restores. > > + * dbg_add_race - restores where the entity was still linked in the > > tree, > > + * meaning the vruntime being restored is still absolute. > > + * > > + * Incremented under rq->lock, so exact per scheduler and only mildly lossy > > + * when summed across rings. Writable so they can be reset between runs: > > + * echo 0 > /sys/module/gpu_sched/parameters/dbg_pop_left > > + */ > > +static unsigned long dbg_pop_stayed; > > +static unsigned long dbg_pop_left; > > +static unsigned long dbg_add_restore; > > +static unsigned long dbg_add_race; > > + > > +module_param(dbg_pop_stayed, ulong, 0644); > > +module_param(dbg_pop_left, ulong, 0644); > > +module_param(dbg_add_restore, ulong, 0644); > > +module_param(dbg_add_race, ulong, 0644); > > + > > static __always_inline bool > > drm_sched_entity_compare_before(struct rb_node *a, const struct rb_node > > *b) > > { > > @@ -266,14 +293,40 @@ > > sched = container_of(rq, typeof(*sched), rq); > > spin_lock(&rq->lock); > > > > + dbg_add_restore++; > > + if (!RB_EMPTY_NODE(&entity->rb_tree_node)) { > > + dbg_add_race++; > > + pr_warn_ratelimited("drm_sched: vruntime race on ring %s (comm > > %s)\n", > > + sched->name, current->comm); > > + } > > + > > if (list_empty(&entity->list)) { > > atomic_inc(sched->score); > > list_add_tail(&entity->list, &rq->entities); > > } > > > > - ts = drm_sched_rq_get_min_vruntime(rq); > > - ts = drm_sched_entity_restore_vruntime(entity, ts, rq->head_prio); > > - drm_sched_rq_update_tree_locked(entity, rq, ts); > > + /* > > + * Only restore the vruntime if the entity actually left the run queue. > > + * > > + * drm_sched_entity_pop_job() dequeues the last job and only afterwards > > + * calls drm_sched_rq_pop_entity(), which is where the entity is removed > > + * from the tree and drm_sched_entity_save_vruntime() converts its > > + * vruntime to min_vruntime-relative form. A push landing in that window > > + * sees an empty queue, takes the "first job" path to here, and would > > + * restore a vruntime which is still absolute -- adding min_vruntime to > > + * it a second time. If the entity is also the leftmost one, > > + * drm_sched_rq_get_min_vruntime() returns the entity's own vruntime and > > + * the value is doubled, placing it far to the right of the tree where > > it > > + * will not be selected again until the run queue catches up. > > + * > > + * A still-linked entity never left, so its vruntime is already absolute > > + * and its tree position is valid. The concurrent pop will update both. > > + */ > > + if (RB_EMPTY_NODE(&entity->rb_tree_node)) { > > + ts = drm_sched_rq_get_min_vruntime(rq); > > + ts = drm_sched_entity_restore_vruntime(entity, ts, > > rq->head_prio); > > + drm_sched_rq_update_tree_locked(entity, rq, ts); > > + } > > > > spin_unlock(&rq->lock); > > spin_unlock(&entity->lock); > > @@ -330,11 +383,13 @@ > > if (next_job) { > > ktime_t ts; > > > > + dbg_pop_stayed++; > > ts = drm_sched_entity_update_vruntime(entity); > > drm_sched_rq_update_tree_locked(entity, rq, ts); > > } else { > > ktime_t min_vruntime; > > > > + dbg_pop_left++; > > drm_sched_rq_remove_tree_locked(entity, rq); > > min_vruntime = drm_sched_rq_get_min_vruntime(rq); > > drm_sched_entity_save_vruntime(entity, min_vruntime); > > > > > > > >
