* [PATCH 1/2] sched/fair: reuse the ENQUEUE_DELAYED calculation in enqueue_task_fair()
@ 2026-08-24 12:52 Kayra Cizmeci
2026-08-24 12:52 ` [PATCH 2/2] sched/fair: avoid recalculating curr status in place_entity() and requeue_delayed_entity() Kayra Cizmeci
2026-08-26 10:15 ` [PATCH v2 0/2] sched/fair: reduce repeated work in enqueue path Kayra Cizmeci
0 siblings, 2 replies; 20+ messages in thread
From: Kayra Cizmeci @ 2026-08-24 12:52 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, K Prateek Nayak
Cc: Kayra Cizmeci, linux-kernel
Currently flags & ENQUEUE_DELAYED checks spread throughout the
enqueue_task_fair(), connect these checks to a bool 'delayed' that
gets calculated at the start of the function.
No functional change intended.
Signed-off-by: Kayra Cizmeci <kayracizmeci@gmail.com>
---
kernel/sched/fair.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 6d881e530f89..b411384125ec 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7985,7 +7985,7 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
struct sched_entity *se = &p->se;
struct cfs_rq *cfs_rq = &rq->cfs;
unsigned long weight;
- bool curr;
+ bool curr, delayed = (flags & ENQUEUE_DELAYED);
if (task_is_throttled(p) && enqueue_throttled_task(p))
return;
@@ -7996,12 +7996,12 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
* Let's add the task's estimated utilization to the cfs_rq's
* estimated utilization, before we update schedutil.
*/
- if (!p->se.sched_delayed || (flags & ENQUEUE_DELAYED))
+ if (!p->se.sched_delayed || delayed)
util_est_enqueue(cfs_rq, p);
update_curr_eevdf(cfs_rq);
- if (flags & ENQUEUE_DELAYED) {
+ if (delayed) {
requeue_delayed_entity(cfs_rq, se);
return;
}
--
2.53.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/2] sched/fair: avoid recalculating curr status in place_entity() and requeue_delayed_entity()
2026-08-24 12:52 [PATCH 1/2] sched/fair: reuse the ENQUEUE_DELAYED calculation in enqueue_task_fair() Kayra Cizmeci
@ 2026-08-24 12:52 ` Kayra Cizmeci
2026-08-26 10:15 ` [PATCH v2 0/2] sched/fair: reduce repeated work in enqueue path Kayra Cizmeci
1 sibling, 0 replies; 20+ messages in thread
From: Kayra Cizmeci @ 2026-08-24 12:52 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, K Prateek Nayak
Cc: Kayra Cizmeci, linux-kernel
In enqueue_task_fair() a bool is calculated by cfs_rq->curr == se.
But this information gets recalculated on requeue_delayed_entity() and
requeue_delayed_entity() only gets called in enqueue_task_fair().
And on place_entity() if se == curr we call the avg_vruntime_weight()
twice with the same input. place_entity() only gets called in
enqueue_task_fair() and requeue_delayed_entity().
Use the information on enqueue_task_fair() in requeue_delayed_entity().
And place_entity() to avoid calling avg_vruntime_weight() twice.
Signed-off-by: Kayra Cizmeci <kayracizmeci@gmail.com>
---
The patch needs curr to be invariant so I added the same
(cfs_rq->curr == se) to places in requeue_delayed_entity() and
and enqueue_task_fair(). Like after the place_entity() call in
requeue_delayed_entity(), or like before if (curr) the old place
of the calculation. And added WARN_ON_ONCE(curr != new_calc_curr)
or something like that I don't know how to say it normally.
Then I booted these changes on x86 (Zen 3)
called perf bench sched messaging with 200 groups and 5000 loops.
And then I make sure if the requeue_delayed_entity() was really working
with ftrace. The results were good but I left the computer on 2 more
hours and then checked the results again. It was still good, but
considering that I booted the kernel with busybox I don't think
much happened on that 2 hour window.
kernel/sched/fair.c | 36 ++++++++++++++++++++++--------------
1 file changed, 22 insertions(+), 14 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index b411384125ec..304ef70685d2 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6175,7 +6175,7 @@ void __setparam_fair(struct task_struct *p, const struct sched_attr *attr)
}
static void
-place_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
+place_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags, bool is_curr)
{
u64 vslice, vruntime = avg_vruntime(cfs_rq);
unsigned int nr_queued = cfs_rq->h_nr_queued;
@@ -6199,7 +6199,7 @@ place_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
*/
if (sched_feat(PLACE_LAG) && nr_queued && se->vlag) {
struct sched_entity *curr = cfs_rq->curr;
- long load, weight;
+ long load, weight, curr_weight;
lag = se->vlag;
@@ -6256,10 +6256,17 @@ place_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
* vl_i = (W + w_i)*vl'_i / W
*/
load = cfs_rq->sum_weight;
- if (curr && curr->on_rq)
- load += avg_vruntime_weight(cfs_rq, curr->h_load.weight);
+ if (curr) {
+ curr_weight = avg_vruntime_weight(cfs_rq, curr->h_load.weight);
+ if (curr->on_rq)
+ load += curr_weight;
+ }
+
+ if (is_curr)
+ weight = curr_weight;
+ else
+ weight = avg_vruntime_weight(cfs_rq, se->h_load.weight);
- weight = avg_vruntime_weight(cfs_rq, se->h_load.weight);
lag *= load + weight;
if (WARN_ON_ONCE(!load))
load = 1;
@@ -7900,7 +7907,7 @@ static int choose_idle_cpu(int cpu, struct task_struct *p)
}
static void
-requeue_delayed_entity(struct cfs_rq *cfs_rq, struct sched_entity *se)
+requeue_delayed_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, bool curr)
{
/*
* se->sched_delayed should imply: se->on_rq == 1.
@@ -7912,10 +7919,10 @@ requeue_delayed_entity(struct cfs_rq *cfs_rq, struct sched_entity *se)
if (update_entity_lag(cfs_rq, se)) {
cfs_rq->h_nr_queued--;
- if (se != cfs_rq->curr)
+ if (!curr)
__dequeue_entity(cfs_rq, se);
- place_entity(cfs_rq, se, 0);
- if (se != cfs_rq->curr)
+ place_entity(cfs_rq, se, 0, curr);
+ if (!curr)
__enqueue_entity(cfs_rq, se);
cfs_rq->h_nr_queued++;
}
@@ -8000,9 +8007,10 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
util_est_enqueue(cfs_rq, p);
update_curr_eevdf(cfs_rq);
+ curr = (cfs_rq->curr == se);
if (delayed) {
- requeue_delayed_entity(cfs_rq, se);
+ requeue_delayed_entity(cfs_rq, se, curr);
return;
}
@@ -8017,18 +8025,18 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
/*
* XXX comment on the curr thing
*/
- curr = (cfs_rq->curr == se);
+
if (curr)
- place_entity(cfs_rq, se, flags);
+ place_entity(cfs_rq, se, flags, curr);
if (se->on_rq && se->sched_delayed)
- requeue_delayed_entity(cfs_rq, se);
+ requeue_delayed_entity(cfs_rq, se, curr);
weight = enqueue_hierarchy(p, flags);
if (!curr) {
reweight_eevdf(cfs_rq, se, weight, false);
- place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
+ place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED, curr);
__enqueue_entity(cfs_rq, se);
}
--
2.53.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 0/2] sched/fair: reduce repeated work in enqueue path
2026-08-24 12:52 [PATCH 1/2] sched/fair: reuse the ENQUEUE_DELAYED calculation in enqueue_task_fair() Kayra Cizmeci
2026-08-24 12:52 ` [PATCH 2/2] sched/fair: avoid recalculating curr status in place_entity() and requeue_delayed_entity() Kayra Cizmeci
@ 2026-08-26 10:15 ` Kayra Cizmeci
2026-08-26 10:15 ` [PATCH v2 1/2] sched/fair: reuse the ENQUEUE_DELAYED calculation in enqueue_task_fair() Kayra Cizmeci
2026-08-26 10:15 ` [PATCH v2 2/2] sched/fair: avoid recalculating curr status in place_entity() and requeue_delayed_entity() Kayra Cizmeci
1 sibling, 2 replies; 20+ messages in thread
From: Kayra Cizmeci @ 2026-08-26 10:15 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, K Prateek Nayak
Cc: linux-kernel, Kayra Cizmeci
1/2: Gather the flags & DELAYED_QUEUE controls in one place.
2/2: give the calculated curr == se onto requeue_delayed_entity()
and place_entity()
Changelog:
1/2: No changes.
2/2: add (is_curr || curr->on_rq) condition to the
check for calculating curr_weight so when both of them are false the
curr_weight won't go to waste.
v1: https://lore.kernel.org/lkml/20260824125223.508178-1-kayracizmeci@gmail.com/
Kayra Cizmeci (2):
sched/fair: reuse the ENQUEUE_DELAYED calculation in
enqueue_task_fair()
sched/fair: avoid recalculating curr status in place_entity() and
requeue_delayed_entity()
kernel/sched/fair.c | 42 +++++++++++++++++++++++++-----------------
1 file changed, 25 insertions(+), 17 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 1/2] sched/fair: reuse the ENQUEUE_DELAYED calculation in enqueue_task_fair()
2026-08-26 10:15 ` [PATCH v2 0/2] sched/fair: reduce repeated work in enqueue path Kayra Cizmeci
@ 2026-08-26 10:15 ` Kayra Cizmeci
2026-08-26 10:41 ` K Prateek Nayak
2026-08-26 10:15 ` [PATCH v2 2/2] sched/fair: avoid recalculating curr status in place_entity() and requeue_delayed_entity() Kayra Cizmeci
1 sibling, 1 reply; 20+ messages in thread
From: Kayra Cizmeci @ 2026-08-26 10:15 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, K Prateek Nayak
Cc: linux-kernel, Kayra Cizmeci
Currently flags & ENQUEUE_DELAYED checks spread throughout the
enqueue_task_fair(), connect these checks to a bool 'delayed' that
gets calculated at the start of the function.
No functional change intended.
Signed-off-by: Kayra Cizmeci <kayracizmeci@gmail.com>
---
kernel/sched/fair.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 6d881e530f89..b411384125ec 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7985,7 +7985,7 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
struct sched_entity *se = &p->se;
struct cfs_rq *cfs_rq = &rq->cfs;
unsigned long weight;
- bool curr;
+ bool curr, delayed = (flags & ENQUEUE_DELAYED);
if (task_is_throttled(p) && enqueue_throttled_task(p))
return;
@@ -7996,12 +7996,12 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
* Let's add the task's estimated utilization to the cfs_rq's
* estimated utilization, before we update schedutil.
*/
- if (!p->se.sched_delayed || (flags & ENQUEUE_DELAYED))
+ if (!p->se.sched_delayed || delayed)
util_est_enqueue(cfs_rq, p);
update_curr_eevdf(cfs_rq);
- if (flags & ENQUEUE_DELAYED) {
+ if (delayed) {
requeue_delayed_entity(cfs_rq, se);
return;
}
--
2.53.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 2/2] sched/fair: avoid recalculating curr status in place_entity() and requeue_delayed_entity()
2026-08-26 10:15 ` [PATCH v2 0/2] sched/fair: reduce repeated work in enqueue path Kayra Cizmeci
2026-08-26 10:15 ` [PATCH v2 1/2] sched/fair: reuse the ENQUEUE_DELAYED calculation in enqueue_task_fair() Kayra Cizmeci
@ 2026-08-26 10:15 ` Kayra Cizmeci
2026-09-11 10:57 ` Kayra Cizmeci
` (2 more replies)
1 sibling, 3 replies; 20+ messages in thread
From: Kayra Cizmeci @ 2026-08-26 10:15 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, K Prateek Nayak
Cc: linux-kernel, Kayra Cizmeci
In enqueue_task_fair() a bool is calculated by cfs_rq->curr == se.
But this information gets recalculated on requeue_delayed_entity() and
requeue_delayed_entity() only gets called in enqueue_task_fair().
And on place_entity() if se == curr we call the avg_vruntime_weight()
twice with the same input. place_entity() only gets called in
enqueue_task_fair() and requeue_delayed_entity().
Use the information on enqueue_task_fair() in requeue_delayed_entity().
And place_entity() to avoid calling avg_vruntime_weight() twice.
Signed-off-by: Kayra Cizmeci <kayracizmeci@gmail.com>
---
The patch needs curr to be invariant so I added the same
(cfs_rq->curr == se) to places in requeue_delayed_entity() and
and enqueue_task_fair(). Like after the place_entity() call in
requeue_delayed_entity(), or like before if (curr) the old place
of the calculation. And added WARN_ON_ONCE(curr != new_calc_curr)
or something like that I don't know how to say it normally.
Then I booted these changes on x86 (Zen 3)
called perf bench sched messaging with 200 groups and 5000 loops.
And then I make sure if the requeue_delayed_entity() was really working
with ftrace. The results were good but I left the computer on 2 more
hours and then checked the results again. It was still good, but
considering that I booted the kernel with busybox I don't think
much happened on that 2 hour window.
kernel/sched/fair.c | 36 ++++++++++++++++++++++--------------
1 file changed, 22 insertions(+), 14 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index b411384125ec..f1b46fe26d12 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6175,7 +6175,7 @@ void __setparam_fair(struct task_struct *p, const struct sched_attr *attr)
}
static void
-place_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
+place_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags, bool is_curr)
{
u64 vslice, vruntime = avg_vruntime(cfs_rq);
unsigned int nr_queued = cfs_rq->h_nr_queued;
@@ -6199,7 +6199,7 @@ place_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
*/
if (sched_feat(PLACE_LAG) && nr_queued && se->vlag) {
struct sched_entity *curr = cfs_rq->curr;
- long load, weight;
+ long load, weight, curr_weight;
lag = se->vlag;
@@ -6256,10 +6256,17 @@ place_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
* vl_i = (W + w_i)*vl'_i / W
*/
load = cfs_rq->sum_weight;
- if (curr && curr->on_rq)
- load += avg_vruntime_weight(cfs_rq, curr->h_load.weight);
+ if (curr && (curr->on_rq || is_curr)) {
+ curr_weight = avg_vruntime_weight(cfs_rq, curr->h_load.weight);
+ if (curr->on_rq)
+ load += curr_weight;
+ }
+
+ if (is_curr)
+ weight = curr_weight;
+ else
+ weight = avg_vruntime_weight(cfs_rq, se->h_load.weight);
- weight = avg_vruntime_weight(cfs_rq, se->h_load.weight);
lag *= load + weight;
if (WARN_ON_ONCE(!load))
load = 1;
@@ -7900,7 +7907,7 @@ static int choose_idle_cpu(int cpu, struct task_struct *p)
}
static void
-requeue_delayed_entity(struct cfs_rq *cfs_rq, struct sched_entity *se)
+requeue_delayed_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, bool curr)
{
/*
* se->sched_delayed should imply: se->on_rq == 1.
@@ -7912,10 +7919,10 @@ requeue_delayed_entity(struct cfs_rq *cfs_rq, struct sched_entity *se)
if (update_entity_lag(cfs_rq, se)) {
cfs_rq->h_nr_queued--;
- if (se != cfs_rq->curr)
+ if (!curr)
__dequeue_entity(cfs_rq, se);
- place_entity(cfs_rq, se, 0);
- if (se != cfs_rq->curr)
+ place_entity(cfs_rq, se, 0, curr);
+ if (!curr)
__enqueue_entity(cfs_rq, se);
cfs_rq->h_nr_queued++;
}
@@ -8000,9 +8007,10 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
util_est_enqueue(cfs_rq, p);
update_curr_eevdf(cfs_rq);
+ curr = (cfs_rq->curr == se);
if (delayed) {
- requeue_delayed_entity(cfs_rq, se);
+ requeue_delayed_entity(cfs_rq, se, curr);
return;
}
@@ -8017,18 +8025,18 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
/*
* XXX comment on the curr thing
*/
- curr = (cfs_rq->curr == se);
+
if (curr)
- place_entity(cfs_rq, se, flags);
+ place_entity(cfs_rq, se, flags, curr);
if (se->on_rq && se->sched_delayed)
- requeue_delayed_entity(cfs_rq, se);
+ requeue_delayed_entity(cfs_rq, se, curr);
weight = enqueue_hierarchy(p, flags);
if (!curr) {
reweight_eevdf(cfs_rq, se, weight, false);
- place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
+ place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED, curr);
__enqueue_entity(cfs_rq, se);
}
--
2.53.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/2] sched/fair: reuse the ENQUEUE_DELAYED calculation in enqueue_task_fair()
2026-08-26 10:15 ` [PATCH v2 1/2] sched/fair: reuse the ENQUEUE_DELAYED calculation in enqueue_task_fair() Kayra Cizmeci
@ 2026-08-26 10:41 ` K Prateek Nayak
2026-08-26 18:44 ` Kayra Cizmeci
2026-09-07 16:05 ` [PATCH] sched/fair: Remove unused autogroup.h include Kayra Cizmeci
0 siblings, 2 replies; 20+ messages in thread
From: K Prateek Nayak @ 2026-08-26 10:41 UTC (permalink / raw)
To: Kayra Cizmeci, Ingo Molnar, Peter Zijlstra, Juri Lelli,
Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
Mel Gorman, Valentin Schneider
Cc: linux-kernel
On 8/26/2026 3:45 PM, Kayra Cizmeci wrote:
> @@ -7996,12 +7996,12 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> * Let's add the task's estimated utilization to the cfs_rq's
> * estimated utilization, before we update schedutil.
> */
> - if (!p->se.sched_delayed || (flags & ENQUEUE_DELAYED))
> + if (!p->se.sched_delayed || delayed)
nit. This reads funny now - not delayed or delayed?
Maybe wakeup_delayed but all of this should be optimized by compiler
at the end and a big ENQUEUE_DELAYED is better for humans who are
reading the code no?
> util_est_enqueue(cfs_rq, p);
>
> update_curr_eevdf(cfs_rq);
>
> - if (flags & ENQUEUE_DELAYED) {
> + if (delayed) {
> requeue_delayed_entity(cfs_rq, se);
> return;
> }
--
Thanks and Regards,
Prateek
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/2] sched/fair: reuse the ENQUEUE_DELAYED calculation in enqueue_task_fair()
2026-08-26 10:41 ` K Prateek Nayak
@ 2026-08-26 18:44 ` Kayra Cizmeci
2026-09-07 16:05 ` [PATCH] sched/fair: Remove unused autogroup.h include Kayra Cizmeci
1 sibling, 0 replies; 20+ messages in thread
From: Kayra Cizmeci @ 2026-08-26 18:44 UTC (permalink / raw)
To: kprateek.nayak
Cc: bsegall, dietmar.eggemann, juri.lelli, kayracizmeci,
linux-kernel, mgorman, mingo, peterz, rostedt, vincent.guittot,
vschneid
> Maybe wakeup_delayed but all of this should be optimized by compiler
> at the end and a big ENQUEUE_DELAYED is better for humans who are
> reading the code no?
Can't we rename it and use it in both places? If not then using ENQUEUE_DELAYED
is better since the approach remains the same in both places, no?
Thanks,
Kayra
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] sched/fair: Remove unused autogroup.h include
2026-08-26 10:41 ` K Prateek Nayak
2026-08-26 18:44 ` Kayra Cizmeci
@ 2026-09-07 16:05 ` Kayra Cizmeci
2026-09-07 16:14 ` [PATCH v2 1/2] sched/fair: reuse the ENQUEUE_DELAYED calculation in enqueue_task_fair() Kayra Cizmeci
1 sibling, 1 reply; 20+ messages in thread
From: Kayra Cizmeci @ 2026-09-07 16:05 UTC (permalink / raw)
To: kprateek.nayak
Cc: bsegall, dietmar.eggemann, juri.lelli, kayracizmeci,
linux-kernel, mgorman, mingo, peterz, rostedt, vincent.guittot,
vschneid
Gentle ping on this patch.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/2] sched/fair: reuse the ENQUEUE_DELAYED calculation in enqueue_task_fair()
2026-09-07 16:05 ` [PATCH] sched/fair: Remove unused autogroup.h include Kayra Cizmeci
@ 2026-09-07 16:14 ` Kayra Cizmeci
0 siblings, 0 replies; 20+ messages in thread
From: Kayra Cizmeci @ 2026-09-07 16:14 UTC (permalink / raw)
To: kayracizmeci
Cc: bsegall, dietmar.eggemann, juri.lelli, kprateek.nayak,
linux-kernel, mgorman, mingo, peterz, rostedt, vincent.guittot,
vschneid
Hello Prateek,
>> @@ -7996,12 +7996,12 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
>> * Let's add the task's estimated utilization to the cfs_rq's
>> * estimated utilization, before we update schedutil.
>> */
>> - if (!p->se.sched_delayed || (flags & ENQUEUE_DELAYED))
>> + if (!p->se.sched_delayed || delayed)
> nit. This reads funny now - not delayed or delayed?
> Maybe wakeup_delayed but all of this should be optimized by compiler
> at the end and a big ENQUEUE_DELAYED is better for humans who are
> reading the code no?
In my first message I was thinking that renaming and using it in both places would be
the better approach. But I thought about this the meantime and I changed my mind.
flags & ENQUEUE_DELAYED reads better and more clear than a bool. And I can't really
see a big advantage of renaming it over this version.
Patch subject is a bit confusing since it says reuse the bla bla
calculation in the enqueue_task_fair(). And this subject
makes it seem like there is a performance claim.
I knew It was getting optimized by the compiler, I thought at the time that
gathering this flags & ENQUEUE_DELAYED in one place would be better.
I'm dropping this patch (1/2) but I'll continue with 2/2 :->.
NOTE: I send the wrong file that was with the same
name with my correct file that I supposed to send. Sorry for this autogroup.h
ping thing. Ah..
Thanks,
Kayra
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/2] sched/fair: avoid recalculating curr status in place_entity() and requeue_delayed_entity()
2026-08-26 10:15 ` [PATCH v2 2/2] sched/fair: avoid recalculating curr status in place_entity() and requeue_delayed_entity() Kayra Cizmeci
@ 2026-09-11 10:57 ` Kayra Cizmeci
2026-09-11 12:19 ` Vincent Guittot
2026-09-11 12:42 ` Peter Zijlstra
2 siblings, 0 replies; 20+ messages in thread
From: Kayra Cizmeci @ 2026-09-11 10:57 UTC (permalink / raw)
To: kayracizmeci
Cc: bsegall, dietmar.eggemann, juri.lelli, kprateek.nayak,
linux-kernel, mgorman, mingo, peterz, rostedt, vincent.guittot,
vschneid
Gentle Ping on this patch, :_:
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/2] sched/fair: avoid recalculating curr status in place_entity() and requeue_delayed_entity()
2026-08-26 10:15 ` [PATCH v2 2/2] sched/fair: avoid recalculating curr status in place_entity() and requeue_delayed_entity() Kayra Cizmeci
2026-09-11 10:57 ` Kayra Cizmeci
@ 2026-09-11 12:19 ` Vincent Guittot
2026-09-11 13:10 ` Kayra Cizmeci
2026-09-11 12:42 ` Peter Zijlstra
2 siblings, 1 reply; 20+ messages in thread
From: Vincent Guittot @ 2026-09-11 12:19 UTC (permalink / raw)
To: Kayra Cizmeci
Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
K Prateek Nayak, linux-kernel
On Wed, 26 Aug 2026 at 12:18, Kayra Cizmeci <kayracizmeci@gmail.com> wrote:
>
> In enqueue_task_fair() a bool is calculated by cfs_rq->curr == se.
> But this information gets recalculated on requeue_delayed_entity() and
> requeue_delayed_entity() only gets called in enqueue_task_fair().
> And on place_entity() if se == curr we call the avg_vruntime_weight()
> twice with the same input. place_entity() only gets called in
> enqueue_task_fair() and requeue_delayed_entity().
>
> Use the information on enqueue_task_fair() in requeue_delayed_entity().
> And place_entity() to avoid calling avg_vruntime_weight() twice.
>
> Signed-off-by: Kayra Cizmeci <kayracizmeci@gmail.com>
> ---
> The patch needs curr to be invariant so I added the same
> (cfs_rq->curr == se) to places in requeue_delayed_entity() and
> and enqueue_task_fair(). Like after the place_entity() call in
> requeue_delayed_entity(), or like before if (curr) the old place
> of the calculation. And added WARN_ON_ONCE(curr != new_calc_curr)
> or something like that I don't know how to say it normally.
>
> Then I booted these changes on x86 (Zen 3)
> called perf bench sched messaging with 200 groups and 5000 loops.
> And then I make sure if the requeue_delayed_entity() was really working
> with ftrace. The results were good but I left the computer on 2 more
> hours and then checked the results again. It was still good, but
> considering that I booted the kernel with busybox I don't think
> much happened on that 2 hour window.
>
> kernel/sched/fair.c | 36 ++++++++++++++++++++++--------------
> 1 file changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index b411384125ec..f1b46fe26d12 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -6175,7 +6175,7 @@ void __setparam_fair(struct task_struct *p, const struct sched_attr *attr)
> }
>
> static void
> -place_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
> +place_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags, bool is_curr)
> {
> u64 vslice, vruntime = avg_vruntime(cfs_rq);
> unsigned int nr_queued = cfs_rq->h_nr_queued;
> @@ -6199,7 +6199,7 @@ place_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
> */
> if (sched_feat(PLACE_LAG) && nr_queued && se->vlag) {
> struct sched_entity *curr = cfs_rq->curr;
> - long load, weight;
> + long load, weight, curr_weight;
>
> lag = se->vlag;
>
> @@ -6256,10 +6256,17 @@ place_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
> * vl_i = (W + w_i)*vl'_i / W
> */
> load = cfs_rq->sum_weight;
> - if (curr && curr->on_rq)
> - load += avg_vruntime_weight(cfs_rq, curr->h_load.weight);
> + if (curr && (curr->on_rq || is_curr)) {
> + curr_weight = avg_vruntime_weight(cfs_rq, curr->h_load.weight);
> + if (curr->on_rq)
> + load += curr_weight;
> + }
> +
> + if (is_curr)
> + weight = curr_weight;
> + else
> + weight = avg_vruntime_weight(cfs_rq, se->h_load.weight);
This makes the code less readable and I don't think it gives any
measurable benefit.
>
> - weight = avg_vruntime_weight(cfs_rq, se->h_load.weight);
> lag *= load + weight;
> if (WARN_ON_ONCE(!load))
> load = 1;
> @@ -7900,7 +7907,7 @@ static int choose_idle_cpu(int cpu, struct task_struct *p)
> }
>
> static void
> -requeue_delayed_entity(struct cfs_rq *cfs_rq, struct sched_entity *se)
> +requeue_delayed_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, bool curr)
> {
> /*
> * se->sched_delayed should imply: se->on_rq == 1.
> @@ -7912,10 +7919,10 @@ requeue_delayed_entity(struct cfs_rq *cfs_rq, struct sched_entity *se)
>
> if (update_entity_lag(cfs_rq, se)) {
> cfs_rq->h_nr_queued--;
> - if (se != cfs_rq->curr)
> + if (!curr)
> __dequeue_entity(cfs_rq, se);
> - place_entity(cfs_rq, se, 0);
> - if (se != cfs_rq->curr)
> + place_entity(cfs_rq, se, 0, curr);
> + if (!curr)
> __enqueue_entity(cfs_rq, se);
> cfs_rq->h_nr_queued++;
> }
> @@ -8000,9 +8007,10 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> util_est_enqueue(cfs_rq, p);
>
> update_curr_eevdf(cfs_rq);
> + curr = (cfs_rq->curr == se);
>
> if (delayed) {
> - requeue_delayed_entity(cfs_rq, se);
> + requeue_delayed_entity(cfs_rq, se, curr);
> return;
> }
>
> @@ -8017,18 +8025,18 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> /*
> * XXX comment on the curr thing
> */
> - curr = (cfs_rq->curr == se);
> +
> if (curr)
> - place_entity(cfs_rq, se, flags);
> + place_entity(cfs_rq, se, flags, curr);
>
> if (se->on_rq && se->sched_delayed)
> - requeue_delayed_entity(cfs_rq, se);
> + requeue_delayed_entity(cfs_rq, se, curr);
>
> weight = enqueue_hierarchy(p, flags);
>
> if (!curr) {
> reweight_eevdf(cfs_rq, se, weight, false);
> - place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
> + place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED, curr);
> __enqueue_entity(cfs_rq, se);
> }
>
> --
> 2.53.0
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/2] sched/fair: avoid recalculating curr status in place_entity() and requeue_delayed_entity()
2026-08-26 10:15 ` [PATCH v2 2/2] sched/fair: avoid recalculating curr status in place_entity() and requeue_delayed_entity() Kayra Cizmeci
2026-09-11 10:57 ` Kayra Cizmeci
2026-09-11 12:19 ` Vincent Guittot
@ 2026-09-11 12:42 ` Peter Zijlstra
2026-09-11 14:08 ` Kayra Cizmeci
2026-09-11 16:02 ` [PATCH v3] sched/fair: remove dead code on enqueue_task_fair() Kayra Cizmeci
2 siblings, 2 replies; 20+ messages in thread
From: Peter Zijlstra @ 2026-09-11 12:42 UTC (permalink / raw)
To: Kayra Cizmeci
Cc: Ingo Molnar, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
K Prateek Nayak, linux-kernel
On Wed, Aug 26, 2026 at 01:15:42PM +0300, Kayra Cizmeci wrote:
> In enqueue_task_fair() a bool is calculated by cfs_rq->curr == se.
> But this information gets recalculated on requeue_delayed_entity() and
> requeue_delayed_entity() only gets called in enqueue_task_fair().
> And on place_entity() if se == curr we call the avg_vruntime_weight()
> twice with the same input. place_entity() only gets called in
> enqueue_task_fair() and requeue_delayed_entity().
I found it very hard to follow your Changelog. Could be my Friday brain.
Sorry if I misunderstood.
> @@ -8000,9 +8007,10 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> util_est_enqueue(cfs_rq, p);
>
> update_curr_eevdf(cfs_rq);
> + curr = (cfs_rq->curr == se);
>
> if (delayed) {
> - requeue_delayed_entity(cfs_rq, se);
> + requeue_delayed_entity(cfs_rq, se, curr);
> return;
> }
>
> @@ -8017,18 +8025,18 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> /*
> * XXX comment on the curr thing
> */
> - curr = (cfs_rq->curr == se);
> +
> if (curr)
> - place_entity(cfs_rq, se, flags);
> + place_entity(cfs_rq, se, flags, curr);
As I argued here:
https://patch.msgid.link/20260813103155.GC1246887%40noisy.programming.kicks-ass.net
I suspect that case is impossible. Which AFAICT renders your whole patch
one big no-op, no?
That is, I think we want to do the below.
---
kernel/sched/fair.c | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 4d0b94465d19..3c7fb0684a76 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7996,7 +7996,6 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
struct sched_entity *se = &p->se;
struct cfs_rq *cfs_rq = &rq->cfs;
unsigned long weight;
- bool curr;
if (task_is_throttled(p) && enqueue_throttled_task(p))
return;
@@ -8025,23 +8024,14 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
if (p->in_iowait)
cpufreq_update_util(rq, SCHED_CPUFREQ_IOWAIT);
- /*
- * XXX comment on the curr thing
- */
- curr = (cfs_rq->curr == se);
- if (curr)
- place_entity(cfs_rq, se, flags);
if (se->on_rq && se->sched_delayed)
requeue_delayed_entity(cfs_rq, se);
weight = enqueue_hierarchy(p, flags);
-
- if (!curr) {
- reweight_eevdf(cfs_rq, se, weight, false);
- place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
- __enqueue_entity(cfs_rq, se);
- }
+ reweight_eevdf(cfs_rq, se, weight, false);
+ place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
+ __enqueue_entity(cfs_rq, se);
if (!rq_h_nr_queued && rq->cfs.h_nr_queued)
dl_server_start(&rq->fair_server);
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/2] sched/fair: avoid recalculating curr status in place_entity() and requeue_delayed_entity()
2026-09-11 12:19 ` Vincent Guittot
@ 2026-09-11 13:10 ` Kayra Cizmeci
0 siblings, 0 replies; 20+ messages in thread
From: Kayra Cizmeci @ 2026-09-11 13:10 UTC (permalink / raw)
To: vincent.guittot
Cc: bsegall, dietmar.eggemann, juri.lelli, kayracizmeci,
kprateek.nayak, linux-kernel, mgorman, mingo, peterz, rostedt,
vschneid
Hello Vincent,
>> @@ -6256,10 +6256,17 @@ place_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
>> * vl_i = (W + w_i)*vl'_i / W
>> */
>> load = cfs_rq->sum_weight;
>> - if (curr && curr->on_rq)
>> - load += avg_vruntime_weight(cfs_rq, curr->h_load.weight);
>> + if (curr && (curr->on_rq || is_curr)) {
>> + curr_weight = avg_vruntime_weight(cfs_rq, curr->h_load.weight);
>> + if (curr->on_rq)
>> + load += curr_weight;
>> + }
>> +
>> + if (is_curr)
>> + weight = curr_weight;
>> + else
>> + weight = avg_vruntime_weight(cfs_rq, se->h_load.weight);
> This makes the code less readable and I don't think it gives any
> measurable benefit.
Well, yeah. 1 avg_vruntime_weight() less or much is not measurable
in any means.
The main thing I wanted was to handle curr == se case
on here.
Maybe we could remove this is_curr bool and look se == curr
in function, and remove this curr weight and let the curr use weight
instead. So, the curr_weightnbecomes weight, we could be done
with the if (is_curr) block since we don't need to set weight to curr_weight.
Well if you think handling curr == se separately is wrong, then I'll drop the patch.
Please let me know what you're thinking :-).
Thanks,
Kayra :_:
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/2] sched/fair: avoid recalculating curr status in place_entity() and requeue_delayed_entity()
2026-09-11 12:42 ` Peter Zijlstra
@ 2026-09-11 14:08 ` Kayra Cizmeci
2026-09-11 16:02 ` [PATCH v3] sched/fair: remove dead code on enqueue_task_fair() Kayra Cizmeci
1 sibling, 0 replies; 20+ messages in thread
From: Kayra Cizmeci @ 2026-09-11 14:08 UTC (permalink / raw)
To: peterz
Cc: bsegall, dietmar.eggemann, juri.lelli, kayracizmeci,
kprateek.nayak, linux-kernel, mgorman, mingo, rostedt,
vincent.guittot, vschneid
Hello Peter :>,
>> In enqueue_task_fair() a bool is calculated by cfs_rq->curr == se.
>> But this information gets recalculated on requeue_delayed_entity() and
>> requeue_delayed_entity() only gets called in enqueue_task_fair().
>> And on place_entity() if se == curr we call the avg_vruntime_weight()
>> twice with the same input. place_entity() only gets called in
>> enqueue_task_fair() and requeue_delayed_entity().
> I found it very hard to follow your Changelog. Could be my Friday brain.
> Sorry if I misunderstood.
It's not your Friday brain, it's my talking skills ;>.
> As I argued here:
> https://patch.msgid.link/20260813103155.GC1246887%40noisy.programming.kicks-ass.net
Oh, I remembered that patch. 1 month ago. But I didn't look into it in detail at the time.
> I suspect that case is impossible. Which AFAICT renders your whole patch
> one big no-op, no?
Well, I run some tests with WARN_ON_ONCE and that case never worked. Atleast on my tests and on my machine. I didn't done so many tests tho.
I remember doing some WARN_ON_ONCE tests on v1 too. But I don't think I did
a test about curr == se, since I thought there was no way that it does not work. Well... :P.
> That is, I think we want to do the below.
> ---
> kernel/sched/fair.c | 16 +++-------------
> 1 file changed, 3 insertions(+), 13 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 4d0b94465d19..3c7fb0684a76 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -7996,7 +7996,6 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> struct sched_entity *se = &p->se;
> struct cfs_rq *cfs_rq = &rq->cfs;
> unsigned long weight;
> - bool curr;
>
> if (task_is_throttled(p) && enqueue_throttled_task(p))
> return;
> @@ -8025,23 +8024,14 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> if (p->in_iowait)
> cpufreq_update_util(rq, SCHED_CPUFREQ_IOWAIT);
>
> - /*
> - * XXX comment on the curr thing
> - */
> - curr = (cfs_rq->curr == se);
> - if (curr)
> - place_entity(cfs_rq, se, flags);
>
> if (se->on_rq && se->sched_delayed)
> requeue_delayed_entity(cfs_rq, se);
>
> weight = enqueue_hierarchy(p, flags);
> -
> - if (!curr) {
> - reweight_eevdf(cfs_rq, se, weight, false);
> - place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
> - __enqueue_entity(cfs_rq, se);
> - }
> + reweight_eevdf(cfs_rq, se, weight, false);
> + place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
> + __enqueue_entity(cfs_rq, se);
if (!rq_h_nr_queued && rq->cfs.h_nr_queued)
dl_server_start(&rq->fair_server);
So, you say remove all of this curr crap and get everything in one place since the
curr = (cfs_rq->curr == se) returns false all the time? That way we wouldn't need to
worry about the place_entity() parts neither. I'm OK. I'll send the v3 today or tommorow
latest.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3] sched/fair: remove dead code on enqueue_task_fair()
2026-09-11 12:42 ` Peter Zijlstra
2026-09-11 14:08 ` Kayra Cizmeci
@ 2026-09-11 16:02 ` Kayra Cizmeci
2026-10-12 12:58 ` Christian Loehle
1 sibling, 1 reply; 20+ messages in thread
From: Kayra Cizmeci @ 2026-09-11 16:02 UTC (permalink / raw)
To: peterz, Ingo Molnar, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, K Prateek Nayak
Cc: kayracizmeci, linux-kernel
cfs_rq->curr == se path on enqueue_task_fair() seems to be unachievable.
Remove the code that depends on this cfs_rq->curr == se case on
enqueue_task_fair() and bool curr too since there are two possibilities
and one has been removed, we can proceed directly with the other.
Signed-off-by: Kayra Cizmeci <kayracizmeci@gmail.com>
---
Ah..
I forgot to add v3. Sorry of the other one.
kernel/sched/fair.c | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index ade1eceb39b8..440e4fdebe2f 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7985,7 +7985,6 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
struct sched_entity *se = &p->se;
struct cfs_rq *cfs_rq = &rq->cfs;
unsigned long weight;
- bool curr;
if (task_is_throttled(p) && enqueue_throttled_task(p))
return;
@@ -8014,23 +8013,14 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
if (p->in_iowait)
cpufreq_update_util(rq, SCHED_CPUFREQ_IOWAIT);
- /*
- * XXX comment on the curr thing
- */
- curr = (cfs_rq->curr == se);
- if (curr)
- place_entity(cfs_rq, se, flags);
if (se->on_rq && se->sched_delayed)
requeue_delayed_entity(cfs_rq, se);
weight = enqueue_hierarchy(p, flags);
-
- if (!curr) {
- reweight_eevdf(cfs_rq, se, weight, false);
- place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
- __enqueue_entity(cfs_rq, se);
- }
+ reweight_eevdf(cfs_rq, se, weight, false);
+ place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
+ __enqueue_entity(cfs_rq, se);
if (!rq_h_nr_queued && rq->cfs.h_nr_queued)
dl_server_start(&rq->fair_server);
--
2.53.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3] sched/fair: remove dead code on enqueue_task_fair()
2026-09-11 16:02 ` [PATCH v3] sched/fair: remove dead code on enqueue_task_fair() Kayra Cizmeci
@ 2026-10-12 12:58 ` Christian Loehle
2026-10-12 13:33 ` Kayra Cizmeci
2026-10-12 14:02 ` Peter Zijlstra
0 siblings, 2 replies; 20+ messages in thread
From: Christian Loehle @ 2026-10-12 12:58 UTC (permalink / raw)
To: Kayra Cizmeci, peterz, Ingo Molnar, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, K Prateek Nayak
Cc: linux-kernel
On 9/11/26 17:02, Kayra Cizmeci wrote:
> cfs_rq->curr == se path on enqueue_task_fair() seems to be unachievable.
Sashiko disagrees so I had a look and indeed cfs_rq unthrottling reaches this:
https://sashiko.dev/#/patchset/179033365207.2819794.11851486661940182495.tip-bot2%40tip-bot2
See the splat below.
I'm afraid we need to revert
819224e506bc ("sched/fair: Remove dead code on enqueue_task_fair()")
on tip/sched/core.
I guess it's non-obvious enough to deserve a comment...
[ 95.833742] ------------[ cut here ]------------
[ 95.833749] WARNING: kernel/sched/fair.c:8028 at enqueue_task_fair+0x7a8/0xa40, CPU#10: python3/6699
[ 95.847486] Modules linked in: qrtr 8021q garp mrp stp llc cppc_cpufreq cdns3_pci_wrap binfmt_misc nf_tables fuse dm_mod r8169
[ 95.858876] CPU: 10 UID: 0 PID: 6699 Comm: python3 Tainted: G S 7.3.0-rc1-mpam-rt-orion-20261008+ #2 PREEMPT_RT
[ 95.870517] Tainted: [S]=CPU_OUT_OF_SPEC
[ 95.874428] Hardware name: Radxa Computer (Shenzhen) Co., Ltd. Radxa Orion O6/Radxa Orion O6, BIOS 1.1.0-1 2025-12-25T02:55:53+00:00
[ 95.886324] pstate: 634000c9 (nZCv daIF +PAN -UAO +TCO +DIT -SSBS BTYPE=--)
[ 95.893275] pc : enqueue_task_fair+0x7a8/0xa40
[ 95.897705] lr : enqueue_task_fair+0x324/0xa40
[ 95.902136] sp : ffff800090323a50
[ 95.905438] x29: ffff800090323a50 x28: 0000000000000009 x27: ffffd8536a9cc5c0
[ 95.912565] x26: fffffdffbf43e1c0 x25: ffff0001fb7729c0 x24: fffffdffbf43e390
[ 95.919692] x23: 0000000000000001 x22: ffff000085d52400 x21: 0000000000000000
[ 95.926817] x20: ffff000085d52380 x19: ffff0001fb7727c0 x18: 0000000000000000
[ 95.933943] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000037d54fe0
[ 95.941069] x14: 0000000000000000 x13: ffffffffffffffff x12: ffffd8536a9cbdf8
[ 95.948195] x11: 00000000000000c0 x10: 02ae55ef2a914684 x9 : ffff00008d3511c0
[ 95.955321] x8 : 0000000000000001 x7 : 0000004100000000 x6 : ffffd8536a9cc5c0
[ 95.962442] x5 : ffff800090323ad8 x4 : ffff800090323ad8 x3 : ffff000085d527b8
[ 95.969563] x2 : 0000000000000001 x1 : ffff000085d52400 x0 : ffff000085d52400
[ 95.976689] Call trace:
[ 95.979124] enqueue_task_fair+0x7a8/0xa40 (P)
[ 95.983559] tg_unthrottle_up+0x14c/0x318
[ 95.987561] walk_tg_tree_from+0x60/0xb8
[ 95.991481] unthrottle_cfs_rq+0xa8/0x204
[ 95.995484] tg_set_bandwidth+0x36c/0x4c4
[ 95.999485] cpu_max_write+0xc8/0x130
[ 96.003138] cgroup_file_write+0xa4/0x194
[ 96.007150] kernfs_fop_write_iter+0x130/0x1fc
[ 96.011598] vfs_write+0x228/0x358
[ 96.015001] ksys_write+0x6c/0x104
[ 96.018395] __arm64_sys_write+0x18/0x24
[ 96.022309] invoke_syscall+0x54/0x110
[ 96.026051] el0_svc_common.constprop.0+0x40/0xe0
[ 96.030744] do_el0_svc+0x1c/0x28
[ 96.034048] el0_svc+0x38/0x260
[ 96.037183] el0t_64_sync_handler+0xa0/0xe4
[ 96.041357] el0t_64_sync+0x17c/0x180
[ 96.045011] ---[ end trace 0000000000000000 ]---
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3] sched/fair: remove dead code on enqueue_task_fair()
2026-10-12 12:58 ` Christian Loehle
@ 2026-10-12 13:33 ` Kayra Cizmeci
2026-10-12 14:02 ` Peter Zijlstra
1 sibling, 0 replies; 20+ messages in thread
From: Kayra Cizmeci @ 2026-10-12 13:33 UTC (permalink / raw)
To: christian.loehle
Cc: bsegall, dietmar.eggemann, juri.lelli, kayracizmeci,
kprateek.nayak, linux-kernel, mgorman, mingo, peterz, rostedt,
vincent.guittot, vschneid
> On 9/11/26 17:02, Kayra Cizmeci wrote:
> > cfs_rq->curr == se path on enqueue_task_fair() seems to be unachievable.
> Sashiko disagrees so I had a look and indeed cfs_rq unthrottling reaches this:
> https://sashiko.dev/#/patchset/179033365207.2819794.11851486661940182495.tip-bot2%40tip-bot2
> See the splat below.
> I'm afraid we need to revert
> 819224e506bc ("sched/fair: Remove dead code on enqueue_task_fair()")
> on tip/sched/core.
Ah... Fuck. Yeah we need to. Thanks for catching this early.
> I guess it's non-obvious enough to deserve a comment...
Probably...
Thanks,
Kayra
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3] sched/fair: remove dead code on enqueue_task_fair()
2026-10-12 12:58 ` Christian Loehle
2026-10-12 13:33 ` Kayra Cizmeci
@ 2026-10-12 14:02 ` Peter Zijlstra
2026-10-12 14:15 ` Kayra Cizmeci
2026-10-12 14:59 ` Christian Loehle
1 sibling, 2 replies; 20+ messages in thread
From: Peter Zijlstra @ 2026-10-12 14:02 UTC (permalink / raw)
To: Christian Loehle
Cc: Kayra Cizmeci, Ingo Molnar, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, K Prateek Nayak, linux-kernel
On Mon, Oct 12, 2026 at 01:58:36PM +0100, Christian Loehle wrote:
> On 9/11/26 17:02, Kayra Cizmeci wrote:
> > cfs_rq->curr == se path on enqueue_task_fair() seems to be unachievable.
>
> Sashiko disagrees so I had a look and indeed cfs_rq unthrottling reaches this:
> https://sashiko.dev/#/patchset/179033365207.2819794.11851486661940182495.tip-bot2%40tip-bot2
> See the splat below.
>
> I'm afraid we need to revert
> 819224e506bc ("sched/fair: Remove dead code on enqueue_task_fair()")
> on tip/sched/core.
>
> I guess it's non-obvious enough to deserve a comment...
>
> [ 95.833742] ------------[ cut here ]------------
> [ 95.833749] WARNING: kernel/sched/fair.c:8028 at enqueue_task_fair+0x7a8/0xa40, CPU#10: python3/6699
> [ 95.847486] Modules linked in: qrtr 8021q garp mrp stp llc cppc_cpufreq cdns3_pci_wrap binfmt_misc nf_tables fuse dm_mod r8169
> [ 95.858876] CPU: 10 UID: 0 PID: 6699 Comm: python3 Tainted: G S 7.3.0-rc1-mpam-rt-orion-20261008+ #2 PREEMPT_RT
> [ 95.870517] Tainted: [S]=CPU_OUT_OF_SPEC
> [ 95.874428] Hardware name: Radxa Computer (Shenzhen) Co., Ltd. Radxa Orion O6/Radxa Orion O6, BIOS 1.1.0-1 2025-12-25T02:55:53+00:00
> [ 95.886324] pstate: 634000c9 (nZCv daIF +PAN -UAO +TCO +DIT -SSBS BTYPE=--)
> [ 95.893275] pc : enqueue_task_fair+0x7a8/0xa40
> [ 95.897705] lr : enqueue_task_fair+0x324/0xa40
> [ 95.902136] sp : ffff800090323a50
> [ 95.905438] x29: ffff800090323a50 x28: 0000000000000009 x27: ffffd8536a9cc5c0
> [ 95.912565] x26: fffffdffbf43e1c0 x25: ffff0001fb7729c0 x24: fffffdffbf43e390
> [ 95.919692] x23: 0000000000000001 x22: ffff000085d52400 x21: 0000000000000000
> [ 95.926817] x20: ffff000085d52380 x19: ffff0001fb7727c0 x18: 0000000000000000
> [ 95.933943] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000037d54fe0
> [ 95.941069] x14: 0000000000000000 x13: ffffffffffffffff x12: ffffd8536a9cbdf8
> [ 95.948195] x11: 00000000000000c0 x10: 02ae55ef2a914684 x9 : ffff00008d3511c0
> [ 95.955321] x8 : 0000000000000001 x7 : 0000004100000000 x6 : ffffd8536a9cc5c0
> [ 95.962442] x5 : ffff800090323ad8 x4 : ffff800090323ad8 x3 : ffff000085d527b8
> [ 95.969563] x2 : 0000000000000001 x1 : ffff000085d52400 x0 : ffff000085d52400
> [ 95.976689] Call trace:
> [ 95.979124] enqueue_task_fair+0x7a8/0xa40 (P)
> [ 95.983559] tg_unthrottle_up+0x14c/0x318
> [ 95.987561] walk_tg_tree_from+0x60/0xb8
> [ 95.991481] unthrottle_cfs_rq+0xa8/0x204
> [ 95.995484] tg_set_bandwidth+0x36c/0x4c4
> [ 95.999485] cpu_max_write+0xc8/0x130
> [ 96.003138] cgroup_file_write+0xa4/0x194
> [ 96.007150] kernfs_fop_write_iter+0x130/0x1fc
> [ 96.011598] vfs_write+0x228/0x358
> [ 96.015001] ksys_write+0x6c/0x104
> [ 96.018395] __arm64_sys_write+0x18/0x24
> [ 96.022309] invoke_syscall+0x54/0x110
> [ 96.026051] el0_svc_common.constprop.0+0x40/0xe0
> [ 96.030744] do_el0_svc+0x1c/0x28
> [ 96.034048] el0_svc+0x38/0x260
> [ 96.037183] el0t_64_sync_handler+0xa0/0xe4
> [ 96.041357] el0t_64_sync+0x17c/0x180
> [ 96.045011] ---[ end trace 0000000000000000 ]---
Argh, so this is bandwidth. Task gets throttled, but before it gets back
to userspace to actually dequeue, it gets hit by a wakeup again while it
is still current?
Hmm, no, in that case it can't actually be on throttled_tasks. So it did
get dequeued, but it never quite scheduled out.
I hate this bandwidth stuff :-)
Anyway, this is different from the initial use-case, where a
sched_entity would get dequeue and re-enqueued while walking the cgroup
hierarchy, and thus had to appear like nothing happened in between.
In this case we're fine to advance time.
Anyway... the normal block/wakeup paths are >< close to also hitting
this in the case where balancing drops the rq->lock after dequeue. The
only reason ttwu can't tickle this is because it has that ->on_cpu
stuff. However, we cannot use that on the unthrottle path :/
Bah.
Does this work?
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 5c98d8dfce5d..0edd8bfb6363 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8250,7 +8250,14 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
weight = enqueue_hierarchy(p, flags);
reweight_eevdf(cfs_rq, se, weight, false);
place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
- __enqueue_entity(cfs_rq, se);
+
+ /*
+ * This can actually happen with CFS_BANDWIDTH, in that case a task
+ * can get throttled, dequeued and then unthrottled, woken *before* it
+ * schedules out and does put_prev_task().
+ */
+ if (cfs_rq->curr != se)
+ __enqueue_entity(cfs_rq, se);
if (!rq_h_nr_queued && rq->cfs.h_nr_queued)
dl_server_start(&rq->fair_server);
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3] sched/fair: remove dead code on enqueue_task_fair()
2026-10-12 14:02 ` Peter Zijlstra
@ 2026-10-12 14:15 ` Kayra Cizmeci
2026-10-12 14:59 ` Christian Loehle
1 sibling, 0 replies; 20+ messages in thread
From: Kayra Cizmeci @ 2026-10-12 14:15 UTC (permalink / raw)
To: peterz
Cc: bsegall, christian.loehle, dietmar.eggemann, juri.lelli,
kayracizmeci, kprateek.nayak, linux-kernel, mgorman, mingo,
rostedt, vincent.guittot, vschneid
> Argh, so this is bandwidth. Task gets throttled, but before it gets back
> to userspace to actually dequeue, it gets hit by a wakeup again while it
> is still current?
> Hmm, no, in that case it can't actually be on throttled_tasks. So it did
> get dequeued, but it never quite scheduled out.
> I hate this bandwidth stuff :-)
Ask me...
> Anyway, this is different from the initial use-case, where a
> sched_entity would get dequeue and re-enqueued while walking the cgroup
> hierarchy, and thus had to appear like nothing happened in between.
> In this case we're fine to advance time.
> Anyway... the normal block/wakeup paths are >< close to also hitting
> this in the case where balancing drops the rq->lock after dequeue. The
> only reason ttwu can't tickle this is because it has that ->on_cpu
> stuff. However, we cannot use that on the unthrottle path :/
> Bah.
> Does this work?
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 5c98d8dfce5d..0edd8bfb6363 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -8250,7 +8250,14 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> weight = enqueue_hierarchy(p, flags);
> reweight_eevdf(cfs_rq, se, weight, false);
> place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
> - __enqueue_entity(cfs_rq, se);
> +
> + /*
> + * This can actually happen with CFS_BANDWIDTH, in that case a task
> + * can get throttled, dequeued and then unthrottled, woken *before* it
> + * schedules out and does put_prev_task().
> + */
> + if (cfs_rq->curr != se)
> + __enqueue_entity(cfs_rq, se);
>
> if (!rq_h_nr_queued && rq->cfs.h_nr_queued)
> dl_server_start(&rq->fair_server);
This burns my brain. I *think* it should work.
But at this point, I don't think me thinking
alone is enough on this subject. So,
I'll try to test this... I'll post
the results tommorow or today if I even could.
Thanks!
Kayra
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3] sched/fair: remove dead code on enqueue_task_fair()
2026-10-12 14:02 ` Peter Zijlstra
2026-10-12 14:15 ` Kayra Cizmeci
@ 2026-10-12 14:59 ` Christian Loehle
1 sibling, 0 replies; 20+ messages in thread
From: Christian Loehle @ 2026-10-12 14:59 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Kayra Cizmeci, Ingo Molnar, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, K Prateek Nayak, linux-kernel
On 10/12/26 15:02, Peter Zijlstra wrote:
> On Mon, Oct 12, 2026 at 01:58:36PM +0100, Christian Loehle wrote:
>> On 9/11/26 17:02, Kayra Cizmeci wrote:
>>> cfs_rq->curr == se path on enqueue_task_fair() seems to be unachievable.
>>
>> Sashiko disagrees so I had a look and indeed cfs_rq unthrottling reaches this:
>> https://sashiko.dev/#/patchset/179033365207.2819794.11851486661940182495.tip-bot2%40tip-bot2
>> See the splat below.
>>
>> I'm afraid we need to revert
>> 819224e506bc ("sched/fair: Remove dead code on enqueue_task_fair()")
>> on tip/sched/core.
>>
>> I guess it's non-obvious enough to deserve a comment...
>>
>> [ 95.833742] ------------[ cut here ]------------
>> [ 95.833749] WARNING: kernel/sched/fair.c:8028 at enqueue_task_fair+0x7a8/0xa40, CPU#10: python3/6699
>> [ 95.847486] Modules linked in: qrtr 8021q garp mrp stp llc cppc_cpufreq cdns3_pci_wrap binfmt_misc nf_tables fuse dm_mod r8169
>> [ 95.858876] CPU: 10 UID: 0 PID: 6699 Comm: python3 Tainted: G S 7.3.0-rc1-mpam-rt-orion-20261008+ #2 PREEMPT_RT
>> [ 95.870517] Tainted: [S]=CPU_OUT_OF_SPEC
>> [ 95.874428] Hardware name: Radxa Computer (Shenzhen) Co., Ltd. Radxa Orion O6/Radxa Orion O6, BIOS 1.1.0-1 2025-12-25T02:55:53+00:00
>> [ 95.886324] pstate: 634000c9 (nZCv daIF +PAN -UAO +TCO +DIT -SSBS BTYPE=--)
>> [ 95.893275] pc : enqueue_task_fair+0x7a8/0xa40
>> [ 95.897705] lr : enqueue_task_fair+0x324/0xa40
>> [ 95.902136] sp : ffff800090323a50
>> [ 95.905438] x29: ffff800090323a50 x28: 0000000000000009 x27: ffffd8536a9cc5c0
>> [ 95.912565] x26: fffffdffbf43e1c0 x25: ffff0001fb7729c0 x24: fffffdffbf43e390
>> [ 95.919692] x23: 0000000000000001 x22: ffff000085d52400 x21: 0000000000000000
>> [ 95.926817] x20: ffff000085d52380 x19: ffff0001fb7727c0 x18: 0000000000000000
>> [ 95.933943] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000037d54fe0
>> [ 95.941069] x14: 0000000000000000 x13: ffffffffffffffff x12: ffffd8536a9cbdf8
>> [ 95.948195] x11: 00000000000000c0 x10: 02ae55ef2a914684 x9 : ffff00008d3511c0
>> [ 95.955321] x8 : 0000000000000001 x7 : 0000004100000000 x6 : ffffd8536a9cc5c0
>> [ 95.962442] x5 : ffff800090323ad8 x4 : ffff800090323ad8 x3 : ffff000085d527b8
>> [ 95.969563] x2 : 0000000000000001 x1 : ffff000085d52400 x0 : ffff000085d52400
>> [ 95.976689] Call trace:
>> [ 95.979124] enqueue_task_fair+0x7a8/0xa40 (P)
>> [ 95.983559] tg_unthrottle_up+0x14c/0x318
>> [ 95.987561] walk_tg_tree_from+0x60/0xb8
>> [ 95.991481] unthrottle_cfs_rq+0xa8/0x204
>> [ 95.995484] tg_set_bandwidth+0x36c/0x4c4
>> [ 95.999485] cpu_max_write+0xc8/0x130
>> [ 96.003138] cgroup_file_write+0xa4/0x194
>> [ 96.007150] kernfs_fop_write_iter+0x130/0x1fc
>> [ 96.011598] vfs_write+0x228/0x358
>> [ 96.015001] ksys_write+0x6c/0x104
>> [ 96.018395] __arm64_sys_write+0x18/0x24
>> [ 96.022309] invoke_syscall+0x54/0x110
>> [ 96.026051] el0_svc_common.constprop.0+0x40/0xe0
>> [ 96.030744] do_el0_svc+0x1c/0x28
>> [ 96.034048] el0_svc+0x38/0x260
>> [ 96.037183] el0t_64_sync_handler+0xa0/0xe4
>> [ 96.041357] el0t_64_sync+0x17c/0x180
>> [ 96.045011] ---[ end trace 0000000000000000 ]---
>
> Argh, so this is bandwidth. Task gets throttled, but before it gets back
> to userspace to actually dequeue, it gets hit by a wakeup again while it
> is still current?
>
> Hmm, no, in that case it can't actually be on throttled_tasks. So it did
> get dequeued, but it never quite scheduled out.
>
> I hate this bandwidth stuff :-)
>
> Anyway, this is different from the initial use-case, where a
> sched_entity would get dequeue and re-enqueued while walking the cgroup
> hierarchy, and thus had to appear like nothing happened in between.
>
> In this case we're fine to advance time.
>
> Anyway... the normal block/wakeup paths are >< close to also hitting
> this in the case where balancing drops the rq->lock after dequeue. The
> only reason ttwu can't tickle this is because it has that ->on_cpu
> stuff. However, we cannot use that on the unthrottle path :/
>
> Bah.
>
> Does this work?
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 5c98d8dfce5d..0edd8bfb6363 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -8250,7 +8250,14 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> weight = enqueue_hierarchy(p, flags);
> reweight_eevdf(cfs_rq, se, weight, false);
> place_entity(cfs_rq, se, flags | ENQUEUE_QUEUED);
> - __enqueue_entity(cfs_rq, se);
> +
> + /*
> + * This can actually happen with CFS_BANDWIDTH, in that case a task
> + * can get throttled, dequeued and then unthrottled, woken *before* it
> + * schedules out and does put_prev_task().
> + */
> + if (cfs_rq->curr != se)
> + __enqueue_entity(cfs_rq, se);
>
> if (!rq_h_nr_queued && rq->cfs.h_nr_queued)
> dl_server_start(&rq->fair_server);
Tested-by: Christian Loehle <christian.loehle@arm.com>
for my reproducer, that being said, and indeed the failure is:
throttle -> dequeue (not put_prev_task()'d yet) -> (from remote CPU) unthrottle -> enqueue()
The dequeue()->enqueue() also lets place_entity()
'double-account' the entity's (on dequeue() saved) lag, right?
After dequeue: curr == se, on_rq == 0 -> excl from average
enqueue_hierarchy(): curr == se, on_rq == 1 -> incl again
place_entity(): calculate placement assuming se is excl
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2026-10-12 14:59 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-24 12:52 [PATCH 1/2] sched/fair: reuse the ENQUEUE_DELAYED calculation in enqueue_task_fair() Kayra Cizmeci
2026-08-24 12:52 ` [PATCH 2/2] sched/fair: avoid recalculating curr status in place_entity() and requeue_delayed_entity() Kayra Cizmeci
2026-08-26 10:15 ` [PATCH v2 0/2] sched/fair: reduce repeated work in enqueue path Kayra Cizmeci
2026-08-26 10:15 ` [PATCH v2 1/2] sched/fair: reuse the ENQUEUE_DELAYED calculation in enqueue_task_fair() Kayra Cizmeci
2026-08-26 10:41 ` K Prateek Nayak
2026-08-26 18:44 ` Kayra Cizmeci
2026-09-07 16:05 ` [PATCH] sched/fair: Remove unused autogroup.h include Kayra Cizmeci
2026-09-07 16:14 ` [PATCH v2 1/2] sched/fair: reuse the ENQUEUE_DELAYED calculation in enqueue_task_fair() Kayra Cizmeci
2026-08-26 10:15 ` [PATCH v2 2/2] sched/fair: avoid recalculating curr status in place_entity() and requeue_delayed_entity() Kayra Cizmeci
2026-09-11 10:57 ` Kayra Cizmeci
2026-09-11 12:19 ` Vincent Guittot
2026-09-11 13:10 ` Kayra Cizmeci
2026-09-11 12:42 ` Peter Zijlstra
2026-09-11 14:08 ` Kayra Cizmeci
2026-09-11 16:02 ` [PATCH v3] sched/fair: remove dead code on enqueue_task_fair() Kayra Cizmeci
2026-10-12 12:58 ` Christian Loehle
2026-10-12 13:33 ` Kayra Cizmeci
2026-10-12 14:02 ` Peter Zijlstra
2026-10-12 14:15 ` Kayra Cizmeci
2026-10-12 14:59 ` Christian Loehle
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®