Re: [PATCH RFC] arm64: Mark set_preempt_need_resched() access to .need_resched

[Will Deacon]

[+KCSAN maintainers]

On Thu, Aug 06, 2026 at 10:25:37AM -0700, Paul E. McKenney wrote:
> On Thu, Aug 06, 2026 at 12:58:40PM +0100, Will Deacon wrote:
> > On Fri, Jul 31, 2026 at 11:39:14AM -0700, Paul E. McKenney wrote:
> > > On Fri, Jul 31, 2026 at 09:44:16AM -0700, Paul E. McKenney wrote:
> > > > On Fri, Jul 31, 2026 at 01:51:26PM +0100, Mark Rutland wrote:
> > > > > On Thu, Jul 30, 2026 at 04:59:57PM -0700, Paul E. McKenney wrote:
> > > > > > The .need_resched field can be accessed from both task level and
> > > > > > from interrrupt handlers, so apply WRITE_ONCE() to the update in
> > > > > > set_preempt_need_resched(). This also brings arm64 in line with s390
> > > > > > (which uses atomic operations) and x86 (which uses inline assembly).
> > > > > > Other architectures avoid this issue via the empty definition in
> > > > > > include/asm-generic/preempt.h.
> > > > > >
> > > > > > KCSAN located this issue.
> > > > >
> > > > > Do you have the KCSAN splat to hand? Seeing the exact race (and the
> > > > > relevant reader(s)) would be handy.
> > > > >
> > > > > > Signed-off-by: Paul E. McKenney <pau...@kernel.org>
> > > > > > Cc: Catalin Marinas <catalin...@arm.com>
> > > > > > Cc: Will Deacon <wi...@kernel.org>
> > > > > > Cc: Jinjie Ruan <ruanj...@huawei.com>
> > > > > > Cc: Ada Couprie Diaz <ada.cou...@arm.com>
> > > > > > Cc: <linux-ar...@lists.infradead.org>
> > > > > >
> > > > > > ---
> > > > > > preempt.h | 2 +-
> > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/arch/arm64/include/asm/preempt.h b/arch/arm64/include/asm/preempt.h
> > > > > > index 932ea4b6204289..610853da140a78 100644
> > > > > > --- a/arch/arm64/include/asm/preempt.h
> > > > > > +++ b/arch/arm64/include/asm/preempt.h
> > > > > > @@ -28,7 +28,7 @@ static inline void preempt_count_set(u64 pc)
> > > > > >
> > > > > > static inline void set_preempt_need_resched(void)
> > > > > > {
> > > > > > - current_thread_info()->preempt.need_resched = 0;
> > > > > > + WRITE_ONCE(current_thread_info()->preempt.need_resched, 0);
> > > > > > }
> > > > >
> > > > > I was under the impression that we only used this in a context where we
> > > > > couldn't have a nested writer (e.g. within a scheduler IRQ hook, with
> > > > > IRQs disabled).
> > > > >
> > > > > If we have concurrent writers, this is probably broken regardless of
> > > > > whether it is single-copy-atomic, and I worry WRITE_ONCE() will mask a
> > > > > bug.
> > > > >
> > > > > If you can share the KCSAN splat, that'd really help.
> > > >
> > > > Let me regenerate it...
> > >
> > > And please see below.
> > >
> > > > > I think we need READ_ONCE() in test_preempt_need_resched(), but today
> > > > > that's only used by tracing to determine what to log.
> > > >
> > > > I do have tracing enabled on some of my runs.
> > >
> > > I did build with CONFIG_TRACEPOINTS=y. Or let me know what sort of tracing
> > > you are interested in. Or I can send you this guy from my ARM test system:
> > >
> > > tools/testing/selftests/rcutorture/res/2026.07.31-10.25.08-torture/results-rcutorture-kcsan/TREE01/.config
> > >
> > > Thanx, Paul
> > >
> > > ------------------------------------------------------------------------
> > >
> > > [ 738.696166] BUG: KCSAN: data-race in __delay / set_need_resched_current
> > > [ 738.696184]
> > > [ 738.696188] read (marked) to 0xffff000005899b48 of 8 bytes by interrupt on cpu 8:
> > > [ 738.696198] __delay+0xb0/0x378
> > > [ 738.696212] __udelay+0x4c/0x60
> > > [ 738.696225] kcsan_setup_watchpoint+0x3b4/0x820
> > > [ 738.696238] __tsan_unaligned_write4+0x228/0x26c
> > > [ 738.696249] set_need_resched_current+0x138/0x1a8
> > > [ 738.696260] rcu_exp_handler+0x418/0x4a0
> > > [ 738.696273] __flush_smp_call_function_queue+0x36c/0x4a0
> > > [ 738.696288] generic_smp_call_function_single_interrupt+0x20/0x30
> >
> > This is a bit weird -- it looks like the read is occuring from within
> > kcsan itself?
>
> Ah, you are right, as part of KCSAN's software watchpoints.
>
> What do you suggest?

I suppose we could add some __no_kcsan annotations to everything that is
called from the KCSAN implementation but it feels error-prone and could
hide races exposed by other callers.

I've added the KCSAN folks to see if they have any other ideas; it looks
like the sort of thing you'd expect them to have encountered already.

Cheers,

Will

[Marco Elver]

Most of KCSAN's internals are not instrumented, but do call out to
some instrumented bits where that's safe. In this case, it looks like
a real data race, even though the access comes from __delay that was
called by KCSAN, I don't think it's any less valid.

I have a fix pending [1] (queued for next merge window), however,
which would have suppressed this one.

[1] https://lore.kernel.org/all/20260722212700...@google.com/

[Will Deacon]

Hi Marco,

Thanks for the help.

Ok, but then I don't understand how these accesses can race. They appear
to be on the same CPU, in the same IPI handler.

Will

[Marco Elver]

On Fri, Aug 07, 2026 at 02:19PM +0100, Will Deacon wrote:
[...]

> Ok, but then I don't understand how these accesses can race. They appear
> to be on the same CPU, in the same IPI handler.

The only way this could happen is with an NMI, but that's not the case
here? I should have looked at the 2nd stack trace, and it seems to be
clear that this is a false positive: KCSAN sets up a watchpoint on an
address that is also accessed by __delay.

One problem with disabling KCSAN in this CPU's context is that we'd fail
to detect data races from nested interrupts.

So yes, the best way forward is to disable KCSAN in the delay
implementation. And I recall doing this for x86, which has this:

[arch/x86/lib/Makefile]
...

# KCSAN uses udelay for introducing watchpoint delay; avoid recursion.
KCSAN_SANITIZE_delay.o := n

So let's do this for arm64, too. Sorry for the noise.

------ >8 ------

From cfea3a0c12b2e685ce04c28b1bd207f0e4c05a56 Mon Sep 17 00:00:00 2001
From: Marco Elver <el...@google.com>
Date: Fri, 7 Aug 2026 13:37:32 +0000
Subject: [PATCH] arm64: Disable KCSAN instrumentation in delay.o

KCSAN relies on udelay() for injecting delays. To avoid recursively
triggering a watchpoint, where KCSAN sets up watchpoint on an address
that is accessed by udelay() in the same thread, disable instrumentation
in arm64's delay implementation.

Paul found a manifestation of this as follows:

| BUG: KCSAN: data-race in __delay / set_need_resched_current
|

| read (marked) to 0xffff000005899b48 of 8 bytes by interrupt on cpu 8:

| __delay+0xb0/0x378
| __udelay+0x4c/0x60
| kcsan_setup_watchpoint+0x3b4/0x820
| __tsan_unaligned_write4+0x228/0x26c
| set_need_resched_current+0x138/0x1a8
| rcu_exp_handler+0x418/0x4a0
| __flush_smp_call_function_queue+0x36c/0x4a0
| generic_smp_call_function_single_interrupt+0x20/0x30
| ipi_handler+0xec/0x558
| handle_percpu_devid_irq+0x220/0x2a0
| generic_handle_domain_irq+0x84/0xb4
| gic_handle_irq+0x64/0x144
| call_on_irq_stack+0x30/0x48
| do_interrupt_handler+0x80/0xb8
| el1_interrupt+0x3c/0x60
| el1h_64_irq_handler+0x18/0x24
| el1h_64_irq+0x6c/0x70
| smp_call_function_single+0x18c/0x25c
| sync_rcu_exp_select_node_cpus+0x534/0x8bc
| rcu_exp_sel_wait_wake+0x358/0xef4
| wait_rcu_exp_gp+0x30/0x44
| kthread_worker_fn+0x1b4/0x5dc
| kthread+0x1d8/0x204
| ret_from_fork+0x10/0x20
|
| write to 0xffff000005899b4c of 4 bytes by interrupt on cpu 8:
| set_need_resched_current+0x138/0x1a8
| [...]

This matches what is already done in arch/x86/lib/Makefile.

Reported-by: "Paul E. McKenney" <pau...@kernel.org>
Fixes: dd03762ab608 ("arm64: Enable KCSAN")
Signed-off-by: Marco Elver <el...@google.com>
---
arch/arm64/lib/Makefile | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
index 448c917494f3..b33e1ca4a781 100644
--- a/arch/arm64/lib/Makefile
+++ b/arch/arm64/lib/Makefile
@@ -1,4 +1,8 @@
# SPDX-License-Identifier: GPL-2.0
+
+# KCSAN uses udelay for introducing watchpoint delay; avoid recursion.
+KCSAN_SANITIZE_delay.o := n
+
lib-y := clear_user.o delay.o copy_from_user.o \
copy_to_user.o copy_page.o \
clear_page.o csum.o insn.o memchr.o memcpy.o \
--
2.55.0.654.g21b8a5bc05-goog

[Paul E. McKenney]

On Fri, Aug 07, 2026 at 01:45:28PM +0000, Marco Elver wrote:
> On Fri, Aug 07, 2026 at 02:19PM +0100, Will Deacon wrote:
> [...]
> > Ok, but then I don't understand how these accesses can race. They appear
> > to be on the same CPU, in the same IPI handler.
>
> The only way this could happen is with an NMI, but that's not the case
> here? I should have looked at the 2nd stack trace, and it seems to be
> clear that this is a false positive: KCSAN sets up a watchpoint on an
> address that is also accessed by __delay.
>
> One problem with disabling KCSAN in this CPU's context is that we'd fail
> to detect data races from nested interrupts.
>
> So yes, the best way forward is to disable KCSAN in the delay
> implementation. And I recall doing this for x86, which has this:
>
> [arch/x86/lib/Makefile]
> ...
>
> # KCSAN uses udelay for introducing watchpoint delay; avoid recursion.
> KCSAN_SANITIZE_delay.o := n
>
> So let's do this for arm64, too. Sorry for the noise.

Thank you both!

I will revert my arm64-specific patch and apply this one. Testing will
take some time, and I will get you know how it goes.

Thanx, Paul