Re: [PATCH RESEND] sched/core: Do not touch SMT siblings that have not joined the core yet

[Seiji Nishikawa]

On Wed, Nov 04, 2026 at 10:34:18AM +0100, Peter Zijlstra wrote:
> Hmm, so I hadn't seen the previous one; but I did see this one:
>
>   https://lore.kernel.org/all/20261101221219.118052-1-araoof@xxxxxxxxx/
>
> And I'm thinking you're both tripping over the same issue.

Yes, it is the same issue, and both patches fix it.

> Now, I asked Ahmed if we couldn't consistently do:
>
>  -     for_each_cpu(i, smt_mask) {
>  +     for_each_cpu_and(i, smt_mask, cpu_online_mask) {
>
> Does that work for you?

I tested your suggestion on v7.4-rc1 with the syzbot C reproducer.
I used cpu_online() in the first walk, and
for_each_cpu_and(i, smt_mask, cpu_online_mask) in the other walks and
in __sched_core_account_forceidle(). To make the window wider, I added
mdelay(2) between the first and the second walk. I also added
pr_warn_once() for a NULL core_pick in the second walk. It crashed
after about 60 seconds, right after CPU 1 came online:

[   61.359433][ T5634] smpboot: Booting Node 0 Processor 1 APIC 0x1
[   61.365674][   T22] sched_core: loop2 on cpu0 sees cpu1 online with NULL core_pick
[   61.365700][   T22] Oops: general protection fault, probably for non-canonical address 0xdffffc0000000002: 0000 [#1] SMP KASAN NOPTI
[   61.371165][   T22] KASAN: null-ptr-deref in range [0x0000000000000010-0x0000000000000017]
[   61.372533][   T22] CPU: 0 UID: 0 PID: 22 Comm: migration/0 Not tainted 7.4.0-rc1-dirty #13 PREEMPT_{RT,(full)}
[   61.376129][   T22] RIP: 0010:__schedule+0x439a/0x5890
[   61.385226][   T22] R13: ffff8881a3185000 R14: 1ffff11026bc7b7a R15: 0000000000000000

faddr2line shows that it crashed in cookie_equals() in the second
walk. It read the CPU of the NULL core_pick (R15) in task_cpu():

__schedule+0x439a/0x5890:
task_cpu at include/linux/sched.h:2263 (discriminator 2)
(inlined by) is_task_rq_idle at kernel/sched/core.c:6255 (discriminator 2)
(inlined by) cookie_equals at kernel/sched/core.c:6260 (discriminator 2)
(inlined by) pick_next_task at kernel/sched/core.c:6460 (discriminator 2)
(inlined by) __schedule at kernel/sched/core.c:7302 (discriminator 2)

It appears that CPU 1 was still offline in the first walk and came
online before the second walk. start_secondary() calls
set_cpu_online() without any rq lock, so the online mask can change
while we hold the core lock.

I also ran Ahmed's patch and mine with the same mdelay(2), and neither
of them crashed. Each ran for about 2700 seconds (about 2500
offline/online cycles of CPU 1). So the mdelay() itself does not cause
the crash.

In my patch, all walks see the same set of CPUs, because
rq_i->core == rq->core does not change while we hold the core lock.

The diff I used for this test is below.

Thanks,
Seiji

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 04b62c16fa8c..bea9712838ee 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -8,6 +8,7 @@
  *  Copyright (C) 1998-2024  Ingo Molnar, Red Hat
  */
 #define INSTANTIATE_EXPORTED_MIGRATE_DISABLE
+#include <linux/delay.h>
 #include <linux/sched.h>
 #include <linux/highmem.h>
 #include <linux/hrtimer_api.h>
@@ -6413,6 +6414,9 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
 	 */
 	max = NULL;
 	for_each_cpu_wrap(i, smt_mask, cpu) {
+		if (!cpu_online(i))
+			continue;
+
 		rq_i = cpu_rq(i);
 
 		/*
@@ -6445,9 +6449,13 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
 	 * For each thread: try and find a runnable task that matches @max or
 	 * force idle.
 	 */
-	for_each_cpu(i, smt_mask) {
+	mdelay(2);
+
+	for_each_cpu_and(i, smt_mask, cpu_online_mask) {
 		rq_i = cpu_rq(i);
 		p = rq_i->core_pick;
+		if (!p)
+			pr_warn_once("sched_core: loop2 on cpu%d sees cpu%d online with NULL core_pick\n", cpu, i);
 
 		if (!cookie_equals(p, cookie)) {
 			p = NULL;
@@ -6490,7 +6498,7 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
 	 * their task. This ensures there is no inter-sibling overlap between
 	 * non-matching user state.
 	 */
-	for_each_cpu(i, smt_mask) {
+	for_each_cpu_and(i, smt_mask, cpu_online_mask) {
 		rq_i = cpu_rq(i);
 
 		/*
diff --git a/kernel/sched/core_sched.c b/kernel/sched/core_sched.c
index 43e0bde3038e..1efde88da76e 100644
--- a/kernel/sched/core_sched.c
+++ b/kernel/sched/core_sched.c
@@ -273,7 +273,7 @@ void __sched_core_account_forceidle(struct rq *rq)
 		delta = div_u64(delta, rq->core->core_forceidle_occupation);
 	}
 
-	for_each_cpu(i, smt_mask) {
+	for_each_cpu_and(i, smt_mask, cpu_online_mask) {
 		rq_i = cpu_rq(i);
 		p = rq_i->core_pick ?: rq_i->curr;