* [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
* 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
* [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 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-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-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: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®