From: Peter Zijlstra <peterz@infradead.org>
To: Josh Don <joshdon@google.com>
Cc: Kayra Cizmeci <kayracizmeci@gmail.com>,
Vincent Guittot <vincent.guittot@linaro.org>,
mingo@redhat.com, juri.lelli@redhat.com,
dietmar.eggemann@arm.com, rostedt@goodmis.org,
bsegall@google.com, mgorman@suse.de, vschneid@redhat.com,
kprateek.nayak@amd.com, linux-kernel@vger.kernel.org,
jakub@okonski.org, longman@redhat.com, tj@kernel.org,
hannes@cmpxchg.org, mkoutny@suse.com, cgroups@vger.kernel.org,
jstultz@google.com, qyousef@layalina.io
Subject: Re: [PATCH v2] sched/eevdf: Fix sched idle task group preemption
Date: Fri, 23 Oct 2026 15:29:56 +0200 [thread overview]
Message-ID: <20261023132956.GD409038@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <20261022212150.GW2823843@noisy.programming.kicks-ass.net>
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;
+}
+
int sched_group_set_idle(struct task_group *tg, long idle)
{
int i;
@@ -15777,33 +15810,37 @@ int sched_group_set_idle(struct task_group *tg, long idle)
if (idle < 0 || idle > 1)
return -EINVAL;
- mutex_lock(&shares_mutex);
-
- if (tg->idle == idle) {
- mutex_unlock(&shares_mutex);
+ guard(mutex)(&shares_mutex);
+ if (tg->idle == idle)
return 0;
- }
tg->idle = idle;
for_each_possible_cpu(i) {
- struct rq *rq = cpu_rq(i);
- struct sched_entity *se = tg_se(tg, i);
struct cfs_rq *grp_cfs_rq = tg_cfs_rq(tg, i);
- bool was_idle = cfs_rq_is_idle(grp_cfs_rq);
- long idle_task_delta;
- struct rq_flags rf;
+ struct sched_entity *se = tg_se(tg, i);
+ int old_idle = grp_cfs_rq->idle;
+ unsigned long task_delta;
- rq_lock_irqsave(rq, &rf);
+ struct rq *rq = cpu_rq(i);
- grp_cfs_rq->idle = idle;
- if (WARN_ON_ONCE(was_idle == cfs_rq_is_idle(grp_cfs_rq)))
- goto next_cpu;
+ guard(rq_lock_irqsave)(rq);
- idle_task_delta = grp_cfs_rq->h_nr_queued -
- grp_cfs_rq->h_nr_idle;
- if (!cfs_rq_is_idle(grp_cfs_rq))
- idle_task_delta *= -1;
+ struct tg_set_idle_struct tsi = {
+ .tg = tg,
+ .idle = idle ? 1 : -1,
+ .cpu = i,
+ };
+ walk_tg_tree_from(tg, tg_nop, tg_set_idle_up, &tsi);
+
+ if (!!old_idle == !!grp_cfs_rq->idle)
+ continue;
+
+ task_delta = grp_cfs_rq->h_nr_queued - grp_cfs_rq->h_nr_idle;
+ if (!task_delta)
+ continue;
+
+ task_delta *= tsi.idle;
for_each_sched_entity(se) {
struct cfs_rq *cfs_rq = cfs_rq_of(se);
@@ -15811,15 +15848,11 @@ int sched_group_set_idle(struct task_group *tg, long idle)
if (!se->on_rq)
break;
- cfs_rq->h_nr_idle += idle_task_delta;
+ cfs_rq->h_nr_idle += task_delta;
- /* Already accounted at parent level and above. */
if (cfs_rq_is_idle(cfs_rq))
break;
}
-
-next_cpu:
- rq_unlock_irqrestore(rq, &rf);
}
/* Idle groups have minimum weight. */
@@ -15828,7 +15861,6 @@ int sched_group_set_idle(struct task_group *tg, long idle)
else
__sched_group_set_shares(tg, NICE_0_LOAD);
- mutex_unlock(&shares_mutex);
return 0;
}
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 499a5649582e..ae4eca092586 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -751,7 +751,7 @@ struct cfs_rq {
struct list_head leaf_cfs_rq_list;
struct task_group *tg; /* Group that "owns" this runqueue */
- /* Locally cached copy of our task_group's idle value */
+ /* Sum of all ancestor tg->idle. */
int idle;
# ifdef CONFIG_CFS_BANDWIDTH
@@ -3499,7 +3499,7 @@ print_numa_stats(struct seq_file *m, int node, unsigned long tsf,
unsigned long tpf, unsigned long gsf, unsigned long gpf);
#endif /* CONFIG_NUMA_BALANCING */
-extern void init_cfs_rq(struct cfs_rq *cfs_rq);
+extern void init_cfs_rq(struct cfs_rq *cfs_rq, struct cfs_rq *parent);
extern void init_rt_rq(struct rt_rq *rt_rq);
extern void init_dl_rq(struct dl_rq *dl_rq);
next prev parent reply other threads:[~2026-10-23 13:30 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-19 18:26 Vincent Guittot
2026-10-21 7:19 ` Kayra Cizmeci
2026-10-21 13:13 ` Peter Zijlstra
2026-10-21 13:32 ` Vincent Guittot
2026-10-21 13:41 ` Kayra Cizmeci
2026-10-21 14:50 ` Peter Zijlstra
2026-10-21 15:18 ` Kayra Cizmeci
2026-10-21 16:39 ` Josh Don
2026-10-22 11:47 ` Peter Zijlstra
2026-10-22 14:04 ` Kayra Cizmeci
2026-10-22 17:49 ` Josh Don
2026-10-22 21:21 ` Peter Zijlstra
2026-10-23 13:29 ` Peter Zijlstra [this message]
2026-10-23 13:49 ` Kayra Cizmeci
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261023132956.GD409038@noisy.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=bsegall@google.com \
--cc=cgroups@vger.kernel.org \
--cc=dietmar.eggemann@arm.com \
--cc=hannes@cmpxchg.org \
--cc=jakub@okonski.org \
--cc=joshdon@google.com \
--cc=jstultz@google.com \
--cc=juri.lelli@redhat.com \
--cc=kayracizmeci@gmail.com \
--cc=kprateek.nayak@amd.com \
--cc=linux-kernel@vger.kernel.org \
--cc=longman@redhat.com \
--cc=mgorman@suse.de \
--cc=mingo@redhat.com \
--cc=mkoutny@suse.com \
--cc=qyousef@layalina.io \
--cc=rostedt@goodmis.org \
--cc=tj@kernel.org \
--cc=vincent.guittot@linaro.org \
--cc=vschneid@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®