mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Seiji Nishikawa <snishika@redhat.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@kernel.org>, Ingo Molnar <mingo@redhat.com>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Valentin Schneider <vschneid@redhat.com>,
	K Prateek Nayak <kprateek.nayak@amd.com>,
	Josh Don <joshdon@google.com>,
	linux-kernel@vger.kernel.org,
	syzbot+681e729b2cf0d4860042@syzkaller.appspotmail.com,
	syzkaller-bugs@googlegroups.com,
	Ahmed Abdelraoof <araoof@amazon.de>
Subject: Re: [PATCH RESEND] sched/core: Do not touch SMT siblings that have not joined the core yet
Date: Thu,  5 Nov 2026 03:16:13 +0900	[thread overview]
Message-ID: <20261104181613.3978049-1-snishika@redhat.com> (raw)
In-Reply-To: <20261104093418.GA1779021@noisy.programming.kicks-ass.net>

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;
 


      reply	other threads:[~2026-11-04 18:16 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-04  2:20 Seiji Nishikawa
2026-11-04  9:34 ` Peter Zijlstra
2026-11-04 18:16   ` Seiji Nishikawa [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261104181613.3978049-1-snishika@redhat.com \
    --to=snishika@redhat.com \
    --cc=araoof@amazon.de \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=joshdon@google.com \
    --cc=juri.lelli@redhat.com \
    --cc=kprateek.nayak@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=syzbot+681e729b2cf0d4860042@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=tglx@kernel.org \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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®