Re: [REGRESSION] drm/sched: FAIR policy causes serious performance degradation at max GPU load on 9070XT

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);
> 
> 
>