* [PATCH v2] sched/fair: Fix wakeup_preempt_fair for not waking up task
@ 2026-05-03 10:45 Vincent Guittot
2026-05-04 10:02 ` Peter Zijlstra
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Vincent Guittot @ 2026-05-03 10:45 UTC (permalink / raw)
To: mingo, peterz, juri.lelli, dietmar.eggemann, rostedt, bsegall,
mgorman, vschneid, kprateek.nayak, linux-kernel, qyousef
Cc: Vincent Guittot
The assumption that p is always enqueued and not delayed, is only true for
wakeup. If p was moved while delayed, pick_next_entity will dequeue it
and the cfs might become empty. Test if there are still queued tasks
before trying again to determine if p could be the next one to be picked.
There are at least 2 cases:
When cfs becomes idle, it tries to pull tasks but if those pulled tasks are
delayed, they will be dequeued when attached to cfs.
attach_tasks() -> attach_task() -> wakeup_preempt(rq, p, 0);
A misfit task running on cfs A triggers a load balance to be pulled on a
better cpu, the load balance on cfs B starts an active load balance to
pulled the running misfit task. If there is a delayed dequeue task on cfs A
, it can be pulled instead of the previously running misfit task.
attach_one_task() -> attach_task() -> wakeup_preempt(rq, p, 0);
Fixes: ac8e69e69363 ("sched/fair: Fix wakeup_preempt_fair() vs delayed dequeue")
Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
---
Change since v1:
- Use cfs_rq->nr_queued instead of WF_TTWU flag to cover all cases instead
of just wakeup.
I'm preparing patches to cover those useless newly idle and active load
balance task migration.
kernel/sched/fair.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 728965851842..24087b0f25f6 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9145,9 +9145,10 @@ static void wakeup_preempt_fair(struct rq *rq, struct task_struct *p, int wake_f
/*
* Because p is enqueued, nse being null can only mean that we
- * dequeued a delayed task.
+ * dequeued a delayed task. If there are still entities queued in
+ * cfs, check if the next one will be p.
*/
- if (!nse)
+ if (!nse && cfs_rq->nr_queued)
goto pick;
if (sched_feat(RUN_TO_PARITY))
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] sched/fair: Fix wakeup_preempt_fair for not waking up task
2026-05-03 10:45 [PATCH v2] sched/fair: Fix wakeup_preempt_fair for not waking up task Vincent Guittot
@ 2026-05-04 10:02 ` Peter Zijlstra
2026-05-05 10:50 ` [tip: sched/urgent] sched/fair: Fix wakeup_preempt_fair() " tip-bot2 for Vincent Guittot
2026-05-05 14:13 ` tip-bot2 for Vincent Guittot
2 siblings, 0 replies; 4+ messages in thread
From: Peter Zijlstra @ 2026-05-04 10:02 UTC (permalink / raw)
To: Vincent Guittot
Cc: mingo, juri.lelli, dietmar.eggemann, rostedt, bsegall, mgorman,
vschneid, kprateek.nayak, linux-kernel, qyousef
On Sun, May 03, 2026 at 12:45:03PM +0200, Vincent Guittot wrote:
> The assumption that p is always enqueued and not delayed, is only true for
> wakeup. If p was moved while delayed, pick_next_entity will dequeue it
> and the cfs might become empty. Test if there are still queued tasks
> before trying again to determine if p could be the next one to be picked.
>
> There are at least 2 cases:
>
> When cfs becomes idle, it tries to pull tasks but if those pulled tasks are
> delayed, they will be dequeued when attached to cfs.
> attach_tasks() -> attach_task() -> wakeup_preempt(rq, p, 0);
>
> A misfit task running on cfs A triggers a load balance to be pulled on a
> better cpu, the load balance on cfs B starts an active load balance to
> pulled the running misfit task. If there is a delayed dequeue task on cfs A
> , it can be pulled instead of the previously running misfit task.
> attach_one_task() -> attach_task() -> wakeup_preempt(rq, p, 0);
>
> Fixes: ac8e69e69363 ("sched/fair: Fix wakeup_preempt_fair() vs delayed dequeue")
> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
> ---
>
> Change since v1:
> - Use cfs_rq->nr_queued instead of WF_TTWU flag to cover all cases instead
> of just wakeup.
>
> I'm preparing patches to cover those useless newly idle and active load
> balance task migration.
>
> kernel/sched/fair.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 728965851842..24087b0f25f6 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -9145,9 +9145,10 @@ static void wakeup_preempt_fair(struct rq *rq, struct task_struct *p, int wake_f
>
> /*
> * Because p is enqueued, nse being null can only mean that we
> - * dequeued a delayed task.
> + * dequeued a delayed task. If there are still entities queued in
> + * cfs, check if the next one will be p.
> */
> - if (!nse)
> + if (!nse && cfs_rq->nr_queued)
> goto pick;
>
> if (sched_feat(RUN_TO_PARITY))
Right, that works. Thanks!
^ permalink raw reply [flat|nested] 4+ messages in thread
* [tip: sched/urgent] sched/fair: Fix wakeup_preempt_fair() for not waking up task
2026-05-03 10:45 [PATCH v2] sched/fair: Fix wakeup_preempt_fair for not waking up task Vincent Guittot
2026-05-04 10:02 ` Peter Zijlstra
@ 2026-05-05 10:50 ` tip-bot2 for Vincent Guittot
2026-05-05 14:13 ` tip-bot2 for Vincent Guittot
2 siblings, 0 replies; 4+ messages in thread
From: tip-bot2 for Vincent Guittot @ 2026-05-05 10:50 UTC (permalink / raw)
To: linux-tip-commits
Cc: Vincent Guittot, Peter Zijlstra (Intel), x86, linux-kernel
The following commit has been merged into the sched/urgent branch of tip:
Commit-ID: 3ed412adc88d914acf45c5a304366be0b30cfd0a
Gitweb: https://git.kernel.org/tip/3ed412adc88d914acf45c5a304366be0b30cfd0a
Author: Vincent Guittot <vincent.guittot@linaro.org>
AuthorDate: Sun, 03 May 2026 12:45:03 +02:00
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 05 May 2026 12:44:24 +02:00
sched/fair: Fix wakeup_preempt_fair() for not waking up task
Make sure to only call pick_next_entity() on an non-empty cfs_rq.
The assumption that p is always enqueued and not delayed, is only true for
wakeup. If p was moved while delayed, pick_next_entity() will dequeue it and
the cfs might become empty. Test if there are still queued tasks before trying
again to determine if p could be the next one to be picked.
There are at least 2 cases:
When cfs becomes idle, it tries to pull tasks but if those pulled tasks are
delayed, they will be dequeued when attached to cfs. attach_tasks() ->
attach_task() -> wakeup_preempt(rq, p, 0);
A misfit task running on cfs A triggers a load balance to be pulled on a better
cpu, the load balance on cfs B starts an active load balance to pulled the
running misfit task. If there is a delayed dequeue task on cfs A, it can be
pulled instead of the previously running misfit task. attach_one_task() ->
attach_task() -> wakeup_preempt(rq, p, 0);
Fixes: ac8e69e69363 ("sched/fair: Fix wakeup_preempt_fair() vs delayed dequeue")
Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://patch.msgid.link/20260503104503.1732682-1-vincent.guittot@linaro.org
---
kernel/sched/fair.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index b91c8b2..3ebec18 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9174,9 +9174,10 @@ pick:
/*
* Because p is enqueued, nse being null can only mean that we
- * dequeued a delayed task.
+ * dequeued a delayed task. If there are still entities queued in
+ * cfs, check if the next one will be p.
*/
- if (!nse)
+ if (!nse && cfs_rq->nr_queued)
goto pick;
if (sched_feat(RUN_TO_PARITY))
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [tip: sched/urgent] sched/fair: Fix wakeup_preempt_fair() for not waking up task
2026-05-03 10:45 [PATCH v2] sched/fair: Fix wakeup_preempt_fair for not waking up task Vincent Guittot
2026-05-04 10:02 ` Peter Zijlstra
2026-05-05 10:50 ` [tip: sched/urgent] sched/fair: Fix wakeup_preempt_fair() " tip-bot2 for Vincent Guittot
@ 2026-05-05 14:13 ` tip-bot2 for Vincent Guittot
2 siblings, 0 replies; 4+ messages in thread
From: tip-bot2 for Vincent Guittot @ 2026-05-05 14:13 UTC (permalink / raw)
To: linux-tip-commits
Cc: Vincent Guittot, Peter Zijlstra (Intel), x86, linux-kernel
The following commit has been merged into the sched/urgent branch of tip:
Commit-ID: 0c0cf14be490975a7431e9d49fcd7b190fff337f
Gitweb: https://git.kernel.org/tip/0c0cf14be490975a7431e9d49fcd7b190fff337f
Author: Vincent Guittot <vincent.guittot@linaro.org>
AuthorDate: Sun, 03 May 2026 12:45:03 +02:00
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 05 May 2026 16:03:12 +02:00
sched/fair: Fix wakeup_preempt_fair() for not waking up task
Make sure to only call pick_next_entity() on an non-empty cfs_rq.
The assumption that p is always enqueued and not delayed, is only true for
wakeup. If p was moved while delayed, pick_next_entity() will dequeue it and
the cfs might become empty. Test if there are still queued tasks before trying
again to determine if p could be the next one to be picked.
There are at least 2 cases:
When cfs becomes idle, it tries to pull tasks but if those pulled tasks are
delayed, they will be dequeued when attached to cfs. attach_tasks() ->
attach_task() -> wakeup_preempt(rq, p, 0);
A misfit task running on cfs A triggers a load balance to be pulled on a better
cpu, the load balance on cfs B starts an active load balance to pulled the
running misfit task. If there is a delayed dequeue task on cfs A, it can be
pulled instead of the previously running misfit task. attach_one_task() ->
attach_task() -> wakeup_preempt(rq, p, 0);
Fixes: ac8e69e69363 ("sched/fair: Fix wakeup_preempt_fair() vs delayed dequeue")
Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://patch.msgid.link/20260503104503.1732682-1-vincent.guittot@linaro.org
---
kernel/sched/fair.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index b91c8b2..3ebec18 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9174,9 +9174,10 @@ pick:
/*
* Because p is enqueued, nse being null can only mean that we
- * dequeued a delayed task.
+ * dequeued a delayed task. If there are still entities queued in
+ * cfs, check if the next one will be p.
*/
- if (!nse)
+ if (!nse && cfs_rq->nr_queued)
goto pick;
if (sched_feat(RUN_TO_PARITY))
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-05-05 14:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-03 10:45 [PATCH v2] sched/fair: Fix wakeup_preempt_fair for not waking up task Vincent Guittot
2026-05-04 10:02 ` Peter Zijlstra
2026-05-05 10:50 ` [tip: sched/urgent] sched/fair: Fix wakeup_preempt_fair() " tip-bot2 for Vincent Guittot
2026-05-05 14:13 ` tip-bot2 for Vincent Guittot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox