Re: [PATCH v2 1/3] sched/fair: Fix warning if NEXT_BUDDY enabled
From: K Prateek Nayak
Date: Thu Nov 28 2024 - 02:30:16 EST
Hello Adam,
On 11/27/2024 11:26 AM, Adam Li wrote:
Enabling NEXT_BUDDY triggers warning, and rcu stall:
[ 124.977300] cfs_rq->next->sched_delayed
I could reproduce this with a run of "perf bench sched messaging" but
given that we hit this warning, it also means that either
set_next_buddy() has incorrectly set a delayed entity as next buddy, or
clear_next_buddy() did not clear a delayed entity.
I also see PSI splats like:
psi: inconsistent task state! task=2524:kworker/u1028:2 cpu=154 psi_flags=10 clear=14 set=0
but the PSI flags it has set "(TSK_MEMSTALL_RUNNING | TSK_MEMSTALL)" and
the flags it is trying to clear
"(TSK_MEMSTALL_RUNNING | TSK_MEMSTALL | TSK_RUNNING)" seem to be only
possible if you have picked a dequeued entity for running before its
wakeup, which is also perhaps why the "nr_running" computation goes awry
and pick_eevdf() returns NULL (which it should never since
pick_next_entity() is only called when rq->cfs.nr_running is > 0)
[ 124.977310] WARNING: CPU: 51 PID: 2150 at kernel/sched/fair.c:5621 pick_task_fair+0x130/0x150
[ 125.049547] CPU: 51 UID: 0 PID: 2150 Comm: kworker/51:1 Tainted: G E 6.12.0.adam+ #1
<snip>
[ 125.163561] Call trace:
[ 125.165996] pick_task_fair+0x130/0x150 (P)
[ 125.170167] pick_task_fair+0x130/0x150 (L)
[ 125.174338] pick_next_task_fair+0x48/0x3c0
[ 125.178512] __pick_next_task+0x4c/0x220
[ 125.182426] pick_next_task+0x44/0x980
[ 125.186163] __schedule+0x3d0/0x628
[ 125.189645] schedule+0x3c/0xe0
[ 125.192776] worker_thread+0x1a4/0x368
[ 125.196516] kthread+0xfc/0x110
[ 125.199647] ret_from_fork+0x10/0x20
[ 125.203213] ---[ end trace 0000000000000000 ]---
<snip>
[ 211.151849] rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
[ 211.159759] rcu: (detected by 141, t=15003 jiffies, g=5629, q=26516 ncpus=384)
<snip>
Do not set next buddy if sched_delayed is set.
Fixes: 152e11f6df29 ("sched/fair: Implement delayed dequeue")
Signed-off-by: Adam Li <adamli@xxxxxxxxxxxxxxxxxxxxxx>
---
kernel/sched/fair.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index fbdca89c677f..cd1188b7f3df 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8748,6 +8748,8 @@ static void set_next_buddy(struct sched_entity *se)
return;
if (se_is_idle(se))
return;
+ if (se->sched_delayed)
+ return;
I tried to put a SCHED_WARN_ON() here to track where this comes from and
seems like it is usually from attach_task() in the load balancing path
pulling a delayed task which is set as the next buddy in
check_preempt_wakeup_fair()
Can you please try the following diff instead of the first two patches
and see if you still hit these warnings, stalls, and pick_eevdf()
returning NULL?
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index ff7cae9274c5..61e74eb5af22 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5478,6 +5478,7 @@ dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
bool sleep = flags & DEQUEUE_SLEEP;
update_curr(cfs_rq);
+ clear_buddies(cfs_rq, se);
if (flags & DEQUEUE_DELAYED) {
SCHED_WARN_ON(!se->sched_delayed);
@@ -5520,8 +5521,6 @@ dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags)
update_stats_dequeue_fair(cfs_rq, se, flags);
- clear_buddies(cfs_rq, se);
-
update_entity_lag(cfs_rq, se);
if (sched_feat(PLACE_REL_DEADLINE) && !sleep) {
se->deadline -= se->vruntime;
@@ -8767,7 +8766,7 @@ static void check_preempt_wakeup_fair(struct rq *rq, struct task_struct *p, int
if (unlikely(throttled_hierarchy(cfs_rq_of(pse))))
return;
- if (sched_feat(NEXT_BUDDY) && !(wake_flags & WF_FORK)) {
+ if (sched_feat(NEXT_BUDDY) && !(wake_flags & WF_FORK) && !pse->sched_delayed) {
set_next_buddy(pse);
}
--
If you are still encountering pick_eevdf() returning NULL, there could
be a larger issues (with eligibility computation, etc.) that the second
patch can hide which can lead to bigger problems later. Thank you.
cfs_rq_of(se)->next = se;
}
}
--
Thanks and Regards,
Prateek