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

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

[Seiji Nishikawa]

When a CPU comes online, it is added to cpu_smt_mask() of its siblings
before sched_core_cpu_starting() sets its rq->core to the core leader.
For example, on x86, ap_starting() calls set_cpu_sibling_map() first,
and calls notify_cpu_starting() after that. This order is intended.

Before sched_core_cpu_starting() runs, rq->core of the new CPU still
points to its own rq. But rq->core_enabled of the new CPU is already
set. __sched_core_flip() sets it for all possible CPUs, also for
offline CPUs, and CPU offline does not clear it. So __rq_lockp() of the
new CPU returns its own rq->__lock. It does not return the core wide
lock.

If a sibling CPU runs pick_next_task() for SCHED_CORE at this time, it
checks all CPUs in cpu_smt_mask(). It calls update_rq_clock() and
pick_task() for the rq of the new CPU. But it holds only its own core
lock, not the lock of the new CPU. Then lockdep shows this warning in
update_rq_clock().

  debug_locks && !(lock_is_held(&(__rq_lockp(rq))->dep_map) != 0)
  WARNING: kernel/sched/sched.h:1666 at update_rq_clock+0x40a/0xd20 kernel/sched/core.c:875, CPU#0: syz-executor251/5712
  Call Trace:
   <TASK>
   pick_next_task kernel/sched/core.c:6375 [inline]
   __schedule+0x1c69/0x6920 kernel/sched/core.c:7199
   preempt_schedule_irq+0x4e/0x90 kernel/sched/core.c:7606

The next loops in pick_next_task() also change rq_i->core_pick of this
rq, and can call resched_curr() for it, without its lock.
resched_curr() has the same lockdep_assert_rq_held() check.
__sched_core_account_forceidle() also checks all CPUs in the same mask.

CPU offline does not have this problem. remove_siblinginfo() and
sched_core_cpu_dying() both run in take_cpu_down() under stop_machine.
So no sibling CPU can be in pick_next_task() at that time.

Fix this. In these loops, skip a sibling CPU if its rq->core is
different from our rq->core. At run time, rq->core of a CPU is changed
by sched_core_cpu_starting(), sched_core_cpu_deactivate() and
sched_core_cpu_dying(). The first two hold sched_core_lock(), and
sched_core_lock() takes the rq locks of all CPUs in the SMT mask. The
last one runs under stop_machine, as written above. So while we hold
our core lock, the result of this check does not change. A CPU that is
not in the core yet will pick its own tasks. The comment in the
reschedule loop of pick_next_task() already expects this case.

I tested this with the syzbot C reproducer in QEMU. The two vCPUs were
SMT siblings (-smp 2,sockets=1,cores=1,threads=2). Without this patch,
the warning came after about 100 seconds. Just before the warning, a
debug print showed that the loop on CPU 0 saw CPU 1 with rq->core
pointing to CPU 1 itself, and cpu_online() was false for CPU 1. With
this patch, the same state still happened, but there was no warning
in more than 2 hours.

Fixes: 3c474b3239f1 ("sched: Fix Core-wide rq->lock for uninitialized CPUs")
Reported-by: syzbot+681e729b2cf0d4860042@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=681e729b2cf0d4860042
Cc: stable@vger.kernel.org
Signed-off-by: Seiji Nishikawa <snishika@redhat.com>
---
Resend of https://lore.kernel.org/all/20261006061253.1199941-1-snishika@redhat.com/
No changes, only rebased on v7.4-rc1.

 kernel/sched/core.c       |  7 +++++++
 kernel/sched/core_sched.c |  3 +++
 kernel/sched/sched.h      | 11 +++++++++++
 3 files changed, 21 insertions(+)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 04b62c16fa8c..7d9f0989eaad 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6414,6 +6414,8 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
 	max = NULL;
 	for_each_cpu_wrap(i, smt_mask, cpu) {
 		rq_i = cpu_rq(i);
+		if (!sched_core_sibling(rq, rq_i))
+			continue;
 
 		/*
 		 * Current cpu always has its clock updated on entrance to
@@ -6447,6 +6449,9 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
 	 */
 	for_each_cpu(i, smt_mask) {
 		rq_i = cpu_rq(i);
+		if (!sched_core_sibling(rq, rq_i))
+			continue;
+
 		p = rq_i->core_pick;
 
 		if (!cookie_equals(p, cookie)) {
@@ -6492,6 +6497,8 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
 	 */
 	for_each_cpu(i, smt_mask) {
 		rq_i = cpu_rq(i);
+		if (!sched_core_sibling(rq, rq_i))
+			continue;
 
 		/*
 		 * An online sibling might have gone offline before a task
diff --git a/kernel/sched/core_sched.c b/kernel/sched/core_sched.c
index 43e0bde3038e..b21eade02c23 100644
--- a/kernel/sched/core_sched.c
+++ b/kernel/sched/core_sched.c
@@ -275,6 +275,9 @@ void __sched_core_account_forceidle(struct rq *rq)
 
 	for_each_cpu(i, smt_mask) {
 		rq_i = cpu_rq(i);
+		if (!sched_core_sibling(rq, rq_i))
+			continue;
+
 		p = rq_i->core_pick ?: rq_i->curr;
 
 		if (p == rq_i->idle)
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 6c7c5633524a..bd0eb5fe6270 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1534,6 +1534,17 @@ static inline raw_spinlock_t *__rq_lockp(struct rq *rq)
 	return &rq->__lock;
 }
 
+/*
+ * A CPU coming online is set in cpu_smt_mask() before
+ * sched_core_cpu_starting() links its rq->core to the core leader.
+ * Until then it does not share the core-wide lock, so it must not be
+ * touched as part of the core.
+ */
+static inline bool sched_core_sibling(struct rq *rq, struct rq *rq_i)
+{
+	return rq_i->core == rq->core;
+}
+
 extern bool
 cfs_prio_less(const struct task_struct *a, const struct task_struct *b, bool fi);
 
-- 
2.56.0

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

[Peter Zijlstra]

On Wed, Nov 04, 2026 at 11:20:04AM +0900, Seiji Nishikawa wrote:
> When a CPU comes online, it is added to cpu_smt_mask() of its siblings
> before sched_core_cpu_starting() sets its rq->core to the core leader.
> For example, on x86, ap_starting() calls set_cpu_sibling_map() first,
> and calls notify_cpu_starting() after that. This order is intended.
> 
> Before sched_core_cpu_starting() runs, rq->core of the new CPU still
> points to its own rq. But rq->core_enabled of the new CPU is already
> set. __sched_core_flip() sets it for all possible CPUs, also for
> offline CPUs, and CPU offline does not clear it. So __rq_lockp() of the
> new CPU returns its own rq->__lock. It does not return the core wide
> lock.
> 
> If a sibling CPU runs pick_next_task() for SCHED_CORE at this time, it
> checks all CPUs in cpu_smt_mask(). It calls update_rq_clock() and
> pick_task() for the rq of the new CPU. But it holds only its own core
> lock, not the lock of the new CPU. Then lockdep shows this warning in
> update_rq_clock().
> 
>   debug_locks && !(lock_is_held(&(__rq_lockp(rq))->dep_map) != 0)
>   WARNING: kernel/sched/sched.h:1666 at update_rq_clock+0x40a/0xd20 kernel/sched/core.c:875, CPU#0: syz-executor251/5712
>   Call Trace:
>    <TASK>
>    pick_next_task kernel/sched/core.c:6375 [inline]
>    __schedule+0x1c69/0x6920 kernel/sched/core.c:7199
>    preempt_schedule_irq+0x4e/0x90 kernel/sched/core.c:7606
> 
> The next loops in pick_next_task() also change rq_i->core_pick of this
> rq, and can call resched_curr() for it, without its lock.
> resched_curr() has the same lockdep_assert_rq_held() check.
> __sched_core_account_forceidle() also checks all CPUs in the same mask.
> 
> CPU offline does not have this problem. remove_siblinginfo() and
> sched_core_cpu_dying() both run in take_cpu_down() under stop_machine.
> So no sibling CPU can be in pick_next_task() at that time.
> 
> Fix this. In these loops, skip a sibling CPU if its rq->core is
> different from our rq->core. At run time, rq->core of a CPU is changed
> by sched_core_cpu_starting(), sched_core_cpu_deactivate() and
> sched_core_cpu_dying(). The first two hold sched_core_lock(), and
> sched_core_lock() takes the rq locks of all CPUs in the SMT mask. The
> last one runs under stop_machine, as written above. So while we hold
> our core lock, the result of this check does not change. A CPU that is
> not in the core yet will pick its own tasks. The comment in the
> reschedule loop of pick_next_task() already expects this case.
> 
> I tested this with the syzbot C reproducer in QEMU. The two vCPUs were
> SMT siblings (-smp 2,sockets=1,cores=1,threads=2). Without this patch,
> the warning came after about 100 seconds. Just before the warning, a
> debug print showed that the loop on CPU 0 saw CPU 1 with rq->core
> pointing to CPU 1 itself, and cpu_online() was false for CPU 1. With
> this patch, the same state still happened, but there was no warning
> in more than 2 hours.
> 
> Fixes: 3c474b3239f1 ("sched: Fix Core-wide rq->lock for uninitialized CPUs")
> Reported-by: syzbot+681e729b2cf0d4860042@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=681e729b2cf0d4860042
> Cc: stable@vger.kernel.org
> Signed-off-by: Seiji Nishikawa <snishika@redhat.com>

Hmm, so I hadn't seen the previous one; but I did see this one:

  https://lore.kernel.org/all/20261101221219.118052-1-araoof@amazon.de/

And I'm thinking you're both tripping over the same issue.

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?

>  kernel/sched/core.c       |  7 +++++++
>  kernel/sched/core_sched.c |  3 +++
>  kernel/sched/sched.h      | 11 +++++++++++
>  3 files changed, 21 insertions(+)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 04b62c16fa8c..7d9f0989eaad 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -6414,6 +6414,8 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
>  	max = NULL;
>  	for_each_cpu_wrap(i, smt_mask, cpu) {
>  		rq_i = cpu_rq(i);
> +		if (!sched_core_sibling(rq, rq_i))
> +			continue;
>  
>  		/*
>  		 * Current cpu always has its clock updated on entrance to
> @@ -6447,6 +6449,9 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
>  	 */
>  	for_each_cpu(i, smt_mask) {
>  		rq_i = cpu_rq(i);
> +		if (!sched_core_sibling(rq, rq_i))
> +			continue;
> +
>  		p = rq_i->core_pick;
>  
>  		if (!cookie_equals(p, cookie)) {
> @@ -6492,6 +6497,8 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
>  	 */
>  	for_each_cpu(i, smt_mask) {
>  		rq_i = cpu_rq(i);
> +		if (!sched_core_sibling(rq, rq_i))
> +			continue;
>  
>  		/*
>  		 * An online sibling might have gone offline before a task
> diff --git a/kernel/sched/core_sched.c b/kernel/sched/core_sched.c
> index 43e0bde3038e..b21eade02c23 100644
> --- a/kernel/sched/core_sched.c
> +++ b/kernel/sched/core_sched.c
> @@ -275,6 +275,9 @@ void __sched_core_account_forceidle(struct rq *rq)
>  
>  	for_each_cpu(i, smt_mask) {
>  		rq_i = cpu_rq(i);
> +		if (!sched_core_sibling(rq, rq_i))
> +			continue;
> +
>  		p = rq_i->core_pick ?: rq_i->curr;
>  
>  		if (p == rq_i->idle)
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 6c7c5633524a..bd0eb5fe6270 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -1534,6 +1534,17 @@ static inline raw_spinlock_t *__rq_lockp(struct rq *rq)
>  	return &rq->__lock;
>  }
>  
> +/*
> + * A CPU coming online is set in cpu_smt_mask() before
> + * sched_core_cpu_starting() links its rq->core to the core leader.
> + * Until then it does not share the core-wide lock, so it must not be
> + * touched as part of the core.
> + */
> +static inline bool sched_core_sibling(struct rq *rq, struct rq *rq_i)
> +{
> +	return rq_i->core == rq->core;
> +}
> +
>  extern bool
>  cfs_prio_less(const struct task_struct *a, const struct task_struct *b, bool fi);
>  
> -- 
> 2.56.0
> 

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@amazon.de/
>
> 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;
 

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

[Peter Zijlstra]

On Thu, Nov 05, 2026 at 03:16:13AM +0900, Seiji Nishikawa wrote:
> 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@amazon.de/
> >
> > 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:
> 

> 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.

Right, I did wonder why Ahmed's patch had that second iteration
differently. I suppose this answers it.

Of the two, I like your better for being more consistent -- but you're
doing the same thing 4 times, which does beg the question if we cannot
wrap that nicer.

How about so then?

---
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 04b62c16fa8c..eb3380c473ba 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6296,7 +6296,6 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
 	__must_hold(__rq_lockp(rq))
 {
 	struct task_struct *next, *p, *max;
-	const struct cpumask *smt_mask;
 	bool fi_before = false;
 	bool core_clock_updated = (rq == rq->core);
 	unsigned long cookie;
@@ -6343,8 +6342,6 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
 
 	prev_balance(rq, rf);
 
-	smt_mask = cpu_smt_mask(cpu);
-
 restart:
 	need_sync |= !!rq->core->core_cookie;
 
@@ -6412,7 +6409,7 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
 	 * Tie-break prio towards the current CPU
 	 */
 	max = NULL;
-	for_each_cpu_wrap(i, smt_mask, cpu) {
+	for_each_core_cpu_wrap(i, rq) {
 		rq_i = cpu_rq(i);
 
 		/*
@@ -6445,7 +6442,7 @@ 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) {
+	for_each_core_cpu(i, rq) {
 		rq_i = cpu_rq(i);
 		p = rq_i->core_pick;
 
@@ -6490,7 +6487,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_core_cpu(i, rq) {
 		rq_i = cpu_rq(i);
 
 		/*
diff --git a/kernel/sched/core_sched.c b/kernel/sched/core_sched.c
index 43e0bde3038e..d56be1c31435 100644
--- a/kernel/sched/core_sched.c
+++ b/kernel/sched/core_sched.c
@@ -241,7 +241,6 @@ int sched_core_share_pid(unsigned int cmd, pid_t pid, enum pid_type type,
 /* REQUIRES: rq->core's clock recently updated. */
 void __sched_core_account_forceidle(struct rq *rq)
 {
-	const struct cpumask *smt_mask = cpu_smt_mask(cpu_of(rq));
 	u64 delta, now = rq_clock(rq->core);
 	struct rq *rq_i;
 	struct task_struct *p;
@@ -273,7 +272,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_core_cpu(i, rq) {
 		rq_i = cpu_rq(i);
 		p = rq_i->core_pick ?: rq_i->curr;
 
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 6c7c5633524a..1b57910288bc 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1534,6 +1534,25 @@ static inline raw_spinlock_t *__rq_lockp(struct rq *rq)
 	return &rq->__lock;
 }
 
+/*
+ * A CPU coming online is set in cpu_smt_mask() before
+ * sched_core_cpu_starting() links its rq->core to the core leader.
+ * Until then it does not share the core-wide lock, so it must not be
+ * touched as part of the core.
+ */
+static inline bool sched_core_sibling(struct rq *rq, struct rq *rq_i)
+{
+	return rq_i->core == rq->core;
+}
+
+#define for_each_core_cpu(i, rq) \
+	for_each_cpu(i, cpu_smt_mask(rq->cpu)) \
+		if (sched_core_sibling(rq, cpu_rq(i)))
+
+#define for_each_core_cpu_wrap(i, rq) \
+	for_each_cpu_wrap(i, cpu_smt_mask(rq->cpu), rq->cpu) \
+		if (sched_core_sibling(rq, cpu_rq(i)))
+
 extern bool
 cfs_prio_less(const struct task_struct *a, const struct task_struct *b, bool fi);
 

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

[Seiji Nishikawa]

On Thu, Nov 05, 2026 at 11:10:18AM +0100, Peter Zijlstra wrote:
> Of the two, I like your better for being more consistent -- but you're
> doing the same thing 4 times, which does beg the question if we cannot
> wrap that nicer.
>
> How about so then?

Thanks, this looks nicer.

I tested it on v7.4-rc1 with the syzbot C reproducer. The warning did
not trigger in about 1250 seconds (about 6100 offline/online cycles of
CPU 1).

Should I send a v2 with this change, or will you apply it yourself?

Thanks,
Seiji

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

[Peter Zijlstra]

On Thu, Nov 05, 2026 at 10:40:35PM +0900, Seiji Nishikawa wrote:
> On Thu, Nov 05, 2026 at 11:10:18AM +0100, Peter Zijlstra wrote:
> > Of the two, I like your better for being more consistent -- but you're
> > doing the same thing 4 times, which does beg the question if we cannot
> > wrap that nicer.
> >
> > How about so then?
> 
> Thanks, this looks nicer.
> 
> I tested it on v7.4-rc1 with the syzbot C reproducer. The warning did
> not trigger in about 1250 seconds (about 6100 offline/online cycles of
> CPU 1).
> 
> Should I send a v2 with this change, or will you apply it yourself?

I have it; if only 0-day would hurry up with the current batch of
sched/urgent patches :/

[tip: sched/urgent] sched/core: Do not touch SMT siblings that have not joined the core yet

[tip-bot2 for Seiji Nishikawa]

The following commit has been merged into the sched/urgent branch of tip:

Commit-ID:     bfee16fd01d4d5c3352a050f560a5ff05a277956
Gitweb:        https://git.kernel.org/tip/bfee16fd01d4d5c3352a050f560a5ff05a277956
Author:        Seiji Nishikawa <snishika@redhat.com>
AuthorDate:    Wed, 04 Nov 2026 11:20:04 +09:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 06 Nov 2026 11:45:18 +01:00

sched/core: Do not touch SMT siblings that have not joined the core yet

When a CPU comes online, it is added to cpu_smt_mask() of its siblings
before sched_core_cpu_starting() sets its rq->core to the core leader.
For example, on x86, ap_starting() calls set_cpu_sibling_map() first,
and calls notify_cpu_starting() after that. This order is intended.

Before sched_core_cpu_starting() runs, rq->core of the new CPU still
points to its own rq. But rq->core_enabled of the new CPU is already
set. __sched_core_flip() sets it for all possible CPUs, also for
offline CPUs, and CPU offline does not clear it. So __rq_lockp() of the
new CPU returns its own rq->__lock. It does not return the core wide
lock.

If a sibling CPU runs pick_next_task() for SCHED_CORE at this time, it
checks all CPUs in cpu_smt_mask(). It calls update_rq_clock() and
pick_task() for the rq of the new CPU. But it holds only its own core
lock, not the lock of the new CPU. Then lockdep shows this warning in
update_rq_clock().

  debug_locks && !(lock_is_held(&(__rq_lockp(rq))->dep_map) != 0)
  WARNING: kernel/sched/sched.h:1666 at update_rq_clock+0x40a/0xd20 kernel/sched/core.c:875, CPU#0: syz-executor251/5712
  Call Trace:
   <TASK>
   pick_next_task kernel/sched/core.c:6375 [inline]
   __schedule+0x1c69/0x6920 kernel/sched/core.c:7199
   preempt_schedule_irq+0x4e/0x90 kernel/sched/core.c:7606

The next loops in pick_next_task() also change rq_i->core_pick of this
rq, and can call resched_curr() for it, without its lock.
resched_curr() has the same lockdep_assert_rq_held() check.
__sched_core_account_forceidle() also checks all CPUs in the same mask.

CPU offline does not have this problem. remove_siblinginfo() and
sched_core_cpu_dying() both run in take_cpu_down() under stop_machine.
So no sibling CPU can be in pick_next_task() at that time.

Fix this. In these loops, skip a sibling CPU if its rq->core is
different from our rq->core. At run time, rq->core of a CPU is changed
by sched_core_cpu_starting(), sched_core_cpu_deactivate() and
sched_core_cpu_dying(). The first two hold sched_core_lock(), and
sched_core_lock() takes the rq locks of all CPUs in the SMT mask. The
last one runs under stop_machine, as written above. So while we hold
our core lock, the result of this check does not change. A CPU that is
not in the core yet will pick its own tasks. The comment in the
reschedule loop of pick_next_task() already expects this case.

I tested this with the syzbot C reproducer in QEMU. The two vCPUs were
SMT siblings (-smp 2,sockets=1,cores=1,threads=2). Without this patch,
the warning came after about 100 seconds. Just before the warning, a
debug print showed that the loop on CPU 0 saw CPU 1 with rq->core
pointing to CPU 1 itself, and cpu_online() was false for CPU 1. With
this patch, the same state still happened, but there was no warning
in more than 2 hours.

Fixes: 3c474b3239f1 ("sched: Fix Core-wide rq->lock for uninitialized CPUs")
Closes: https://syzkaller.appspot.com/bug?extid=681e729b2cf0d4860042
Reported-by: syzbot+681e729b2cf0d4860042@syzkaller.appspotmail.com
Signed-off-by: Seiji Nishikawa <snishika@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://patch.msgid.link/20261104022004.3810619-1-snishika@redhat.com
Cc: stable@vger.kernel.org
---
 kernel/sched/core.c       |  9 +++------
 kernel/sched/core_sched.c |  3 +--
 kernel/sched/sched.h      | 19 +++++++++++++++++++
 3 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 1a088d4..5ed660f 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6297,7 +6297,6 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
 	__must_hold(__rq_lockp(rq))
 {
 	struct task_struct *next, *p, *max;
-	const struct cpumask *smt_mask;
 	bool fi_before = false;
 	bool core_clock_updated = (rq == rq->core);
 	unsigned long cookie;
@@ -6344,8 +6343,6 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
 
 	prev_balance(rq, rf);
 
-	smt_mask = cpu_smt_mask(cpu);
-
 restart:
 	need_sync |= !!rq->core->core_cookie;
 
@@ -6413,7 +6410,7 @@ restart:
 	 * Tie-break prio towards the current CPU
 	 */
 	max = NULL;
-	for_each_cpu_wrap(i, smt_mask, cpu) {
+	for_each_core_cpu_wrap(i, rq) {
 		rq_i = cpu_rq(i);
 
 		/*
@@ -6446,7 +6443,7 @@ restart:
 	 * For each thread: try and find a runnable task that matches @max or
 	 * force idle.
 	 */
-	for_each_cpu(i, smt_mask) {
+	for_each_core_cpu(i, rq) {
 		rq_i = cpu_rq(i);
 		p = rq_i->core_pick;
 
@@ -6491,7 +6488,7 @@ restart:
 	 * their task. This ensures there is no inter-sibling overlap between
 	 * non-matching user state.
 	 */
-	for_each_cpu(i, smt_mask) {
+	for_each_core_cpu(i, rq) {
 		rq_i = cpu_rq(i);
 
 		/*
diff --git a/kernel/sched/core_sched.c b/kernel/sched/core_sched.c
index 43e0bde..d56be1c 100644
--- a/kernel/sched/core_sched.c
+++ b/kernel/sched/core_sched.c
@@ -241,7 +241,6 @@ out:
 /* REQUIRES: rq->core's clock recently updated. */
 void __sched_core_account_forceidle(struct rq *rq)
 {
-	const struct cpumask *smt_mask = cpu_smt_mask(cpu_of(rq));
 	u64 delta, now = rq_clock(rq->core);
 	struct rq *rq_i;
 	struct task_struct *p;
@@ -273,7 +272,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_core_cpu(i, rq) {
 		rq_i = cpu_rq(i);
 		p = rq_i->core_pick ?: rq_i->curr;
 
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 1d76950..92134ce 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1534,6 +1534,25 @@ static inline raw_spinlock_t *__rq_lockp(struct rq *rq)
 	return &rq->__lock;
 }
 
+/*
+ * A CPU coming online is set in cpu_smt_mask() before
+ * sched_core_cpu_starting() links its rq->core to the core leader.
+ * Until then it does not share the core-wide lock, so it must not be
+ * touched as part of the core.
+ */
+static inline bool sched_core_sibling(struct rq *rq, struct rq *rq_i)
+{
+	return rq_i->core == rq->core;
+}
+
+#define for_each_core_cpu(i, rq) \
+	for_each_cpu(i, cpu_smt_mask(rq->cpu)) \
+		if (sched_core_sibling(rq, cpu_rq(i)))
+
+#define for_each_core_cpu_wrap(i, rq) \
+	for_each_cpu_wrap(i, cpu_smt_mask(rq->cpu), rq->cpu) \
+		if (sched_core_sibling(rq, cpu_rq(i)))
+
 extern bool
 cfs_prio_less(const struct task_struct *a, const struct task_struct *b, bool fi);
 

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

[tip-bot2 for Seiji Nishikawa]

The following commit has been merged into the sched/core branch of tip:

Commit-ID:     ec6f6731538e0d45872eb279fdeb477bed152d6f
Gitweb:        https://git.kernel.org/tip/ec6f6731538e0d45872eb279fdeb477bed152d6f
Author:        Seiji Nishikawa <snishika@redhat.com>
AuthorDate:    Wed, 04 Nov 2026 11:20:04 +09:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Mon, 09 Nov 2026 10:09:45 +01:00

sched/core: Do not touch SMT siblings that have not joined the core yet

When a CPU comes online, it is added to cpu_smt_mask() of its siblings
before sched_core_cpu_starting() sets its rq->core to the core leader.
For example, on x86, ap_starting() calls set_cpu_sibling_map() first,
and calls notify_cpu_starting() after that. This order is intended.

Before sched_core_cpu_starting() runs, rq->core of the new CPU still
points to its own rq. But rq->core_enabled of the new CPU is already
set. __sched_core_flip() sets it for all possible CPUs, also for
offline CPUs, and CPU offline does not clear it. So __rq_lockp() of the
new CPU returns its own rq->__lock. It does not return the core wide
lock.

If a sibling CPU runs pick_next_task() for SCHED_CORE at this time, it
checks all CPUs in cpu_smt_mask(). It calls update_rq_clock() and
pick_task() for the rq of the new CPU. But it holds only its own core
lock, not the lock of the new CPU. Then lockdep shows this warning in
update_rq_clock().

  debug_locks && !(lock_is_held(&(__rq_lockp(rq))->dep_map) != 0)
  WARNING: kernel/sched/sched.h:1666 at update_rq_clock+0x40a/0xd20 kernel/sched/core.c:875, CPU#0: syz-executor251/5712
  Call Trace:
   <TASK>
   pick_next_task kernel/sched/core.c:6375 [inline]
   __schedule+0x1c69/0x6920 kernel/sched/core.c:7199
   preempt_schedule_irq+0x4e/0x90 kernel/sched/core.c:7606

The next loops in pick_next_task() also change rq_i->core_pick of this
rq, and can call resched_curr() for it, without its lock.
resched_curr() has the same lockdep_assert_rq_held() check.
__sched_core_account_forceidle() also checks all CPUs in the same mask.

CPU offline does not have this problem. remove_siblinginfo() and
sched_core_cpu_dying() both run in take_cpu_down() under stop_machine.
So no sibling CPU can be in pick_next_task() at that time.

Fix this. In these loops, skip a sibling CPU if its rq->core is
different from our rq->core. At run time, rq->core of a CPU is changed
by sched_core_cpu_starting(), sched_core_cpu_deactivate() and
sched_core_cpu_dying(). The first two hold sched_core_lock(), and
sched_core_lock() takes the rq locks of all CPUs in the SMT mask. The
last one runs under stop_machine, as written above. So while we hold
our core lock, the result of this check does not change. A CPU that is
not in the core yet will pick its own tasks. The comment in the
reschedule loop of pick_next_task() already expects this case.

I tested this with the syzbot C reproducer in QEMU. The two vCPUs were
SMT siblings (-smp 2,sockets=1,cores=1,threads=2). Without this patch,
the warning came after about 100 seconds. Just before the warning, a
debug print showed that the loop on CPU 0 saw CPU 1 with rq->core
pointing to CPU 1 itself, and cpu_online() was false for CPU 1. With
this patch, the same state still happened, but there was no warning
in more than 2 hours.

Fixes: 3c474b3239f1 ("sched: Fix Core-wide rq->lock for uninitialized CPUs")
Closes: https://syzkaller.appspot.com/bug?extid=681e729b2cf0d4860042
Reported-by: syzbot+681e729b2cf0d4860042@syzkaller.appspotmail.com
Signed-off-by: Seiji Nishikawa <snishika@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20261104022004.3810619-1-snishika@redhat.com
---
 kernel/sched/core.c       |  9 +++------
 kernel/sched/core_sched.c |  3 +--
 kernel/sched/sched.h      | 19 +++++++++++++++++++
 3 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 1a088d4..5ed660f 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6297,7 +6297,6 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
 	__must_hold(__rq_lockp(rq))
 {
 	struct task_struct *next, *p, *max;
-	const struct cpumask *smt_mask;
 	bool fi_before = false;
 	bool core_clock_updated = (rq == rq->core);
 	unsigned long cookie;
@@ -6344,8 +6343,6 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
 
 	prev_balance(rq, rf);
 
-	smt_mask = cpu_smt_mask(cpu);
-
 restart:
 	need_sync |= !!rq->core->core_cookie;
 
@@ -6413,7 +6410,7 @@ restart:
 	 * Tie-break prio towards the current CPU
 	 */
 	max = NULL;
-	for_each_cpu_wrap(i, smt_mask, cpu) {
+	for_each_core_cpu_wrap(i, rq) {
 		rq_i = cpu_rq(i);
 
 		/*
@@ -6446,7 +6443,7 @@ restart:
 	 * For each thread: try and find a runnable task that matches @max or
 	 * force idle.
 	 */
-	for_each_cpu(i, smt_mask) {
+	for_each_core_cpu(i, rq) {
 		rq_i = cpu_rq(i);
 		p = rq_i->core_pick;
 
@@ -6491,7 +6488,7 @@ restart:
 	 * their task. This ensures there is no inter-sibling overlap between
 	 * non-matching user state.
 	 */
-	for_each_cpu(i, smt_mask) {
+	for_each_core_cpu(i, rq) {
 		rq_i = cpu_rq(i);
 
 		/*
diff --git a/kernel/sched/core_sched.c b/kernel/sched/core_sched.c
index 43e0bde..d56be1c 100644
--- a/kernel/sched/core_sched.c
+++ b/kernel/sched/core_sched.c
@@ -241,7 +241,6 @@ out:
 /* REQUIRES: rq->core's clock recently updated. */
 void __sched_core_account_forceidle(struct rq *rq)
 {
-	const struct cpumask *smt_mask = cpu_smt_mask(cpu_of(rq));
 	u64 delta, now = rq_clock(rq->core);
 	struct rq *rq_i;
 	struct task_struct *p;
@@ -273,7 +272,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_core_cpu(i, rq) {
 		rq_i = cpu_rq(i);
 		p = rq_i->core_pick ?: rq_i->curr;
 
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 1d76950..92134ce 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1534,6 +1534,25 @@ static inline raw_spinlock_t *__rq_lockp(struct rq *rq)
 	return &rq->__lock;
 }
 
+/*
+ * A CPU coming online is set in cpu_smt_mask() before
+ * sched_core_cpu_starting() links its rq->core to the core leader.
+ * Until then it does not share the core-wide lock, so it must not be
+ * touched as part of the core.
+ */
+static inline bool sched_core_sibling(struct rq *rq, struct rq *rq_i)
+{
+	return rq_i->core == rq->core;
+}
+
+#define for_each_core_cpu(i, rq) \
+	for_each_cpu(i, cpu_smt_mask(rq->cpu)) \
+		if (sched_core_sibling(rq, cpu_rq(i)))
+
+#define for_each_core_cpu_wrap(i, rq) \
+	for_each_cpu_wrap(i, cpu_smt_mask(rq->cpu), rq->cpu) \
+		if (sched_core_sibling(rq, cpu_rq(i)))
+
 extern bool
 cfs_prio_less(const struct task_struct *a, const struct task_struct *b, bool fi);