Re: [PATCH v2] sched/eevdf: Fix sched idle task group preemption

[Kayra Cizmeci]

On Fri, Oct 23, 2026 at 03:29:56PM +0200, Peter Zijlstra wrote:
> On Thu, Oct 22, 2026 at 11:21:50PM +0200, Peter Zijlstra wrote:
> > On Thu, Oct 22, 2026 at 10:49:36AM -0700, Josh Don wrote:
> 
> > > Took me a bit to parse this double traversal. I think this is broken
> > > for the clear side?
> > > 
> > > root
> > >   |
> > >  A ( 2 tasks)
> > >  |
> > >  B (3 tasks)
> > > 
> > > If we start out with A marked as idle, then we have
> > > root: h_nr_idle = 5
> > > A: h_nr_idle = 3
> > > B: h_nr_idle = 0
> > > 
> > > When we clear, we'll first start with B and subtract 3...
> > > root: h_nr_idle = 2
> > > A: h_nr_idle = 0
> > > B: h_nr_idle = 0
> > > 
> > > Then we go to A and try to subtract 5 leading to
> > > root: h_nr_idle = -3
> > > 
> > 
> > Argh. Yes, I seem to have gotten it wrong. I'll try again in the
> > morning.
> 
> Bah, so I can't operate cgroup well enough to actually set up the above
> scenario. I think I'm stuck in cgroup-v2 mode which disallows this. I
> should probably ask one of the AI thingies to write me a script.
> 
> Anyway, I think the below *should* work. It seems to behave for the
> simple cases I did test.
> 
> This also fixes that init thing you noted, and yes, we can use this as
> an optimization for the find_matching_se() -- although I still think
> those semantics are weird, but whatever. If that's what people really
> want.
> 
> ---
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 0ab15f03cb94..731befb0f671 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -8970,7 +8970,7 @@ void __init sched_init(void)
>  		rq->nr_running = 0;
>  		rq->calc_load_active = 0;
>  		rq->calc_load_update = jiffies + LOAD_FREQ;
> -		init_cfs_rq(&rq->cfs);
> +		init_cfs_rq(&rq->cfs, NULL);
>  		init_rt_rq(&rq->rt);
>  		init_dl_rq(&rq->dl);
>  #ifdef CONFIG_FAIR_GROUP_SCHED
> diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
> index e6a3b516c703..9914498df0e2 100644
> --- a/kernel/sched/debug.c
> +++ b/kernel/sched/debug.c
> @@ -1090,6 +1090,7 @@ void print_cfs_rq(struct seq_file *m, int cpu, struct cfs_rq *cfs_rq)
>  	SEQ_printf(m, "  .%-30s: %d\n", "h_nr_runnable", cfs_rq->h_nr_runnable);
>  	SEQ_printf(m, "  .%-30s: %d\n", "h_nr_queued", cfs_rq->h_nr_queued);
>  	SEQ_printf(m, "  .%-30s: %d\n", "h_nr_idle", cfs_rq->h_nr_idle);
> +	SEQ_printf(m, "  .%-30s: %d\n", "idle", cfs_rq->idle);
>  	SEQ_printf(m, "  .%-30s: %ld\n", "load", cfs_rq->load.weight);
>  	SEQ_printf(m, "  .%-30s: %lu\n", "load_avg",
>  			cfs_rq->avg.load_avg);
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index b5546cd27dba..2fcb01a514c5 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -15581,8 +15581,10 @@ static void set_next_task_fair(struct rq *rq, struct task_struct *p, enum snt_e
>  		hrtick_start_fair(rq, p);
>  }
>  
> -void init_cfs_rq(struct cfs_rq *cfs_rq)
> +void init_cfs_rq(struct cfs_rq *cfs_rq, struct cfs_rq *parent)
>  {
> +	if (parent)
> +		cfs_rq->idle = parent->idle;
>  	cfs_rq->tasks_timeline = RB_ROOT_CACHED;
>  	cfs_rq->zero_vruntime = (u64)(-(1LL << 20));
>  	raw_spin_lock_init(&cfs_rq->removed.lock);
> @@ -15633,7 +15635,7 @@ int alloc_fair_sched_group(struct task_group *tg, struct task_group *parent)
>  			goto err;
>  
>  		se = tg_se(tg, i);
> -		init_cfs_rq(cfs_rq);
> +		init_cfs_rq(cfs_rq, tg_cfs_rq(parent, i));
>  		init_tg_cfs_entry(tg, cfs_rq, se, i, tg_se(parent, i));
>  		init_entity_runnable_average(se);
>  	}
> @@ -15767,6 +15769,37 @@ int sched_group_set_shares(struct task_group *tg, unsigned long shares)
>  	return ret;
>  }
>  
> +struct tg_set_idle_struct {
> +	struct task_group *tg;
> +	long idle;
> +	int cpu;
> +};
> +
> +static int tg_set_idle_up(struct task_group *tg, void *data)
> +{
> +	struct tg_set_idle_struct *tsi = data;
> +	struct cfs_rq *grp_cfs_rq = tg_cfs_rq(tg, tsi->cpu);
> +	struct sched_entity *se = tg_se(tg, tsi->cpu);
> +	int old_idle = grp_cfs_rq->idle;
> +	long task_delta;
> +
> +	grp_cfs_rq->idle += tsi->idle;
> +
> +	if (tg == tsi->tg)
> +		return 0;
> +
> +	if (!!old_idle == !!grp_cfs_rq->idle)
> +		return 0;
> +
> +	task_delta = grp_cfs_rq->h_nr_queued - grp_cfs_rq->h_nr_idle;
> +	if (!task_delta)
> +		return 0;
> +
> +	task_delta *= tsi->idle;
> +	cfs_rq_of(se)->h_nr_idle += task_delta;
> +	return 0;
> +}

I think this works wrong again on here AFAICT :-):

ROOT
  |
  A (3 Tasks)
  |
  B (2 Tasks)

When we do A idle, it should be:

B = 0, A = 2, ROOT = 5.

We start from B, We add +2 to A. Now, B = 0, A = 2, ROOT = 0.
Then we get to A. 5 - 2 is 3, we add h_nr_idle 3 and ROOT = 3.
While it should be 5, no?

Thanks!

Kayra