[PATCH 0/5] Add READ_ONCE and WRITE_ONCE to Rust

[PATCH 0/5] Add READ_ONCE and WRITE_ONCE to Rust

[Alice Ryhl]

There are currently a few places in the kernel where we use volatile
reads when we really should be using `READ_ONCE`. To make it possible to
replace these with proper `READ_ONCE` calls, introduce a Rust version of
`READ_ONCE`.

A new config option CONFIG_ARCH_USE_CUSTOM_READ_ONCE is introduced so
that Rust is able to use conditional compilation to implement READ_ONCE
in terms of either a volatile read, or by calling into a C helper
function, depending on the architecture.

This series is intended to be merged through ATOMIC INFRASTRUCTURE.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Alice Ryhl (5):
      arch: add CONFIG_ARCH_USE_CUSTOM_READ_ONCE for arm64/alpha
      rust: sync: add READ_ONCE and WRITE_ONCE
      rust: sync: support using bool with READ_ONCE
      rust: hrtimer: use READ_ONCE instead of read_volatile
      rust: fs: use READ_ONCE instead of read_volatile

 MAINTAINERS                     |   2 +
 arch/Kconfig                    |  11 +++
 arch/alpha/Kconfig              |   1 +
 arch/alpha/include/asm/rwonce.h |   4 +-
 arch/arm64/Kconfig              |   1 +
 arch/arm64/include/asm/rwonce.h |   4 +-
 rust/helpers/helpers.c          |   1 +
 rust/helpers/rwonce.c           |  34 +++++++
 rust/kernel/fs/file.rs          |   8 +-
 rust/kernel/sync.rs             |   2 +
 rust/kernel/sync/rwonce.rs      | 207 ++++++++++++++++++++++++++++++++++++++++
 rust/kernel/time/hrtimer.rs     |   8 +-
 12 files changed, 268 insertions(+), 15 deletions(-)
---
base-commit: f8f9c1f4d0c7a64600e2ca312dec824a0bc2f1da
change-id: 20251230-rwonce-1e8d2ee0bcf9

Best regards,
-- 
Alice Ryhl <aliceryhl@google.com>

[PATCH 1/5] arch: add CONFIG_ARCH_USE_CUSTOM_READ_ONCE for arm64/alpha

[Alice Ryhl]

By exposing this variable as a config option, conditional compilation in
Rust code may rely on it to determine whether it should use a volatile
read or call a C helper function to perform a READ_ONCE operation.

This config option is also added on alpha for consistency, even if Rust
does not support alpha right now.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 arch/Kconfig                    | 11 +++++++++++
 arch/alpha/Kconfig              |  1 +
 arch/alpha/include/asm/rwonce.h |  4 ++--
 arch/arm64/Kconfig              |  1 +
 arch/arm64/include/asm/rwonce.h |  4 ++--
 5 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 31220f512b16d5cfbc259935c2d3675b60c1e25c..683176bb09e50e31f398bb92678283e5de66b282 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -229,6 +229,17 @@ config HAVE_EFFICIENT_UNALIGNED_ACCESS
 	  See Documentation/core-api/unaligned-memory-access.rst for more
 	  information on the topic of unaligned memory accesses.
 
+config ARCH_USE_CUSTOM_READ_ONCE
+	bool
+	help
+	  Some architectures reorder address-dependent volatile loads,
+	  which means that the default implementation of READ_ONCE that
+	  relies on a volatile load is not appropriate.
+
+	  This symbol should be selected by an architecture if it
+	  redefines READ_ONCE to use a different implementation than a
+	  volatile load.
+
 config ARCH_USE_BUILTIN_BSWAP
 	bool
 	help
diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig
index 80367f2cf821ceb4fc29485b7b21b37d5c310765..1d5d48153ba0087554221e9412a6af0c672d3f5c 100644
--- a/arch/alpha/Kconfig
+++ b/arch/alpha/Kconfig
@@ -11,6 +11,7 @@ config ALPHA
 	select ARCH_NO_PREEMPT
 	select ARCH_NO_SG_CHAIN
 	select ARCH_USE_CMPXCHG_LOCKREF
+	select ARCH_USE_CUSTOM_READ_ONCE if SMP
 	select FORCE_PCI
 	select PCI_DOMAINS if PCI
 	select PCI_SYSCALL if PCI
diff --git a/arch/alpha/include/asm/rwonce.h b/arch/alpha/include/asm/rwonce.h
index 35542bcf92b3a883df353784bcb2d243475ccd91..c9f21aa0764625b24e8957923926d09a2eb97e7c 100644
--- a/arch/alpha/include/asm/rwonce.h
+++ b/arch/alpha/include/asm/rwonce.h
@@ -5,7 +5,7 @@
 #ifndef __ASM_RWONCE_H
 #define __ASM_RWONCE_H
 
-#ifdef CONFIG_SMP
+#ifdef CONFIG_ARCH_USE_CUSTOM_READ_ONCE
 
 #include <asm/barrier.h>
 
@@ -28,7 +28,7 @@
 	(typeof(x))__x;							\
 })
 
-#endif /* CONFIG_SMP */
+#endif /* CONFIG_ARCH_USE_CUSTOM_READ_ONCE */
 
 #include <asm-generic/rwonce.h>
 
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 93173f0a09c7deb07b46ab4f16a1a0e4320dfbf1..cd16053c8302479458a05c23ba9cfb73ee50232c 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -89,6 +89,7 @@ config ARM64
 	select ARCH_KEEP_MEMBLOCK
 	select ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE
 	select ARCH_USE_CMPXCHG_LOCKREF
+	select ARCH_USE_CUSTOM_READ_ONCE if LTO
 	select ARCH_USE_GNU_PROPERTY
 	select ARCH_USE_MEMTEST
 	select ARCH_USE_QUEUED_RWLOCKS
diff --git a/arch/arm64/include/asm/rwonce.h b/arch/arm64/include/asm/rwonce.h
index 78beceec10cda47b319db29d9f79d2a5df35e92d..5da6b2d6a12399a520f7a3310014de723baa278a 100644
--- a/arch/arm64/include/asm/rwonce.h
+++ b/arch/arm64/include/asm/rwonce.h
@@ -5,7 +5,7 @@
 #ifndef __ASM_RWONCE_H
 #define __ASM_RWONCE_H
 
-#if defined(CONFIG_LTO) && !defined(__ASSEMBLER__)
+#if defined(CONFIG_ARCH_USE_CUSTOM_READ_ONCE) && !defined(__ASSEMBLER__)
 
 #include <linux/compiler_types.h>
 #include <asm/alternative-macros.h>
@@ -62,7 +62,7 @@
 })
 
 #endif	/* !BUILD_VDSO */
-#endif	/* CONFIG_LTO && !__ASSEMBLER__ */
+#endif	/* CONFIG_ARCH_USE_CUSTOM_READ_ONCE && !__ASSEMBLER__ */
 
 #include <asm-generic/rwonce.h>
 

-- 
2.52.0.351.gbe84eed79e-goog

[PATCH 2/5] rust: sync: add READ_ONCE and WRITE_ONCE

[Alice Ryhl]

There are currently a few places in the kernel where we use volatile
reads when we really should be using `READ_ONCE`. To make it possible to
replace these with proper `READ_ONCE` calls, introduce a Rust version of
`READ_ONCE`.

I've written the code to use Rust's volatile ops directly when possible.
This results in a small amount of code duplication, but I think it makes
sense for READ_ONCE and WRITE_ONCE to be implemented in pure Rust when
possible. Otherwise they would unconditionally be a function call unless
you have a system where you can perform cross-language inlining.

I considered these functions in the bindings crate instead of kernel
crate. I actually think it would make a lot of sense. But it implies
some annoying complications on old compilers since the #![feature()]
invocations in kernel/lib.rs do not apply in the bindings crate.

For now, we do not support using READ_ONCE on compound types even if
they have the right size. This can be added later.

This fails checkpatch due to a misordered MAINTAINERS entry, but this is
a pre-existing problem.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 MAINTAINERS                |   2 +
 rust/helpers/helpers.c     |   1 +
 rust/helpers/rwonce.c      |  34 ++++++++
 rust/kernel/sync.rs        |   2 +
 rust/kernel/sync/rwonce.rs | 188 +++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 227 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 12f49de7fe036c2439c00f9f4c67b2219d72a4c3..1d0cae158fe2cc7d99b6a64c11176b635e2d14e4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4117,9 +4117,11 @@ F:	arch/*/include/asm/atomic*.h
 F:	include/*/atomic*.h
 F:	include/linux/refcount.h
 F:	scripts/atomic/
+F:	rust/helpers/rwonce.c
 F:	rust/kernel/sync/atomic.rs
 F:	rust/kernel/sync/atomic/
 F:	rust/kernel/sync/refcount.rs
+F:	rust/kernel/sync/rwonce.rs
 
 ATTO EXPRESSSAS SAS/SATA RAID SCSI DRIVER
 M:	Bradley Grove <linuxdrivers@attotech.com>
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 79c72762ad9c4b473971e6210c9577860d2e2b08..28b79ca7844fb744e5ad128238824921c055ec82 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -48,6 +48,7 @@
 #include "rcu.c"
 #include "refcount.c"
 #include "regulator.c"
+#include "rwonce.c"
 #include "scatterlist.c"
 #include "security.c"
 #include "signal.c"
diff --git a/rust/helpers/rwonce.c b/rust/helpers/rwonce.c
new file mode 100644
index 0000000000000000000000000000000000000000..55c621678cd632e728cb925b6a4a2e34e2fc4884
--- /dev/null
+++ b/rust/helpers/rwonce.c
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright (C) 2025 Google LLC.
+ */
+
+#ifdef CONFIG_ARCH_USE_CUSTOM_READ_ONCE
+
+__rust_helper u8 rust_helper_read_once_1(const u8 *ptr)
+{
+	return READ_ONCE(*ptr);
+}
+
+__rust_helper u16 rust_helper_read_once_2(const u16 *ptr)
+{
+	return READ_ONCE(*ptr);
+}
+
+__rust_helper u32 rust_helper_read_once_4(const u32 *ptr)
+{
+	return READ_ONCE(*ptr);
+}
+
+__rust_helper u64 rust_helper_read_once_8(const u64 *ptr)
+{
+	return READ_ONCE(*ptr);
+}
+
+__rust_helper void *rust_helper_read_once_ptr(void * const *ptr)
+{
+	return READ_ONCE(*ptr);
+}
+
+#endif
diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index 5df87e2bd212e192b8a67644bd99f05b9d4afd75..a5bf7bdc3fa8a044786eafae39fe8844aeeef057 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -20,6 +20,7 @@
 pub mod poll;
 pub mod rcu;
 mod refcount;
+pub mod rwonce;
 mod set_once;
 
 pub use arc::{Arc, ArcBorrow, UniqueArc};
@@ -30,6 +31,7 @@
 pub use lock::spinlock::{new_spinlock, SpinLock, SpinLockGuard};
 pub use locked_by::LockedBy;
 pub use refcount::Refcount;
+pub use rwonce::{READ_ONCE, WRITE_ONCE};
 pub use set_once::SetOnce;
 
 /// Represents a lockdep class. It's a wrapper around C's `lock_class_key`.
diff --git a/rust/kernel/sync/rwonce.rs b/rust/kernel/sync/rwonce.rs
new file mode 100644
index 0000000000000000000000000000000000000000..a1660e43c9ef94011812d1816713cf031a73de1d
--- /dev/null
+++ b/rust/kernel/sync/rwonce.rs
@@ -0,0 +1,188 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2025 Google LLC.
+
+//! Rust version of the raw `READ_ONCE`/`WRITE_ONCE` functions.
+//!
+//! C header: [`include/asm-generic/rwonce.h`](srctree/include/asm-generic/rwonce.h)
+
+/// Read the pointer once.
+///
+/// # Safety
+///
+/// It must be safe to `READ_ONCE` the `ptr` with this type.
+#[inline(always)]
+#[must_use]
+#[track_caller]
+#[expect(non_snake_case)]
+pub unsafe fn READ_ONCE<T: RwOnceType>(ptr: *const T) -> T {
+    // SAFETY: It's safe to read `ptr` once with this type.
+    unsafe { T::read_once(ptr) }
+}
+
+/// Write the pointer once.
+///
+/// # Safety
+///
+/// It must be safe to `WRITE_ONCE` the `ptr` with this type.
+#[inline(always)]
+#[track_caller]
+#[expect(non_snake_case)]
+pub unsafe fn WRITE_ONCE<T: RwOnceType>(ptr: *mut T, val: T) {
+    // SAFETY: It's safe to write `ptr` once with this type.
+    unsafe { T::write_once(ptr, val) };
+}
+
+/// This module contains the generic implementations.
+#[expect(clippy::undocumented_unsafe_blocks)]
+#[expect(clippy::missing_safety_doc)]
+mod rwonce_generic_impl {
+    use core::ffi::c_void;
+    #[allow(unused_imports)]
+    use core::ptr::{read_volatile, write_volatile};
+
+    #[inline(always)]
+    #[track_caller]
+    #[cfg(not(CONFIG_ARCH_USE_CUSTOM_READ_ONCE))]
+    pub(super) unsafe fn read_once_1(ptr: *const u8) -> u8 {
+        unsafe { read_volatile::<u8>(ptr) }
+    }
+
+    #[inline(always)]
+    #[track_caller]
+    #[cfg(not(CONFIG_ARCH_USE_CUSTOM_READ_ONCE))]
+    pub(super) unsafe fn read_once_2(ptr: *const u16) -> u16 {
+        unsafe { read_volatile::<u16>(ptr) }
+    }
+
+    #[inline(always)]
+    #[track_caller]
+    #[cfg(not(CONFIG_ARCH_USE_CUSTOM_READ_ONCE))]
+    pub(super) unsafe fn read_once_4(ptr: *const u32) -> u32 {
+        unsafe { read_volatile::<u32>(ptr) }
+    }
+
+    #[inline(always)]
+    #[track_caller]
+    #[cfg(not(CONFIG_ARCH_USE_CUSTOM_READ_ONCE))]
+    pub(super) unsafe fn read_once_8(ptr: *const u64) -> u64 {
+        unsafe { read_volatile::<u64>(ptr) }
+    }
+
+    #[inline(always)]
+    #[track_caller]
+    #[cfg(not(CONFIG_ARCH_USE_CUSTOM_READ_ONCE))]
+    pub(super) unsafe fn read_once_ptr(ptr: *const *mut c_void) -> *mut c_void {
+        unsafe { read_volatile::<*mut c_void>(ptr) }
+    }
+
+    #[inline(always)]
+    #[track_caller]
+    pub(super) unsafe fn write_once_1(ptr: *mut u8, val: u8) {
+        unsafe { write_volatile::<u8>(ptr, val) }
+    }
+
+    #[inline(always)]
+    #[track_caller]
+    pub(super) unsafe fn write_once_2(ptr: *mut u16, val: u16) {
+        unsafe { write_volatile::<u16>(ptr, val) }
+    }
+
+    #[inline(always)]
+    #[track_caller]
+    pub(super) unsafe fn write_once_4(ptr: *mut u32, val: u32) {
+        unsafe { write_volatile::<u32>(ptr, val) }
+    }
+
+    #[inline(always)]
+    #[track_caller]
+    pub(super) unsafe fn write_once_8(ptr: *mut u64, val: u64) {
+        unsafe { write_volatile::<u64>(ptr, val) }
+    }
+
+    #[inline(always)]
+    #[track_caller]
+    pub(super) unsafe fn write_once_ptr(ptr: *mut *mut c_void, val: *mut c_void) {
+        unsafe { write_volatile::<*mut c_void>(ptr, val) }
+    }
+}
+use rwonce_generic_impl::*;
+
+#[cfg(CONFIG_ARCH_USE_CUSTOM_READ_ONCE)]
+use bindings::{read_once_1, read_once_2, read_once_4, read_once_8, read_once_ptr};
+
+/// Rust trait for types that may be used with `READ_ONCE`/`WRITE_ONCE`.
+///
+/// This serves a similar purpose to the `compiletime_assert_rwonce_type` macro in the C header.
+pub trait RwOnceType {
+    /// The `READ_ONCE` for this type.
+    ///
+    /// # Safety
+    ///
+    /// It must be safe to `READ_ONCE` the `ptr` with this type.
+    unsafe fn read_once(ptr: *const Self) -> Self;
+
+    /// The `WRITE_ONCE` for this type.
+    ///
+    /// # Safety
+    ///
+    /// It must be safe to `WRITE_ONCE` the `ptr` with this type.
+    unsafe fn write_once(ptr: *mut Self, val: Self);
+}
+
+macro_rules! impl_rw_once_type {
+    ($($t:ty, $read:ident, $write:ident $(, <$u:ident>)?;)*) => {$(
+        #[allow(unknown_lints, reason = "unnecessary_transmutes is unknown prior to MSRV 1.88.0")]
+        #[allow(unnecessary_transmutes)]
+        #[allow(clippy::missing_transmute_annotations)]
+        #[allow(clippy::useless_transmute)]
+        impl$(<$u>)? RwOnceType for $t {
+            #[inline(always)]
+            #[track_caller]
+            unsafe fn read_once(ptr: *const Self) -> Self {
+                // SAFETY: The caller ensures we can `READ_ONCE`.
+                //
+                // Note that `transmute` fails to compile if the two types are of different sizes.
+                unsafe { core::mem::transmute($read(ptr.cast())) }
+            }
+
+            #[inline(always)]
+            #[track_caller]
+            unsafe fn write_once(ptr: *mut Self, val: Self) {
+                // SAFETY: The caller ensures we can `WRITE_ONCE`.
+                unsafe { $write(ptr.cast(), core::mem::transmute(val)) };
+            }
+        }
+    )*}
+}
+
+// These macros determine which types may be used with rwonce, and which helper function should be
+// used if so.
+//
+// Note that `core::mem::transmute` fails the build if the source and target type have different
+// sizes, so picking the wrong helper should lead to a build error.
+
+impl_rw_once_type! {
+    u8,   read_once_1, write_once_1;
+    i8,   read_once_1, write_once_1;
+    u16,  read_once_2, write_once_2;
+    i16,  read_once_2, write_once_2;
+    u32,  read_once_4, write_once_4;
+    i32,  read_once_4, write_once_4;
+    u64,  read_once_8, write_once_8;
+    i64,  read_once_8, write_once_8;
+    *mut T, read_once_ptr, write_once_ptr, <T>;
+    *const T, read_once_ptr, write_once_ptr, <T>;
+}
+
+#[cfg(target_pointer_width = "32")]
+impl_rw_once_type! {
+    usize, read_once_4, write_once_4;
+    isize, read_once_4, write_once_4;
+}
+
+#[cfg(target_pointer_width = "64")]
+impl_rw_once_type! {
+    usize, read_once_8, write_once_8;
+    isize, read_once_8, write_once_8;
+}

-- 
2.52.0.351.gbe84eed79e-goog

Re: [PATCH 2/5] rust: sync: add READ_ONCE and WRITE_ONCE

[Andreas Hindborg]

"Alice Ryhl" <aliceryhl@google.com> writes:

> There are currently a few places in the kernel where we use volatile
> reads when we really should be using `READ_ONCE`. To make it possible to
> replace these with proper `READ_ONCE` calls, introduce a Rust version of
> `READ_ONCE`.
>
> I've written the code to use Rust's volatile ops directly when possible.
> This results in a small amount of code duplication, but I think it makes
> sense for READ_ONCE and WRITE_ONCE to be implemented in pure Rust when
> possible. Otherwise they would unconditionally be a function call unless
> you have a system where you can perform cross-language inlining.
>
> I considered these functions in the bindings crate instead of kernel
> crate. I actually think it would make a lot of sense. But it implies
> some annoying complications on old compilers since the #![feature()]
> invocations in kernel/lib.rs do not apply in the bindings crate.
>
> For now, we do not support using READ_ONCE on compound types even if
> they have the right size. This can be added later.
>
> This fails checkpatch due to a misordered MAINTAINERS entry, but this is
> a pre-existing problem.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  MAINTAINERS                |   2 +
>  rust/helpers/helpers.c     |   1 +
>  rust/helpers/rwonce.c      |  34 ++++++++
>  rust/kernel/sync.rs        |   2 +
>  rust/kernel/sync/rwonce.rs | 188 +++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 227 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 12f49de7fe036c2439c00f9f4c67b2219d72a4c3..1d0cae158fe2cc7d99b6a64c11176b635e2d14e4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4117,9 +4117,11 @@ F:	arch/*/include/asm/atomic*.h
>  F:	include/*/atomic*.h
>  F:	include/linux/refcount.h
>  F:	scripts/atomic/
> +F:	rust/helpers/rwonce.c
>  F:	rust/kernel/sync/atomic.rs
>  F:	rust/kernel/sync/atomic/
>  F:	rust/kernel/sync/refcount.rs
> +F:	rust/kernel/sync/rwonce.rs
>
>  ATTO EXPRESSSAS SAS/SATA RAID SCSI DRIVER
>  M:	Bradley Grove <linuxdrivers@attotech.com>
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 79c72762ad9c4b473971e6210c9577860d2e2b08..28b79ca7844fb744e5ad128238824921c055ec82 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -48,6 +48,7 @@
>  #include "rcu.c"
>  #include "refcount.c"
>  #include "regulator.c"
> +#include "rwonce.c"
>  #include "scatterlist.c"
>  #include "security.c"
>  #include "signal.c"
> diff --git a/rust/helpers/rwonce.c b/rust/helpers/rwonce.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..55c621678cd632e728cb925b6a4a2e34e2fc4884
> --- /dev/null
> +++ b/rust/helpers/rwonce.c
> @@ -0,0 +1,34 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Copyright (C) 2025 Google LLC.
> + */
> +
> +#ifdef CONFIG_ARCH_USE_CUSTOM_READ_ONCE
> +
> +__rust_helper u8 rust_helper_read_once_1(const u8 *ptr)
> +{
> +	return READ_ONCE(*ptr);
> +}
> +
> +__rust_helper u16 rust_helper_read_once_2(const u16 *ptr)
> +{
> +	return READ_ONCE(*ptr);
> +}
> +
> +__rust_helper u32 rust_helper_read_once_4(const u32 *ptr)
> +{
> +	return READ_ONCE(*ptr);
> +}
> +
> +__rust_helper u64 rust_helper_read_once_8(const u64 *ptr)
> +{
> +	return READ_ONCE(*ptr);
> +}
> +
> +__rust_helper void *rust_helper_read_once_ptr(void * const *ptr)
> +{
> +	return READ_ONCE(*ptr);
> +}
> +
> +#endif
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index 5df87e2bd212e192b8a67644bd99f05b9d4afd75..a5bf7bdc3fa8a044786eafae39fe8844aeeef057 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -20,6 +20,7 @@
>  pub mod poll;
>  pub mod rcu;
>  mod refcount;
> +pub mod rwonce;
>  mod set_once;
>
>  pub use arc::{Arc, ArcBorrow, UniqueArc};
> @@ -30,6 +31,7 @@
>  pub use lock::spinlock::{new_spinlock, SpinLock, SpinLockGuard};
>  pub use locked_by::LockedBy;
>  pub use refcount::Refcount;
> +pub use rwonce::{READ_ONCE, WRITE_ONCE};
>  pub use set_once::SetOnce;
>
>  /// Represents a lockdep class. It's a wrapper around C's `lock_class_key`.
> diff --git a/rust/kernel/sync/rwonce.rs b/rust/kernel/sync/rwonce.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..a1660e43c9ef94011812d1816713cf031a73de1d
> --- /dev/null
> +++ b/rust/kernel/sync/rwonce.rs
> @@ -0,0 +1,188 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +// Copyright (C) 2025 Google LLC.
> +
> +//! Rust version of the raw `READ_ONCE`/`WRITE_ONCE` functions.
> +//!
> +//! C header: [`include/asm-generic/rwonce.h`](srctree/include/asm-generic/rwonce.h)
> +
> +/// Read the pointer once.
> +///
> +/// # Safety
> +///
> +/// It must be safe to `READ_ONCE` the `ptr` with this type.
> +#[inline(always)]
> +#[must_use]
> +#[track_caller]
> +#[expect(non_snake_case)]

I really do not think we need to have screaming snake case here. I
understand that this is what the macro looks like in C code, but we do
not need to carry that over.


Best regards,
Andreas Hindborg

Re: [PATCH 2/5] rust: sync: add READ_ONCE and WRITE_ONCE

[Boqun Feng]

On Tue, Jan 06, 2026 at 01:29:11PM +0100, Andreas Hindborg wrote:
> "Alice Ryhl" <aliceryhl@google.com> writes:
> 
> > There are currently a few places in the kernel where we use volatile
> > reads when we really should be using `READ_ONCE`. To make it possible to
> > replace these with proper `READ_ONCE` calls, introduce a Rust version of
> > `READ_ONCE`.
> >
> > I've written the code to use Rust's volatile ops directly when possible.
> > This results in a small amount of code duplication, but I think it makes
> > sense for READ_ONCE and WRITE_ONCE to be implemented in pure Rust when
> > possible. Otherwise they would unconditionally be a function call unless
> > you have a system where you can perform cross-language inlining.
> >
> > I considered these functions in the bindings crate instead of kernel
> > crate. I actually think it would make a lot of sense. But it implies
> > some annoying complications on old compilers since the #![feature()]
> > invocations in kernel/lib.rs do not apply in the bindings crate.
> >
> > For now, we do not support using READ_ONCE on compound types even if
> > they have the right size. This can be added later.
> >
> > This fails checkpatch due to a misordered MAINTAINERS entry, but this is
> > a pre-existing problem.
> >
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > ---
> >  MAINTAINERS                |   2 +
> >  rust/helpers/helpers.c     |   1 +
> >  rust/helpers/rwonce.c      |  34 ++++++++
> >  rust/kernel/sync.rs        |   2 +
> >  rust/kernel/sync/rwonce.rs | 188 +++++++++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 227 insertions(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 12f49de7fe036c2439c00f9f4c67b2219d72a4c3..1d0cae158fe2cc7d99b6a64c11176b635e2d14e4 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -4117,9 +4117,11 @@ F:	arch/*/include/asm/atomic*.h
> >  F:	include/*/atomic*.h
> >  F:	include/linux/refcount.h
> >  F:	scripts/atomic/
> > +F:	rust/helpers/rwonce.c
> >  F:	rust/kernel/sync/atomic.rs
> >  F:	rust/kernel/sync/atomic/
> >  F:	rust/kernel/sync/refcount.rs
> > +F:	rust/kernel/sync/rwonce.rs
> >
> >  ATTO EXPRESSSAS SAS/SATA RAID SCSI DRIVER
> >  M:	Bradley Grove <linuxdrivers@attotech.com>
> > diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> > index 79c72762ad9c4b473971e6210c9577860d2e2b08..28b79ca7844fb744e5ad128238824921c055ec82 100644
> > --- a/rust/helpers/helpers.c
> > +++ b/rust/helpers/helpers.c
> > @@ -48,6 +48,7 @@
> >  #include "rcu.c"
> >  #include "refcount.c"
> >  #include "regulator.c"
> > +#include "rwonce.c"
> >  #include "scatterlist.c"
> >  #include "security.c"
> >  #include "signal.c"
> > diff --git a/rust/helpers/rwonce.c b/rust/helpers/rwonce.c
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..55c621678cd632e728cb925b6a4a2e34e2fc4884
> > --- /dev/null
> > +++ b/rust/helpers/rwonce.c
> > @@ -0,0 +1,34 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +/*
> > + * Copyright (C) 2025 Google LLC.
> > + */
> > +
> > +#ifdef CONFIG_ARCH_USE_CUSTOM_READ_ONCE
> > +
> > +__rust_helper u8 rust_helper_read_once_1(const u8 *ptr)
> > +{
> > +	return READ_ONCE(*ptr);
> > +}
> > +
> > +__rust_helper u16 rust_helper_read_once_2(const u16 *ptr)
> > +{
> > +	return READ_ONCE(*ptr);
> > +}
> > +
> > +__rust_helper u32 rust_helper_read_once_4(const u32 *ptr)
> > +{
> > +	return READ_ONCE(*ptr);
> > +}
> > +
> > +__rust_helper u64 rust_helper_read_once_8(const u64 *ptr)
> > +{
> > +	return READ_ONCE(*ptr);
> > +}
> > +
> > +__rust_helper void *rust_helper_read_once_ptr(void * const *ptr)
> > +{
> > +	return READ_ONCE(*ptr);
> > +}
> > +
> > +#endif
> > diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> > index 5df87e2bd212e192b8a67644bd99f05b9d4afd75..a5bf7bdc3fa8a044786eafae39fe8844aeeef057 100644
> > --- a/rust/kernel/sync.rs
> > +++ b/rust/kernel/sync.rs
> > @@ -20,6 +20,7 @@
> >  pub mod poll;
> >  pub mod rcu;
> >  mod refcount;
> > +pub mod rwonce;
> >  mod set_once;
> >
> >  pub use arc::{Arc, ArcBorrow, UniqueArc};
> > @@ -30,6 +31,7 @@
> >  pub use lock::spinlock::{new_spinlock, SpinLock, SpinLockGuard};
> >  pub use locked_by::LockedBy;
> >  pub use refcount::Refcount;
> > +pub use rwonce::{READ_ONCE, WRITE_ONCE};
> >  pub use set_once::SetOnce;
> >
> >  /// Represents a lockdep class. It's a wrapper around C's `lock_class_key`.
> > diff --git a/rust/kernel/sync/rwonce.rs b/rust/kernel/sync/rwonce.rs
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..a1660e43c9ef94011812d1816713cf031a73de1d
> > --- /dev/null
> > +++ b/rust/kernel/sync/rwonce.rs
> > @@ -0,0 +1,188 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +// Copyright (C) 2025 Google LLC.
> > +
> > +//! Rust version of the raw `READ_ONCE`/`WRITE_ONCE` functions.
> > +//!
> > +//! C header: [`include/asm-generic/rwonce.h`](srctree/include/asm-generic/rwonce.h)
> > +
> > +/// Read the pointer once.
> > +///
> > +/// # Safety
> > +///
> > +/// It must be safe to `READ_ONCE` the `ptr` with this type.
> > +#[inline(always)]
> > +#[must_use]
> > +#[track_caller]
> > +#[expect(non_snake_case)]
> 
> I really do not think we need to have screaming snake case here. I
> understand that this is what the macro looks like in C code, but we do
> not need to carry that over.
> 

We should really do a atomic_load and atomic_store (implemented by
Atomic::from_ptr()) to those usages:

    unsafe atomic_load<T: AtomicType, O: AcquireOrRelaxed>(ptr: *mut T, ordering: O) -> T {
        Atomic::from_ptr(ptr).load(ordering)
    }

    unsafe atomic_store<T: AtomicType, O: AcquireOrRelaxed>(ptr: *mut T, v: T, ordering: O) {
        Atomic::from_ptr(ptr).store(v, ordering);
    }

This unifies the underlying implementation and clarify the atomicity of
these operations.

Regards,
Boqun

> 
> Best regards,
> Andreas Hindborg
> 

[PATCH 3/5] rust: sync: support using bool with READ_ONCE

[Alice Ryhl]

Normally it is undefined behavior for a bool to take any value other
than 0 or 1. However, in the case of READ_ONCE(some_bool) is used, this
UB seems dangerous and unnecessary. I can easily imagine some Rust code
that looks like this:

	if READ_ONCE(&raw const (*my_c_struct).my_bool_field) {
	    ...
	}

And by making an analogy to what the equivalent C code is, anyone
writing this probably just meant to treat any non-zero value as true.

For WRITE_ONCE no special logic is required.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/sync/rwonce.rs | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/rust/kernel/sync/rwonce.rs b/rust/kernel/sync/rwonce.rs
index a1660e43c9ef94011812d1816713cf031a73de1d..73477f53131926996614df573b2d50fff98e624f 100644
--- a/rust/kernel/sync/rwonce.rs
+++ b/rust/kernel/sync/rwonce.rs
@@ -163,6 +163,7 @@ unsafe fn write_once(ptr: *mut Self, val: Self) {
 // sizes, so picking the wrong helper should lead to a build error.
 
 impl_rw_once_type! {
+    bool, read_once_bool, write_once_1;
     u8,   read_once_1, write_once_1;
     i8,   read_once_1, write_once_1;
     u16,  read_once_2, write_once_2;
@@ -186,3 +187,21 @@ unsafe fn write_once(ptr: *mut Self, val: Self) {
     usize, read_once_8, write_once_8;
     isize, read_once_8, write_once_8;
 }
+
+/// Read an integer as a boolean once.
+///
+/// Returns `true` if the value behind the pointer is non-zero. Otherwise returns `false`.
+///
+/// # Safety
+///
+/// It must be safe to `READ_ONCE` the `ptr` with type `u8`.
+#[inline(always)]
+#[track_caller]
+unsafe fn read_once_bool(ptr: *const bool) -> bool {
+    // Implement `read_once_bool` in terms of `read_once_1`. The arch-specific logic is inside
+    // of `read_once_1`.
+    //
+    // SAFETY: It is safe to `READ_ONCE` the `ptr` with type `u8`.
+    let byte = unsafe { read_once_1(ptr.cast::<u8>()) };
+    byte != 0u8
+}

-- 
2.52.0.351.gbe84eed79e-goog

Re: [PATCH 3/5] rust: sync: support using bool with READ_ONCE

[Gary Guo]

On Wed, 31 Dec 2025 12:22:27 +0000
Alice Ryhl <aliceryhl@google.com> wrote:

> Normally it is undefined behavior for a bool to take any value other
> than 0 or 1. However, in the case of READ_ONCE(some_bool) is used, this
> UB seems dangerous and unnecessary. I can easily imagine some Rust code
> that looks like this:
> 
> 	if READ_ONCE(&raw const (*my_c_struct).my_bool_field) {
> 	    ...
> 	}
> 
> And by making an analogy to what the equivalent C code is, anyone
> writing this probably just meant to treat any non-zero value as true.

In C, bool can only hold value `false` and `true`, too, and putting
any other value there is going to be UB.

The C language provides automatic cast so when you write an integer to it,
non-zero values will cause `true` to be written. However, you're not
allowed to cast it into a char ptr and write other values into it.

So I think there shouldn't be any special treatment to boolean type in
this regard.

Best,
Gary

> 
> For WRITE_ONCE no special logic is required.
> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/sync/rwonce.rs | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/rust/kernel/sync/rwonce.rs b/rust/kernel/sync/rwonce.rs
> index a1660e43c9ef94011812d1816713cf031a73de1d..73477f53131926996614df573b2d50fff98e624f 100644
> --- a/rust/kernel/sync/rwonce.rs
> +++ b/rust/kernel/sync/rwonce.rs
> @@ -163,6 +163,7 @@ unsafe fn write_once(ptr: *mut Self, val: Self) {
>  // sizes, so picking the wrong helper should lead to a build error.
>  
>  impl_rw_once_type! {
> +    bool, read_once_bool, write_once_1;
>      u8,   read_once_1, write_once_1;
>      i8,   read_once_1, write_once_1;
>      u16,  read_once_2, write_once_2;
> @@ -186,3 +187,21 @@ unsafe fn write_once(ptr: *mut Self, val: Self) {
>      usize, read_once_8, write_once_8;
>      isize, read_once_8, write_once_8;
>  }
> +
> +/// Read an integer as a boolean once.
> +///
> +/// Returns `true` if the value behind the pointer is non-zero. Otherwise returns `false`.
> +///
> +/// # Safety
> +///
> +/// It must be safe to `READ_ONCE` the `ptr` with type `u8`.
> +#[inline(always)]
> +#[track_caller]
> +unsafe fn read_once_bool(ptr: *const bool) -> bool {
> +    // Implement `read_once_bool` in terms of `read_once_1`. The arch-specific logic is inside
> +    // of `read_once_1`.
> +    //
> +    // SAFETY: It is safe to `READ_ONCE` the `ptr` with type `u8`.
> +    let byte = unsafe { read_once_1(ptr.cast::<u8>()) };
> +    byte != 0u8
> +}
> 

Re: [PATCH 3/5] rust: sync: support using bool with READ_ONCE

[Peter Zijlstra]

On Wed, Dec 31, 2025 at 12:22:27PM +0000, Alice Ryhl wrote:
> Normally it is undefined behavior for a bool to take any value other
> than 0 or 1. However, in the case of READ_ONCE(some_bool) is used, this
> UB seems dangerous and unnecessary. I can easily imagine some Rust code
> that looks like this:
> 
> 	if READ_ONCE(&raw const (*my_c_struct).my_bool_field) {
> 	    ...
> 	}
> 
> And by making an analogy to what the equivalent C code is, anyone
> writing this probably just meant to treat any non-zero value as true.
> 
> For WRITE_ONCE no special logic is required.
> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/sync/rwonce.rs | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/rust/kernel/sync/rwonce.rs b/rust/kernel/sync/rwonce.rs
> index a1660e43c9ef94011812d1816713cf031a73de1d..73477f53131926996614df573b2d50fff98e624f 100644
> --- a/rust/kernel/sync/rwonce.rs
> +++ b/rust/kernel/sync/rwonce.rs
> @@ -163,6 +163,7 @@ unsafe fn write_once(ptr: *mut Self, val: Self) {
>  // sizes, so picking the wrong helper should lead to a build error.
>  
>  impl_rw_once_type! {
> +    bool, read_once_bool, write_once_1;
>      u8,   read_once_1, write_once_1;
>      i8,   read_once_1, write_once_1;
>      u16,  read_once_2, write_once_2;
> @@ -186,3 +187,21 @@ unsafe fn write_once(ptr: *mut Self, val: Self) {
>      usize, read_once_8, write_once_8;
>      isize, read_once_8, write_once_8;
>  }
> +
> +/// Read an integer as a boolean once.
> +///
> +/// Returns `true` if the value behind the pointer is non-zero. Otherwise returns `false`.
> +///
> +/// # Safety
> +///
> +/// It must be safe to `READ_ONCE` the `ptr` with type `u8`.
> +#[inline(always)]
> +#[track_caller]
> +unsafe fn read_once_bool(ptr: *const bool) -> bool {
> +    // Implement `read_once_bool` in terms of `read_once_1`. The arch-specific logic is inside
> +    // of `read_once_1`.
> +    //
> +    // SAFETY: It is safe to `READ_ONCE` the `ptr` with type `u8`.
> +    let byte = unsafe { read_once_1(ptr.cast::<u8>()) };
> +    byte != 0u8
> +}

Does this hardcode that sizeof(_Bool) == 1? There are ABIs where this is
not the case.

Re: [PATCH 3/5] rust: sync: support using bool with READ_ONCE

[Alice Ryhl]

On Tue, Jan 06, 2026 at 01:43:26PM +0100, Peter Zijlstra wrote:
> On Wed, Dec 31, 2025 at 12:22:27PM +0000, Alice Ryhl wrote:
> > +/// Read an integer as a boolean once.
> > +///
> > +/// Returns `true` if the value behind the pointer is non-zero. Otherwise returns `false`.
> > +///
> > +/// # Safety
> > +///
> > +/// It must be safe to `READ_ONCE` the `ptr` with type `u8`.
> > +#[inline(always)]
> > +#[track_caller]
> > +unsafe fn read_once_bool(ptr: *const bool) -> bool {
> > +    // Implement `read_once_bool` in terms of `read_once_1`. The arch-specific logic is inside
> > +    // of `read_once_1`.
> > +    //
> > +    // SAFETY: It is safe to `READ_ONCE` the `ptr` with type `u8`.
> > +    let byte = unsafe { read_once_1(ptr.cast::<u8>()) };
> > +    byte != 0u8
> > +}
> 
> Does this hardcode that sizeof(_Bool) == 1? There are ABIs where this is
> not the case.

Hm, it hardcodes that the Rust type called bool is sizeof(_) == 1.
Presumably bindgen will not translate _Bool to bool when it appears in C
types on such platforms. But I don't really know - I have not looked
into this case.

Alice

Re: [PATCH 3/5] rust: sync: support using bool with READ_ONCE

[Gary Guo]

On Tue, 6 Jan 2026 13:43:26 +0100
Peter Zijlstra <peterz@infradead.org> wrote:

> On Wed, Dec 31, 2025 at 12:22:27PM +0000, Alice Ryhl wrote:
> > Normally it is undefined behavior for a bool to take any value other
> > than 0 or 1. However, in the case of READ_ONCE(some_bool) is used, this
> > UB seems dangerous and unnecessary. I can easily imagine some Rust code
> > that looks like this:
> > 
> > 	if READ_ONCE(&raw const (*my_c_struct).my_bool_field) {
> > 	    ...
> > 	}
> > 
> > And by making an analogy to what the equivalent C code is, anyone
> > writing this probably just meant to treat any non-zero value as true.
> > 
> > For WRITE_ONCE no special logic is required.
> > 
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > ---
> >  rust/kernel/sync/rwonce.rs | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> > 
> > diff --git a/rust/kernel/sync/rwonce.rs b/rust/kernel/sync/rwonce.rs
> > index a1660e43c9ef94011812d1816713cf031a73de1d..73477f53131926996614df573b2d50fff98e624f 100644
> > --- a/rust/kernel/sync/rwonce.rs
> > +++ b/rust/kernel/sync/rwonce.rs
> > @@ -163,6 +163,7 @@ unsafe fn write_once(ptr: *mut Self, val: Self) {
> >  // sizes, so picking the wrong helper should lead to a build error.
> >  
> >  impl_rw_once_type! {
> > +    bool, read_once_bool, write_once_1;
> >      u8,   read_once_1, write_once_1;
> >      i8,   read_once_1, write_once_1;
> >      u16,  read_once_2, write_once_2;
> > @@ -186,3 +187,21 @@ unsafe fn write_once(ptr: *mut Self, val: Self) {
> >      usize, read_once_8, write_once_8;
> >      isize, read_once_8, write_once_8;
> >  }
> > +
> > +/// Read an integer as a boolean once.
> > +///
> > +/// Returns `true` if the value behind the pointer is non-zero. Otherwise returns `false`.
> > +///
> > +/// # Safety
> > +///
> > +/// It must be safe to `READ_ONCE` the `ptr` with type `u8`.
> > +#[inline(always)]
> > +#[track_caller]
> > +unsafe fn read_once_bool(ptr: *const bool) -> bool {
> > +    // Implement `read_once_bool` in terms of `read_once_1`. The arch-specific logic is inside
> > +    // of `read_once_1`.
> > +    //
> > +    // SAFETY: It is safe to `READ_ONCE` the `ptr` with type `u8`.
> > +    let byte = unsafe { read_once_1(ptr.cast::<u8>()) };
> > +    byte != 0u8
> > +}  
> 
> Does this hardcode that sizeof(_Bool) == 1? There are ABIs where this is
> not the case.

Hi Peter,

Do you have a concrete example on which ABI/arch this is not true?

I know that the C spec doesn't mandate _Bool and char are of the same size
but we have tons of assumptions that is not guaranteed by standard C..

Best,
Gary

Re: [PATCH 3/5] rust: sync: support using bool with READ_ONCE

[Peter Zijlstra]

On Tue, Jan 06, 2026 at 06:12:01PM +0000, Gary Guo wrote:
> On Tue, 6 Jan 2026 13:43:26 +0100
> Peter Zijlstra <peterz@infradead.org> wrote:

> > Does this hardcode that sizeof(_Bool) == 1? There are ABIs where this is
> > not the case.
> 
> Hi Peter,
> 
> Do you have a concrete example on which ABI/arch this is not true?
> 
> I know that the C spec doesn't mandate _Bool and char are of the same size
> but we have tons of assumptions that is not guaranteed by standard C..

Darwin/PowerPC famously has sizeof(_Bool) == 4

Win32: Visual C++ 4.2 (and earlier) had sizeof(bool)==4 (they mapped
bool to int), while Visual C++ 5.0 introduced a native _Bool and moved
to 1 byte.

Early RISC CPUs (MIPS, PowerPC, Alpha) had severe penalties for byte
access and their compilers would've had sizeof(bool)=={4,8}.

I think AVR/Arduino also has sizeof(bool) == sizeof(int) which is 2.

Re: [PATCH 3/5] rust: sync: support using bool with READ_ONCE

[Gary Guo]

On Wed, 7 Jan 2026 09:33:27 +0100
Peter Zijlstra <peterz@infradead.org> wrote:

> On Tue, Jan 06, 2026 at 06:12:01PM +0000, Gary Guo wrote:
> > On Tue, 6 Jan 2026 13:43:26 +0100
> > Peter Zijlstra <peterz@infradead.org> wrote:  
> 
> > > Does this hardcode that sizeof(_Bool) == 1? There are ABIs where this is
> > > not the case.  
> > 
> > Hi Peter,
> > 
> > Do you have a concrete example on which ABI/arch this is not true?
> > 
> > I know that the C spec doesn't mandate _Bool and char are of the same size
> > but we have tons of assumptions that is not guaranteed by standard C..  
> 
> Darwin/PowerPC famously has sizeof(_Bool) == 4
> 
> Win32: Visual C++ 4.2 (and earlier) had sizeof(bool)==4 (they mapped
> bool to int), while Visual C++ 5.0 introduced a native _Bool and moved
> to 1 byte.
> 
> Early RISC CPUs (MIPS, PowerPC, Alpha) had severe penalties for byte
> access and their compilers would've had sizeof(bool)=={4,8}.
> 
> I think AVR/Arduino also has sizeof(bool) == sizeof(int) which is 2.
> 
> 

It sounds like that none of these matter for the kernel?

In which case I think it's good to keep the assertion; if someone is ought
to introduce a new arch where _Bool (or Rust bool) is not 1 then we should
know about it.

Best,
Gary

[PATCH 4/5] rust: hrtimer: use READ_ONCE instead of read_volatile

[Alice Ryhl]

Using `READ_ONCE` is the correct way to read the `node.expires` field.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/time/hrtimer.rs | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index 856d2d929a00892dc8eaec63cebdf547817953d3..e2b7a26f8aade972356c3eb5f6489bcda3e2e849 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -239,11 +239,9 @@ pub fn expires(&self) -> HrTimerInstant<T>
         // - Timers cannot have negative ktime_t values as their expiration time.
         // - There's no actual locking here, a racy read is fine and expected
         unsafe {
-            Instant::from_ktime(
-                // This `read_volatile` is intended to correspond to a READ_ONCE call.
-                // FIXME(read_once): Replace with `read_once` when available on the Rust side.
-                core::ptr::read_volatile(&raw const ((*c_timer_ptr).node.expires)),
-            )
+            Instant::from_ktime(kernel::sync::READ_ONCE(
+                &raw const (*c_timer_ptr).node.expires,
+            ))
         }
     }
 }

-- 
2.52.0.351.gbe84eed79e-goog

Re: [PATCH 4/5] rust: hrtimer: use READ_ONCE instead of read_volatile

[FUJITA Tomonori]

On Wed, 31 Dec 2025 12:22:28 +0000
Alice Ryhl <aliceryhl@google.com> wrote:

> Using `READ_ONCE` is the correct way to read the `node.expires` field.
> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/time/hrtimer.rs | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
> index 856d2d929a00892dc8eaec63cebdf547817953d3..e2b7a26f8aade972356c3eb5f6489bcda3e2e849 100644
> --- a/rust/kernel/time/hrtimer.rs
> +++ b/rust/kernel/time/hrtimer.rs
> @@ -239,11 +239,9 @@ pub fn expires(&self) -> HrTimerInstant<T>
>          // - Timers cannot have negative ktime_t values as their expiration time.
>          // - There's no actual locking here, a racy read is fine and expected
>          unsafe {
> -            Instant::from_ktime(
> -                // This `read_volatile` is intended to correspond to a READ_ONCE call.
> -                // FIXME(read_once): Replace with `read_once` when available on the Rust side.
> -                core::ptr::read_volatile(&raw const ((*c_timer_ptr).node.expires)),
> -            )
> +            Instant::from_ktime(kernel::sync::READ_ONCE(
> +                &raw const (*c_timer_ptr).node.expires,
> +            ))
>          }

Do we actually need READ_ONCE() here? I'm not sure but would it be
better to call the C-side API?

diff --git a/rust/helpers/time.c b/rust/helpers/time.c
index 67a36ccc3ec4..73162dea2a29 100644
--- a/rust/helpers/time.c
+++ b/rust/helpers/time.c
@@ -2,6 +2,7 @@
 
 #include <linux/delay.h>
 #include <linux/ktime.h>
+#include <linux/hrtimer.h>
 #include <linux/timekeeping.h>
 
 void rust_helper_fsleep(unsigned long usecs)
@@ -38,3 +39,8 @@ void rust_helper_udelay(unsigned long usec)
 {
 	udelay(usec);
 }
+
+__rust_helper ktime_t rust_helper_hrtimer_get_expires(const struct hrtimer *timer)
+{
+	return timer->node.expires;
+}
diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index 856d2d929a00..61e656a65216 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -237,14 +237,7 @@ pub fn expires(&self) -> HrTimerInstant<T>
 
         // SAFETY:
         // - Timers cannot have negative ktime_t values as their expiration time.
-        // - There's no actual locking here, a racy read is fine and expected
-        unsafe {
-            Instant::from_ktime(
-                // This `read_volatile` is intended to correspond to a READ_ONCE call.
-                // FIXME(read_once): Replace with `read_once` when available on the Rust side.
-                core::ptr::read_volatile(&raw const ((*c_timer_ptr).node.expires)),
-            )
-        }
+        unsafe { Instant::from_ktime(bindings::hrtimer_get_expires(c_timer_ptr)) }
     }
 }
 

Re: [PATCH 4/5] rust: hrtimer: use READ_ONCE instead of read_volatile

[FUJITA Tomonori]

On Thu, 01 Jan 2026 11:11:23 +0900 (JST)
FUJITA Tomonori <fujita.tomonori@gmail.com> wrote:

> On Wed, 31 Dec 2025 12:22:28 +0000
> Alice Ryhl <aliceryhl@google.com> wrote:
> 
>> Using `READ_ONCE` is the correct way to read the `node.expires` field.
>> 
>> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
>> ---
>>  rust/kernel/time/hrtimer.rs | 8 +++-----
>>  1 file changed, 3 insertions(+), 5 deletions(-)
>> 
>> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
>> index 856d2d929a00892dc8eaec63cebdf547817953d3..e2b7a26f8aade972356c3eb5f6489bcda3e2e849 100644
>> --- a/rust/kernel/time/hrtimer.rs
>> +++ b/rust/kernel/time/hrtimer.rs
>> @@ -239,11 +239,9 @@ pub fn expires(&self) -> HrTimerInstant<T>
>>          // - Timers cannot have negative ktime_t values as their expiration time.
>>          // - There's no actual locking here, a racy read is fine and expected
>>          unsafe {
>> -            Instant::from_ktime(
>> -                // This `read_volatile` is intended to correspond to a READ_ONCE call.
>> -                // FIXME(read_once): Replace with `read_once` when available on the Rust side.
>> -                core::ptr::read_volatile(&raw const ((*c_timer_ptr).node.expires)),
>> -            )
>> +            Instant::from_ktime(kernel::sync::READ_ONCE(
>> +                &raw const (*c_timer_ptr).node.expires,
>> +            ))
>>          }
> 
> Do we actually need READ_ONCE() here? I'm not sure but would it be
> better to call the C-side API?
> 
> diff --git a/rust/helpers/time.c b/rust/helpers/time.c
> index 67a36ccc3ec4..73162dea2a29 100644
> --- a/rust/helpers/time.c
> +++ b/rust/helpers/time.c
> @@ -2,6 +2,7 @@
>  
>  #include <linux/delay.h>
>  #include <linux/ktime.h>
> +#include <linux/hrtimer.h>
>  #include <linux/timekeeping.h>
>  
>  void rust_helper_fsleep(unsigned long usecs)
> @@ -38,3 +39,8 @@ void rust_helper_udelay(unsigned long usec)
>  {
>  	udelay(usec);
>  }
> +
> +__rust_helper ktime_t rust_helper_hrtimer_get_expires(const struct hrtimer *timer)
> +{
> +	return timer->node.expires;
> +}

Sorry, of course this should be:

+__rust_helper ktime_t rust_helper_hrtimer_get_expires(const struct hrtimer *timer)
+{
+	return hrtimer_get_expires(timer);
+}

> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
> index 856d2d929a00..61e656a65216 100644
> --- a/rust/kernel/time/hrtimer.rs
> +++ b/rust/kernel/time/hrtimer.rs
> @@ -237,14 +237,7 @@ pub fn expires(&self) -> HrTimerInstant<T>
>  
>          // SAFETY:
>          // - Timers cannot have negative ktime_t values as their expiration time.
> -        // - There's no actual locking here, a racy read is fine and expected
> -        unsafe {
> -            Instant::from_ktime(
> -                // This `read_volatile` is intended to correspond to a READ_ONCE call.
> -                // FIXME(read_once): Replace with `read_once` when available on the Rust side.
> -                core::ptr::read_volatile(&raw const ((*c_timer_ptr).node.expires)),
> -            )
> -        }
> +        unsafe { Instant::from_ktime(bindings::hrtimer_get_expires(c_timer_ptr)) }
>      }
>  }
>  
> 

Re: [PATCH 4/5] rust: hrtimer: use READ_ONCE instead of read_volatile

[Andreas Hindborg]

"FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:

> On Thu, 01 Jan 2026 11:11:23 +0900 (JST)
> FUJITA Tomonori <fujita.tomonori@gmail.com> wrote:
>
>> On Wed, 31 Dec 2025 12:22:28 +0000
>> Alice Ryhl <aliceryhl@google.com> wrote:
>>
>>> Using `READ_ONCE` is the correct way to read the `node.expires` field.
>>>
>>> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
>>> ---
>>>  rust/kernel/time/hrtimer.rs | 8 +++-----
>>>  1 file changed, 3 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
>>> index 856d2d929a00892dc8eaec63cebdf547817953d3..e2b7a26f8aade972356c3eb5f6489bcda3e2e849 100644
>>> --- a/rust/kernel/time/hrtimer.rs
>>> +++ b/rust/kernel/time/hrtimer.rs
>>> @@ -239,11 +239,9 @@ pub fn expires(&self) -> HrTimerInstant<T>
>>>          // - Timers cannot have negative ktime_t values as their expiration time.
>>>          // - There's no actual locking here, a racy read is fine and expected
>>>          unsafe {
>>> -            Instant::from_ktime(
>>> -                // This `read_volatile` is intended to correspond to a READ_ONCE call.
>>> -                // FIXME(read_once): Replace with `read_once` when available on the Rust side.
>>> -                core::ptr::read_volatile(&raw const ((*c_timer_ptr).node.expires)),
>>> -            )
>>> +            Instant::from_ktime(kernel::sync::READ_ONCE(
>>> +                &raw const (*c_timer_ptr).node.expires,
>>> +            ))
>>>          }
>>
>> Do we actually need READ_ONCE() here? I'm not sure but would it be
>> better to call the C-side API?
>>
>> diff --git a/rust/helpers/time.c b/rust/helpers/time.c
>> index 67a36ccc3ec4..73162dea2a29 100644
>> --- a/rust/helpers/time.c
>> +++ b/rust/helpers/time.c
>> @@ -2,6 +2,7 @@
>>
>>  #include <linux/delay.h>
>>  #include <linux/ktime.h>
>> +#include <linux/hrtimer.h>
>>  #include <linux/timekeeping.h>
>>
>>  void rust_helper_fsleep(unsigned long usecs)
>> @@ -38,3 +39,8 @@ void rust_helper_udelay(unsigned long usec)
>>  {
>>  	udelay(usec);
>>  }
>> +
>> +__rust_helper ktime_t rust_helper_hrtimer_get_expires(const struct hrtimer *timer)
>> +{
>> +	return timer->node.expires;
>> +}
>
> Sorry, of course this should be:
>
> +__rust_helper ktime_t rust_helper_hrtimer_get_expires(const struct hrtimer *timer)
> +{
> +	return hrtimer_get_expires(timer);
> +}
>

This is a potentially racy read. As far as I recall, we determined that
using read_once is the proper way to handle the situation.

I do not think it makes a difference that the read is done by C code.


Best regards,
Andreas Hindborg

Re: [PATCH 4/5] rust: hrtimer: use READ_ONCE instead of read_volatile

[FUJITA Tomonori]

On Tue, 06 Jan 2026 13:37:34 +0100
Andreas Hindborg <a.hindborg@kernel.org> wrote:

> "FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:
> 
>> On Thu, 01 Jan 2026 11:11:23 +0900 (JST)
>> FUJITA Tomonori <fujita.tomonori@gmail.com> wrote:
>>
>>> On Wed, 31 Dec 2025 12:22:28 +0000
>>> Alice Ryhl <aliceryhl@google.com> wrote:
>>>
>>>> Using `READ_ONCE` is the correct way to read the `node.expires` field.
>>>>
>>>> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
>>>> ---
>>>>  rust/kernel/time/hrtimer.rs | 8 +++-----
>>>>  1 file changed, 3 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
>>>> index 856d2d929a00892dc8eaec63cebdf547817953d3..e2b7a26f8aade972356c3eb5f6489bcda3e2e849 100644
>>>> --- a/rust/kernel/time/hrtimer.rs
>>>> +++ b/rust/kernel/time/hrtimer.rs
>>>> @@ -239,11 +239,9 @@ pub fn expires(&self) -> HrTimerInstant<T>
>>>>          // - Timers cannot have negative ktime_t values as their expiration time.
>>>>          // - There's no actual locking here, a racy read is fine and expected
>>>>          unsafe {
>>>> -            Instant::from_ktime(
>>>> -                // This `read_volatile` is intended to correspond to a READ_ONCE call.
>>>> -                // FIXME(read_once): Replace with `read_once` when available on the Rust side.
>>>> -                core::ptr::read_volatile(&raw const ((*c_timer_ptr).node.expires)),
>>>> -            )
>>>> +            Instant::from_ktime(kernel::sync::READ_ONCE(
>>>> +                &raw const (*c_timer_ptr).node.expires,
>>>> +            ))
>>>>          }
>>>
>>> Do we actually need READ_ONCE() here? I'm not sure but would it be
>>> better to call the C-side API?
>>>
>>> diff --git a/rust/helpers/time.c b/rust/helpers/time.c
>>> index 67a36ccc3ec4..73162dea2a29 100644
>>> --- a/rust/helpers/time.c
>>> +++ b/rust/helpers/time.c
>>> @@ -2,6 +2,7 @@
>>>
>>>  #include <linux/delay.h>
>>>  #include <linux/ktime.h>
>>> +#include <linux/hrtimer.h>
>>>  #include <linux/timekeeping.h>
>>>
>>>  void rust_helper_fsleep(unsigned long usecs)
>>> @@ -38,3 +39,8 @@ void rust_helper_udelay(unsigned long usec)
>>>  {
>>>  	udelay(usec);
>>>  }
>>> +
>>> +__rust_helper ktime_t rust_helper_hrtimer_get_expires(const struct hrtimer *timer)
>>> +{
>>> +	return timer->node.expires;
>>> +}
>>
>> Sorry, of course this should be:
>>
>> +__rust_helper ktime_t rust_helper_hrtimer_get_expires(const struct hrtimer *timer)
>> +{
>> +	return hrtimer_get_expires(timer);
>> +}
>>
> 
> This is a potentially racy read. As far as I recall, we determined that
> using read_once is the proper way to handle the situation.
> 
> I do not think it makes a difference that the read is done by C code.

What does "racy read" mean here?

The C side doesn't use WRITE_ONCE() or READ_ONCE for node.expires. How
would using READ_ONCE() on the Rust side make a difference?

Re: [PATCH 4/5] rust: hrtimer: use READ_ONCE instead of read_volatile

[Andreas Hindborg]

FUJITA Tomonori <fujita.tomonori@gmail.com> writes:

> On Tue, 06 Jan 2026 13:37:34 +0100
> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
>> "FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:
>> 
>>> On Thu, 01 Jan 2026 11:11:23 +0900 (JST)
>>> FUJITA Tomonori <fujita.tomonori@gmail.com> wrote:
>>>
>>>> On Wed, 31 Dec 2025 12:22:28 +0000
>>>> Alice Ryhl <aliceryhl@google.com> wrote:
>>>>
>>>>> Using `READ_ONCE` is the correct way to read the `node.expires` field.
>>>>>
>>>>> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
>>>>> ---
>>>>>  rust/kernel/time/hrtimer.rs | 8 +++-----
>>>>>  1 file changed, 3 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
>>>>> index 856d2d929a00892dc8eaec63cebdf547817953d3..e2b7a26f8aade972356c3eb5f6489bcda3e2e849 100644
>>>>> --- a/rust/kernel/time/hrtimer.rs
>>>>> +++ b/rust/kernel/time/hrtimer.rs
>>>>> @@ -239,11 +239,9 @@ pub fn expires(&self) -> HrTimerInstant<T>
>>>>>          // - Timers cannot have negative ktime_t values as their expiration time.
>>>>>          // - There's no actual locking here, a racy read is fine and expected
>>>>>          unsafe {
>>>>> -            Instant::from_ktime(
>>>>> -                // This `read_volatile` is intended to correspond to a READ_ONCE call.
>>>>> -                // FIXME(read_once): Replace with `read_once` when available on the Rust side.
>>>>> -                core::ptr::read_volatile(&raw const ((*c_timer_ptr).node.expires)),
>>>>> -            )
>>>>> +            Instant::from_ktime(kernel::sync::READ_ONCE(
>>>>> +                &raw const (*c_timer_ptr).node.expires,
>>>>> +            ))
>>>>>          }
>>>>
>>>> Do we actually need READ_ONCE() here? I'm not sure but would it be
>>>> better to call the C-side API?
>>>>
>>>> diff --git a/rust/helpers/time.c b/rust/helpers/time.c
>>>> index 67a36ccc3ec4..73162dea2a29 100644
>>>> --- a/rust/helpers/time.c
>>>> +++ b/rust/helpers/time.c
>>>> @@ -2,6 +2,7 @@
>>>>
>>>>  #include <linux/delay.h>
>>>>  #include <linux/ktime.h>
>>>> +#include <linux/hrtimer.h>
>>>>  #include <linux/timekeeping.h>
>>>>
>>>>  void rust_helper_fsleep(unsigned long usecs)
>>>> @@ -38,3 +39,8 @@ void rust_helper_udelay(unsigned long usec)
>>>>  {
>>>>  	udelay(usec);
>>>>  }
>>>> +
>>>> +__rust_helper ktime_t rust_helper_hrtimer_get_expires(const struct hrtimer *timer)
>>>> +{
>>>> +	return timer->node.expires;
>>>> +}
>>>
>>> Sorry, of course this should be:
>>>
>>> +__rust_helper ktime_t rust_helper_hrtimer_get_expires(const struct hrtimer *timer)
>>> +{
>>> +	return hrtimer_get_expires(timer);
>>> +}
>>>
>> 
>> This is a potentially racy read. As far as I recall, we determined that
>> using read_once is the proper way to handle the situation.
>> 
>> I do not think it makes a difference that the read is done by C code.
>
> What does "racy read" mean here?
>
> The C side doesn't use WRITE_ONCE() or READ_ONCE for node.expires. How
> would using READ_ONCE() on the Rust side make a difference?

Data races like this are UB in Rust. As far as I understand, using this
READ_ONCE implementation or a relaxed atomic read would make the read
well defined. I am not aware if this is only the case if all writes to
the location from C also use atomic operations or WRITE_ONCE. @Boqun?


Best regards,
Andreas Hindborg

Re: [PATCH 4/5] rust: hrtimer: use READ_ONCE instead of read_volatile

[FUJITA Tomonori]

On Wed, 07 Jan 2026 11:11:43 +0100
Andreas Hindborg <a.hindborg@kernel.org> wrote:

> FUJITA Tomonori <fujita.tomonori@gmail.com> writes:
> 
>> On Tue, 06 Jan 2026 13:37:34 +0100
>> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>>> "FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:
>>> 
>>>> On Thu, 01 Jan 2026 11:11:23 +0900 (JST)
>>>> FUJITA Tomonori <fujita.tomonori@gmail.com> wrote:
>>>>
>>>>> On Wed, 31 Dec 2025 12:22:28 +0000
>>>>> Alice Ryhl <aliceryhl@google.com> wrote:
>>>>>
>>>>>> Using `READ_ONCE` is the correct way to read the `node.expires` field.
>>>>>>
>>>>>> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
>>>>>> ---
>>>>>>  rust/kernel/time/hrtimer.rs | 8 +++-----
>>>>>>  1 file changed, 3 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
>>>>>> index 856d2d929a00892dc8eaec63cebdf547817953d3..e2b7a26f8aade972356c3eb5f6489bcda3e2e849 100644
>>>>>> --- a/rust/kernel/time/hrtimer.rs
>>>>>> +++ b/rust/kernel/time/hrtimer.rs
>>>>>> @@ -239,11 +239,9 @@ pub fn expires(&self) -> HrTimerInstant<T>
>>>>>>          // - Timers cannot have negative ktime_t values as their expiration time.
>>>>>>          // - There's no actual locking here, a racy read is fine and expected
>>>>>>          unsafe {
>>>>>> -            Instant::from_ktime(
>>>>>> -                // This `read_volatile` is intended to correspond to a READ_ONCE call.
>>>>>> -                // FIXME(read_once): Replace with `read_once` when available on the Rust side.
>>>>>> -                core::ptr::read_volatile(&raw const ((*c_timer_ptr).node.expires)),
>>>>>> -            )
>>>>>> +            Instant::from_ktime(kernel::sync::READ_ONCE(
>>>>>> +                &raw const (*c_timer_ptr).node.expires,
>>>>>> +            ))
>>>>>>          }
>>>>>
>>>>> Do we actually need READ_ONCE() here? I'm not sure but would it be
>>>>> better to call the C-side API?
>>>>>
>>>>> diff --git a/rust/helpers/time.c b/rust/helpers/time.c
>>>>> index 67a36ccc3ec4..73162dea2a29 100644
>>>>> --- a/rust/helpers/time.c
>>>>> +++ b/rust/helpers/time.c
>>>>> @@ -2,6 +2,7 @@
>>>>>
>>>>>  #include <linux/delay.h>
>>>>>  #include <linux/ktime.h>
>>>>> +#include <linux/hrtimer.h>
>>>>>  #include <linux/timekeeping.h>
>>>>>
>>>>>  void rust_helper_fsleep(unsigned long usecs)
>>>>> @@ -38,3 +39,8 @@ void rust_helper_udelay(unsigned long usec)
>>>>>  {
>>>>>  	udelay(usec);
>>>>>  }
>>>>> +
>>>>> +__rust_helper ktime_t rust_helper_hrtimer_get_expires(const struct hrtimer *timer)
>>>>> +{
>>>>> +	return timer->node.expires;
>>>>> +}
>>>>
>>>> Sorry, of course this should be:
>>>>
>>>> +__rust_helper ktime_t rust_helper_hrtimer_get_expires(const struct hrtimer *timer)
>>>> +{
>>>> +	return hrtimer_get_expires(timer);
>>>> +}
>>>>
>>> 
>>> This is a potentially racy read. As far as I recall, we determined that
>>> using read_once is the proper way to handle the situation.
>>> 
>>> I do not think it makes a difference that the read is done by C code.
>>
>> What does "racy read" mean here?
>>
>> The C side doesn't use WRITE_ONCE() or READ_ONCE for node.expires. How
>> would using READ_ONCE() on the Rust side make a difference?
> 
> Data races like this are UB in Rust. As far as I understand, using this
> READ_ONCE implementation or a relaxed atomic read would make the read
> well defined. I am not aware if this is only the case if all writes to
> the location from C also use atomic operations or WRITE_ONCE. @Boqun?

The C side updates node.expires without WRITE_ONCE()/atomics so a
Rust-side READ_ONCE() can still observe a torn value; I think that
this is still a data race / UB from Rust's perspective.

And since expires is 64-bit, WRITE_ONCE() on 32-bit architectures does
not inherently guarantee tear-free stores either.

I think that the expires() method should follow the same safety
requirements as raw_forward(): it should only be considered safe when
holding exclusive access to hrtimer or within the context of the timer
callback. Under those conditions, it would be fine to call C's
hrtimer_get_expires().

Re: [PATCH 4/5] rust: hrtimer: use READ_ONCE instead of read_volatile

[Andreas Hindborg]

"FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:

> On Wed, 07 Jan 2026 11:11:43 +0100
> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
>> FUJITA Tomonori <fujita.tomonori@gmail.com> writes:
>>
>>> On Tue, 06 Jan 2026 13:37:34 +0100
>>> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>>
>>>> "FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:
>>>>
>>>>> On Thu, 01 Jan 2026 11:11:23 +0900 (JST)
>>>>> FUJITA Tomonori <fujita.tomonori@gmail.com> wrote:
>>>>>
>>>>>> On Wed, 31 Dec 2025 12:22:28 +0000
>>>>>> Alice Ryhl <aliceryhl@google.com> wrote:
>>>>>>
>>>>>>> Using `READ_ONCE` is the correct way to read the `node.expires` field.
>>>>>>>
>>>>>>> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
>>>>>>> ---
>>>>>>>  rust/kernel/time/hrtimer.rs | 8 +++-----
>>>>>>>  1 file changed, 3 insertions(+), 5 deletions(-)
>>>>>>>
>>>>>>> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
>>>>>>> index 856d2d929a00892dc8eaec63cebdf547817953d3..e2b7a26f8aade972356c3eb5f6489bcda3e2e849 100644
>>>>>>> --- a/rust/kernel/time/hrtimer.rs
>>>>>>> +++ b/rust/kernel/time/hrtimer.rs
>>>>>>> @@ -239,11 +239,9 @@ pub fn expires(&self) -> HrTimerInstant<T>
>>>>>>>          // - Timers cannot have negative ktime_t values as their expiration time.
>>>>>>>          // - There's no actual locking here, a racy read is fine and expected
>>>>>>>          unsafe {
>>>>>>> -            Instant::from_ktime(
>>>>>>> -                // This `read_volatile` is intended to correspond to a READ_ONCE call.
>>>>>>> -                // FIXME(read_once): Replace with `read_once` when available on the Rust side.
>>>>>>> -                core::ptr::read_volatile(&raw const ((*c_timer_ptr).node.expires)),
>>>>>>> -            )
>>>>>>> +            Instant::from_ktime(kernel::sync::READ_ONCE(
>>>>>>> +                &raw const (*c_timer_ptr).node.expires,
>>>>>>> +            ))
>>>>>>>          }
>>>>>>
>>>>>> Do we actually need READ_ONCE() here? I'm not sure but would it be
>>>>>> better to call the C-side API?
>>>>>>
>>>>>> diff --git a/rust/helpers/time.c b/rust/helpers/time.c
>>>>>> index 67a36ccc3ec4..73162dea2a29 100644
>>>>>> --- a/rust/helpers/time.c
>>>>>> +++ b/rust/helpers/time.c
>>>>>> @@ -2,6 +2,7 @@
>>>>>>
>>>>>>  #include <linux/delay.h>
>>>>>>  #include <linux/ktime.h>
>>>>>> +#include <linux/hrtimer.h>
>>>>>>  #include <linux/timekeeping.h>
>>>>>>
>>>>>>  void rust_helper_fsleep(unsigned long usecs)
>>>>>> @@ -38,3 +39,8 @@ void rust_helper_udelay(unsigned long usec)
>>>>>>  {
>>>>>>  	udelay(usec);
>>>>>>  }
>>>>>> +
>>>>>> +__rust_helper ktime_t rust_helper_hrtimer_get_expires(const struct hrtimer *timer)
>>>>>> +{
>>>>>> +	return timer->node.expires;
>>>>>> +}
>>>>>
>>>>> Sorry, of course this should be:
>>>>>
>>>>> +__rust_helper ktime_t rust_helper_hrtimer_get_expires(const struct hrtimer *timer)
>>>>> +{
>>>>> +	return hrtimer_get_expires(timer);
>>>>> +}
>>>>>
>>>>
>>>> This is a potentially racy read. As far as I recall, we determined that
>>>> using read_once is the proper way to handle the situation.
>>>>
>>>> I do not think it makes a difference that the read is done by C code.
>>>
>>> What does "racy read" mean here?
>>>
>>> The C side doesn't use WRITE_ONCE() or READ_ONCE for node.expires. How
>>> would using READ_ONCE() on the Rust side make a difference?
>>
>> Data races like this are UB in Rust. As far as I understand, using this
>> READ_ONCE implementation or a relaxed atomic read would make the read
>> well defined. I am not aware if this is only the case if all writes to
>> the location from C also use atomic operations or WRITE_ONCE. @Boqun?
>
> The C side updates node.expires without WRITE_ONCE()/atomics so a
> Rust-side READ_ONCE() can still observe a torn value; I think that
> this is still a data race / UB from Rust's perspective.
>
> And since expires is 64-bit, WRITE_ONCE() on 32-bit architectures does
> not inherently guarantee tear-free stores either.
>
> I think that the expires() method should follow the same safety
> requirements as raw_forward(): it should only be considered safe when
> holding exclusive access to hrtimer or within the context of the timer
> callback. Under those conditions, it would be fine to call C's
> hrtimer_get_expires().

We can make it safe, please see my comment here [1].

Best regards,
Andreas Hindborg

[1] https://lore.kernel.org/r/87v7hdh9m4.fsf@t14s.mail-host-address-is-not-set

Re: [PATCH 4/5] rust: hrtimer: use READ_ONCE instead of read_volatile

[FUJITA Tomonori]

On Wed, 07 Jan 2026 19:21:11 +0100
Andreas Hindborg <a.hindborg@kernel.org> wrote:

> "FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:
> 
>> On Wed, 07 Jan 2026 11:11:43 +0100
>> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>>> FUJITA Tomonori <fujita.tomonori@gmail.com> writes:
>>>
>>>> On Tue, 06 Jan 2026 13:37:34 +0100
>>>> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>>>
>>>>> "FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:
>>>>>
>>>>>> On Thu, 01 Jan 2026 11:11:23 +0900 (JST)
>>>>>> FUJITA Tomonori <fujita.tomonori@gmail.com> wrote:
>>>>>>
>>>>>>> On Wed, 31 Dec 2025 12:22:28 +0000
>>>>>>> Alice Ryhl <aliceryhl@google.com> wrote:
>>>>>>>
>>>>>>>> Using `READ_ONCE` is the correct way to read the `node.expires` field.
>>>>>>>>
>>>>>>>> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
>>>>>>>> ---
>>>>>>>>  rust/kernel/time/hrtimer.rs | 8 +++-----
>>>>>>>>  1 file changed, 3 insertions(+), 5 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
>>>>>>>> index 856d2d929a00892dc8eaec63cebdf547817953d3..e2b7a26f8aade972356c3eb5f6489bcda3e2e849 100644
>>>>>>>> --- a/rust/kernel/time/hrtimer.rs
>>>>>>>> +++ b/rust/kernel/time/hrtimer.rs
>>>>>>>> @@ -239,11 +239,9 @@ pub fn expires(&self) -> HrTimerInstant<T>
>>>>>>>>          // - Timers cannot have negative ktime_t values as their expiration time.
>>>>>>>>          // - There's no actual locking here, a racy read is fine and expected
>>>>>>>>          unsafe {
>>>>>>>> -            Instant::from_ktime(
>>>>>>>> -                // This `read_volatile` is intended to correspond to a READ_ONCE call.
>>>>>>>> -                // FIXME(read_once): Replace with `read_once` when available on the Rust side.
>>>>>>>> -                core::ptr::read_volatile(&raw const ((*c_timer_ptr).node.expires)),
>>>>>>>> -            )
>>>>>>>> +            Instant::from_ktime(kernel::sync::READ_ONCE(
>>>>>>>> +                &raw const (*c_timer_ptr).node.expires,
>>>>>>>> +            ))
>>>>>>>>          }
>>>>>>>
>>>>>>> Do we actually need READ_ONCE() here? I'm not sure but would it be
>>>>>>> better to call the C-side API?
>>>>>>>
>>>>>>> diff --git a/rust/helpers/time.c b/rust/helpers/time.c
>>>>>>> index 67a36ccc3ec4..73162dea2a29 100644
>>>>>>> --- a/rust/helpers/time.c
>>>>>>> +++ b/rust/helpers/time.c
>>>>>>> @@ -2,6 +2,7 @@
>>>>>>>
>>>>>>>  #include <linux/delay.h>
>>>>>>>  #include <linux/ktime.h>
>>>>>>> +#include <linux/hrtimer.h>
>>>>>>>  #include <linux/timekeeping.h>
>>>>>>>
>>>>>>>  void rust_helper_fsleep(unsigned long usecs)
>>>>>>> @@ -38,3 +39,8 @@ void rust_helper_udelay(unsigned long usec)
>>>>>>>  {
>>>>>>>  	udelay(usec);
>>>>>>>  }
>>>>>>> +
>>>>>>> +__rust_helper ktime_t rust_helper_hrtimer_get_expires(const struct hrtimer *timer)
>>>>>>> +{
>>>>>>> +	return timer->node.expires;
>>>>>>> +}
>>>>>>
>>>>>> Sorry, of course this should be:
>>>>>>
>>>>>> +__rust_helper ktime_t rust_helper_hrtimer_get_expires(const struct hrtimer *timer)
>>>>>> +{
>>>>>> +	return hrtimer_get_expires(timer);
>>>>>> +}
>>>>>>
>>>>>
>>>>> This is a potentially racy read. As far as I recall, we determined that
>>>>> using read_once is the proper way to handle the situation.
>>>>>
>>>>> I do not think it makes a difference that the read is done by C code.
>>>>
>>>> What does "racy read" mean here?
>>>>
>>>> The C side doesn't use WRITE_ONCE() or READ_ONCE for node.expires. How
>>>> would using READ_ONCE() on the Rust side make a difference?
>>>
>>> Data races like this are UB in Rust. As far as I understand, using this
>>> READ_ONCE implementation or a relaxed atomic read would make the read
>>> well defined. I am not aware if this is only the case if all writes to
>>> the location from C also use atomic operations or WRITE_ONCE. @Boqun?
>>
>> The C side updates node.expires without WRITE_ONCE()/atomics so a
>> Rust-side READ_ONCE() can still observe a torn value; I think that
>> this is still a data race / UB from Rust's perspective.
>>
>> And since expires is 64-bit, WRITE_ONCE() on 32-bit architectures does
>> not inherently guarantee tear-free stores either.
>>
>> I think that the expires() method should follow the same safety
>> requirements as raw_forward(): it should only be considered safe when
>> holding exclusive access to hrtimer or within the context of the timer
>> callback. Under those conditions, it would be fine to call C's
>> hrtimer_get_expires().
> 
> We can make it safe, please see my comment here [1].
> 
> Best regards,
> Andreas Hindborg
> 
> [1] https://lore.kernel.org/r/87v7hdh9m4.fsf@t14s.mail-host-address-is-not-set

I agree. My point was that expire() can be safe only under the same
constraints as forward()/forward_now() so the API should require
Pin<&mut Self> and expose it on HrTimerCallbackContext.

Re: [PATCH 4/5] rust: hrtimer: use READ_ONCE instead of read_volatile

[Andreas Hindborg]

"FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:

> On Wed, 07 Jan 2026 19:21:11 +0100
> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
>> "FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:
>>
>>> On Wed, 07 Jan 2026 11:11:43 +0100
>>> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>>
>>>> FUJITA Tomonori <fujita.tomonori@gmail.com> writes:
>>>>
>>>>> On Tue, 06 Jan 2026 13:37:34 +0100
>>>>> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>>>>
>>>>>> "FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:
>>>>>>
>>>>>>> On Thu, 01 Jan 2026 11:11:23 +0900 (JST)
>>>>>>> FUJITA Tomonori <fujita.tomonori@gmail.com> wrote:
>>>>>>>
>>>>>>>> On Wed, 31 Dec 2025 12:22:28 +0000
>>>>>>>> Alice Ryhl <aliceryhl@google.com> wrote:
>>>>>>>>
>>>>>>>>> Using `READ_ONCE` is the correct way to read the `node.expires` field.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
>>>>>>>>> ---
>>>>>>>>>  rust/kernel/time/hrtimer.rs | 8 +++-----
>>>>>>>>>  1 file changed, 3 insertions(+), 5 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
>>>>>>>>> index 856d2d929a00892dc8eaec63cebdf547817953d3..e2b7a26f8aade972356c3eb5f6489bcda3e2e849 100644
>>>>>>>>> --- a/rust/kernel/time/hrtimer.rs
>>>>>>>>> +++ b/rust/kernel/time/hrtimer.rs
>>>>>>>>> @@ -239,11 +239,9 @@ pub fn expires(&self) -> HrTimerInstant<T>
>>>>>>>>>          // - Timers cannot have negative ktime_t values as their expiration time.
>>>>>>>>>          // - There's no actual locking here, a racy read is fine and expected
>>>>>>>>>          unsafe {
>>>>>>>>> -            Instant::from_ktime(
>>>>>>>>> -                // This `read_volatile` is intended to correspond to a READ_ONCE call.
>>>>>>>>> -                // FIXME(read_once): Replace with `read_once` when available on the Rust side.
>>>>>>>>> -                core::ptr::read_volatile(&raw const ((*c_timer_ptr).node.expires)),
>>>>>>>>> -            )
>>>>>>>>> +            Instant::from_ktime(kernel::sync::READ_ONCE(
>>>>>>>>> +                &raw const (*c_timer_ptr).node.expires,
>>>>>>>>> +            ))
>>>>>>>>>          }
>>>>>>>>
>>>>>>>> Do we actually need READ_ONCE() here? I'm not sure but would it be
>>>>>>>> better to call the C-side API?
>>>>>>>>
>>>>>>>> diff --git a/rust/helpers/time.c b/rust/helpers/time.c
>>>>>>>> index 67a36ccc3ec4..73162dea2a29 100644
>>>>>>>> --- a/rust/helpers/time.c
>>>>>>>> +++ b/rust/helpers/time.c
>>>>>>>> @@ -2,6 +2,7 @@
>>>>>>>>
>>>>>>>>  #include <linux/delay.h>
>>>>>>>>  #include <linux/ktime.h>
>>>>>>>> +#include <linux/hrtimer.h>
>>>>>>>>  #include <linux/timekeeping.h>
>>>>>>>>
>>>>>>>>  void rust_helper_fsleep(unsigned long usecs)
>>>>>>>> @@ -38,3 +39,8 @@ void rust_helper_udelay(unsigned long usec)
>>>>>>>>  {
>>>>>>>>  	udelay(usec);
>>>>>>>>  }
>>>>>>>> +
>>>>>>>> +__rust_helper ktime_t rust_helper_hrtimer_get_expires(const struct hrtimer *timer)
>>>>>>>> +{
>>>>>>>> +	return timer->node.expires;
>>>>>>>> +}
>>>>>>>
>>>>>>> Sorry, of course this should be:
>>>>>>>
>>>>>>> +__rust_helper ktime_t rust_helper_hrtimer_get_expires(const struct hrtimer *timer)
>>>>>>> +{
>>>>>>> +	return hrtimer_get_expires(timer);
>>>>>>> +}
>>>>>>>
>>>>>>
>>>>>> This is a potentially racy read. As far as I recall, we determined that
>>>>>> using read_once is the proper way to handle the situation.
>>>>>>
>>>>>> I do not think it makes a difference that the read is done by C code.
>>>>>
>>>>> What does "racy read" mean here?
>>>>>
>>>>> The C side doesn't use WRITE_ONCE() or READ_ONCE for node.expires. How
>>>>> would using READ_ONCE() on the Rust side make a difference?
>>>>
>>>> Data races like this are UB in Rust. As far as I understand, using this
>>>> READ_ONCE implementation or a relaxed atomic read would make the read
>>>> well defined. I am not aware if this is only the case if all writes to
>>>> the location from C also use atomic operations or WRITE_ONCE. @Boqun?
>>>
>>> The C side updates node.expires without WRITE_ONCE()/atomics so a
>>> Rust-side READ_ONCE() can still observe a torn value; I think that
>>> this is still a data race / UB from Rust's perspective.
>>>
>>> And since expires is 64-bit, WRITE_ONCE() on 32-bit architectures does
>>> not inherently guarantee tear-free stores either.
>>>
>>> I think that the expires() method should follow the same safety
>>> requirements as raw_forward(): it should only be considered safe when
>>> holding exclusive access to hrtimer or within the context of the timer
>>> callback. Under those conditions, it would be fine to call C's
>>> hrtimer_get_expires().
>>
>> We can make it safe, please see my comment here [1].
>>
>> Best regards,
>> Andreas Hindborg
>>
>> [1] https://lore.kernel.org/r/87v7hdh9m4.fsf@t14s.mail-host-address-is-not-set
>
> I agree. My point was that expire() can be safe only under the same
> constraints as forward()/forward_now() so the API should require
> Pin<&mut Self> and expose it on HrTimerCallbackContext.

Do you want to send a patch?


Best regards,
Andreas Hindborg

Re: [PATCH 4/5] rust: hrtimer: use READ_ONCE instead of read_volatile

[Boqun Feng]

On Wed, Jan 07, 2026 at 11:11:43AM +0100, Andreas Hindborg wrote:
> FUJITA Tomonori <fujita.tomonori@gmail.com> writes:
> 
[...]
> >>>
> >> 
> >> This is a potentially racy read. As far as I recall, we determined that
> >> using read_once is the proper way to handle the situation.
> >> 
> >> I do not think it makes a difference that the read is done by C code.
> >
> > What does "racy read" mean here?
> >
> > The C side doesn't use WRITE_ONCE() or READ_ONCE for node.expires. How
> > would using READ_ONCE() on the Rust side make a difference?
> 
> Data races like this are UB in Rust. As far as I understand, using this
> READ_ONCE implementation or a relaxed atomic read would make the read
> well defined. I am not aware if this is only the case if all writes to
> the location from C also use atomic operations or WRITE_ONCE. @Boqun?
> 

I took a look into this, the current C code is probably fine (i.e.
without READ_ONCE() or WRITE_ONCE()) because the accesses are

1) protected by timer base locking or
2) in a timer callback which provides exclusive accesses to .expires as
   well. Note that hrtimer_cancel() doesn't need to access .expires, so
   a timer callback racing with a hrtimer_cancel() is fine.

(I may miss one or two cases, but most of the cases are fine)

The problem in Rust code is that HrTimer::expires() is a pub function,
so in 2) a HrTimer::expires() can race with hrtimer_forward(), which
causes data races.

We either change hrtimer C code to support such a usage (against data
races) or change the usage of this HrTimer::expires() function. Using
READ_ONCE() here won't work. (Yes, we could say assuming all plain
writes on .expires in C are atomic as some other code does, but hrtimer
doesn't rely on this, so I don't think we should either)

Regards,
Boqun

> 
> Best regards,
> Andreas Hindborg
> 
> 

Re: [PATCH 4/5] rust: hrtimer: use READ_ONCE instead of read_volatile

[Andreas Hindborg]

Boqun Feng <boqun.feng@gmail.com> writes:

> On Wed, Jan 07, 2026 at 11:11:43AM +0100, Andreas Hindborg wrote:
>> FUJITA Tomonori <fujita.tomonori@gmail.com> writes:
>> 
> [...]
>> >>>
>> >> 
>> >> This is a potentially racy read. As far as I recall, we determined that
>> >> using read_once is the proper way to handle the situation.
>> >> 
>> >> I do not think it makes a difference that the read is done by C code.
>> >
>> > What does "racy read" mean here?
>> >
>> > The C side doesn't use WRITE_ONCE() or READ_ONCE for node.expires. How
>> > would using READ_ONCE() on the Rust side make a difference?
>> 
>> Data races like this are UB in Rust. As far as I understand, using this
>> READ_ONCE implementation or a relaxed atomic read would make the read
>> well defined. I am not aware if this is only the case if all writes to
>> the location from C also use atomic operations or WRITE_ONCE. @Boqun?
>> 
>
> I took a look into this, the current C code is probably fine (i.e.
> without READ_ONCE() or WRITE_ONCE()) because the accesses are
>
> 1) protected by timer base locking or
> 2) in a timer callback which provides exclusive accesses to .expires as
>    well. Note that hrtimer_cancel() doesn't need to access .expires, so
>    a timer callback racing with a hrtimer_cancel() is fine.
>
> (I may miss one or two cases, but most of the cases are fine)
>
> The problem in Rust code is that HrTimer::expires() is a pub function,
> so in 2) a HrTimer::expires() can race with hrtimer_forward(), which
> causes data races.
>
> We either change hrtimer C code to support such a usage (against data
> races) or change the usage of this HrTimer::expires() function. Using
> READ_ONCE() here won't work. (Yes, we could say assuming all plain
> writes on .expires in C are atomic as some other code does, but hrtimer
> doesn't rely on this, so I don't think we should either)

I don't think we should change the C code, I think the Rust API is
simply wrong. The function should have same constraints as
`forward_now`, ie. call while having exclusive access to the timer
(during setup for instance), or in callback context.

We should change it to take `self: Pin<&mut Self>` and add it on
`HrTimerCallbackContext` as well.

@Tomo, do you know of any users of this function?


Best regards,
Andreas Hindborg

Re: [PATCH 4/5] rust: hrtimer: use READ_ONCE instead of read_volatile

[Gary Guo]

On Tue, 06 Jan 2026 13:37:34 +0100
Andreas Hindborg <a.hindborg@kernel.org> wrote:

> "FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:
> >
> > Sorry, of course this should be:
> >
> > +__rust_helper ktime_t rust_helper_hrtimer_get_expires(const struct hrtimer *timer)
> > +{
> > +	return hrtimer_get_expires(timer);
> > +}
> >  
> 
> This is a potentially racy read. As far as I recall, we determined that
> using read_once is the proper way to handle the situation.
> 
> I do not think it makes a difference that the read is done by C code.

If that's the case I think the C code should be fixed by inserting the
READ_ONCE?

Best,
Gary

Re: [PATCH 4/5] rust: hrtimer: use READ_ONCE instead of read_volatile

[Alice Ryhl]

On Tue, Jan 06, 2026 at 03:23:00PM +0000, Gary Guo wrote:
> On Tue, 06 Jan 2026 13:37:34 +0100
> Andreas Hindborg <a.hindborg@kernel.org> wrote:
> 
> > "FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:
> > >
> > > Sorry, of course this should be:
> > >
> > > +__rust_helper ktime_t rust_helper_hrtimer_get_expires(const struct hrtimer *timer)
> > > +{
> > > +	return hrtimer_get_expires(timer);
> > > +}
> > >  
> > 
> > This is a potentially racy read. As far as I recall, we determined that
> > using read_once is the proper way to handle the situation.
> > 
> > I do not think it makes a difference that the read is done by C code.
> 
> If that's the case I think the C code should be fixed by inserting the
> READ_ONCE?

I maintain my position that if this is what you recommend C code does,
it's confusing to not make the same recommendation for Rust abstractions
to the same thing.

After all, nothing is stopping you from calling atomic_read() in C too.

Alice

Re: [PATCH 4/5] rust: hrtimer: use READ_ONCE instead of read_volatile

[John Hubbard]

On 1/6/26 10:43 AM, Alice Ryhl wrote:
> On Tue, Jan 06, 2026 at 03:23:00PM +0000, Gary Guo wrote:
>> On Tue, 06 Jan 2026 13:37:34 +0100
>> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>>> "FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:
>>>>
>>>> Sorry, of course this should be:
>>>>
>>>> +__rust_helper ktime_t rust_helper_hrtimer_get_expires(const struct hrtimer *timer)
>>>> +{
>>>> +	return hrtimer_get_expires(timer);
>>>> +}
>>>>  
>>>
>>> This is a potentially racy read. As far as I recall, we determined that
>>> using read_once is the proper way to handle the situation.
>>>
>>> I do not think it makes a difference that the read is done by C code.
>>
>> If that's the case I think the C code should be fixed by inserting the
>> READ_ONCE?
> 
> I maintain my position that if this is what you recommend C code does,
> it's confusing to not make the same recommendation for Rust abstractions
> to the same thing.
> 
> After all, nothing is stopping you from calling atomic_read() in C too.
> 

Hi Alice and everyone!

I'm having trouble fully understanding the latest reply, so maybe what
I'm saying is actually what you just said.

Anyway, we should use READ_ONCE in both the C and Rust code. Relying
on the compiler for that is no longer OK. We shouldn't be shy about
fixing the C side (not that I think you have been, so far!).

thanks,
-- 
John Hubbard

Re: [PATCH 4/5] rust: hrtimer: use READ_ONCE instead of read_volatile

[Boqun Feng]

On Tue, Jan 06, 2026 at 04:47:35PM -0800, John Hubbard wrote:
> On 1/6/26 10:43 AM, Alice Ryhl wrote:
> > On Tue, Jan 06, 2026 at 03:23:00PM +0000, Gary Guo wrote:
> >> On Tue, 06 Jan 2026 13:37:34 +0100
> >> Andreas Hindborg <a.hindborg@kernel.org> wrote:
> >>
> >>> "FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:
> >>>>
> >>>> Sorry, of course this should be:
> >>>>
> >>>> +__rust_helper ktime_t rust_helper_hrtimer_get_expires(const struct hrtimer *timer)
> >>>> +{
> >>>> +	return hrtimer_get_expires(timer);
> >>>> +}
> >>>>  
> >>>
> >>> This is a potentially racy read. As far as I recall, we determined that
> >>> using read_once is the proper way to handle the situation.
> >>>
> >>> I do not think it makes a difference that the read is done by C code.
> >>
> >> If that's the case I think the C code should be fixed by inserting the
> >> READ_ONCE?
> > 
> > I maintain my position that if this is what you recommend C code does,
> > it's confusing to not make the same recommendation for Rust abstractions
> > to the same thing.
> > 
> > After all, nothing is stopping you from calling atomic_read() in C too.
> > 
> 
> Hi Alice and everyone!
> 
> I'm having trouble fully understanding the latest reply, so maybe what
> I'm saying is actually what you just said.
> 
> Anyway, we should use READ_ONCE in both the C and Rust code. Relying
> on the compiler for that is no longer OK. We shouldn't be shy about
> fixing the C side (not that I think you have been, so far!).
> 

Agreed on most of it, except that we should be more explicit in Rust,
by using atomic_load[1] instead of READ_ONCE().

[1]: https://lore.kernel.org/rust-for-linux/aV0FxCRzXFrNLZik@tardis-2.local/

Regards,
Boqun

> thanks,
> -- 
> John Hubbard
> 

Re: [PATCH 4/5] rust: hrtimer: use READ_ONCE instead of read_volatile

[John Hubbard]

On 1/6/26 5:08 PM, Boqun Feng wrote:
> On Tue, Jan 06, 2026 at 04:47:35PM -0800, John Hubbard wrote:
>> On 1/6/26 10:43 AM, Alice Ryhl wrote:
>>> On Tue, Jan 06, 2026 at 03:23:00PM +0000, Gary Guo wrote:
>>>> On Tue, 06 Jan 2026 13:37:34 +0100
>>>> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>>>> "FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:
...
>>>>> This is a potentially racy read. As far as I recall, we determined that
>>>>> using read_once is the proper way to handle the situation.
>>>>>
>>>>> I do not think it makes a difference that the read is done by C code.
>>>>
>>>> If that's the case I think the C code should be fixed by inserting the
>>>> READ_ONCE?
>>>
>>> I maintain my position that if this is what you recommend C code does,
>>> it's confusing to not make the same recommendation for Rust abstractions
>>> to the same thing.
>>>
>>> After all, nothing is stopping you from calling atomic_read() in C too.
>>>
>>
>> Hi Alice and everyone!
>>
>> I'm having trouble fully understanding the latest reply, so maybe what
>> I'm saying is actually what you just said.
>>
>> Anyway, we should use READ_ONCE in both the C and Rust code. Relying
>> on the compiler for that is no longer OK. We shouldn't be shy about
>> fixing the C side (not that I think you have been, so far!).
>>
> 
> Agreed on most of it, except that we should be more explicit in Rust,
> by using atomic_load[1] instead of READ_ONCE().
> 
> [1]: https://lore.kernel.org/rust-for-linux/aV0FxCRzXFrNLZik@tardis-2.local/
> 

I see. That does put things in a much clearer state, yes.

thanks,
-- 
John Hubbard

Re: [PATCH 4/5] rust: hrtimer: use READ_ONCE instead of read_volatile

[Boqun Feng]

On Tue, Jan 06, 2026 at 06:43:17PM +0000, Alice Ryhl wrote:
> On Tue, Jan 06, 2026 at 03:23:00PM +0000, Gary Guo wrote:
> > On Tue, 06 Jan 2026 13:37:34 +0100
> > Andreas Hindborg <a.hindborg@kernel.org> wrote:
> > 
> > > "FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:
> > > >
> > > > Sorry, of course this should be:
> > > >
> > > > +__rust_helper ktime_t rust_helper_hrtimer_get_expires(const struct hrtimer *timer)
> > > > +{
> > > > +	return hrtimer_get_expires(timer);
> > > > +}
> > > >  
> > > 
> > > This is a potentially racy read. As far as I recall, we determined that
> > > using read_once is the proper way to handle the situation.
> > > 
> > > I do not think it makes a difference that the read is done by C code.
> > 
> > If that's the case I think the C code should be fixed by inserting the
> > READ_ONCE?
> 
> I maintain my position that if this is what you recommend C code does,
> it's confusing to not make the same recommendation for Rust abstractions
> to the same thing.

The problem here is that C code should use atomic operation here, and
it can be done via READ_ONCE() in C, and in Rust, it should be done by
Atomic::from_ptr().load().

The recommendation is not "using READ_ONCE()" for C, it should be "using
reads that are atomic here", and that's why introducing READ_ONCE() in
Rust is a bad idea, because what we need here is an atomic operation not
a "magical thing that C relies on so we are fine".

> 
> After all, nothing is stopping you from calling atomic_read() in C too.
> 

Actually using atomic_read() in C should also work, it'll be technically
correct as well.

Regards,
Boqun

> Alice

[PATCH 5/5] rust: fs: use READ_ONCE instead of read_volatile

[Alice Ryhl]

Using `READ_ONCE` is the correct way to read the `f_flags` field.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/fs/file.rs | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/rust/kernel/fs/file.rs b/rust/kernel/fs/file.rs
index 23ee689bd2400565223181645157d832a836589f..6b07f08e7012f512e53743266096ce0076d29e1c 100644
--- a/rust/kernel/fs/file.rs
+++ b/rust/kernel/fs/file.rs
@@ -335,12 +335,8 @@ pub fn cred(&self) -> &Credential {
     /// The flags are a combination of the constants in [`flags`].
     #[inline]
     pub fn flags(&self) -> u32 {
-        // This `read_volatile` is intended to correspond to a READ_ONCE call.
-        //
-        // SAFETY: The file is valid because the shared reference guarantees a nonzero refcount.
-        //
-        // FIXME(read_once): Replace with `read_once` when available on the Rust side.
-        unsafe { core::ptr::addr_of!((*self.as_ptr()).f_flags).read_volatile() }
+        // SAFETY: The `f_flags` field of `struct file` is readable with `READ_ONCE`.
+        unsafe { kernel::sync::READ_ONCE(&raw const (*self.as_ptr()).f_flags) }
     }
 }
 

-- 
2.52.0.351.gbe84eed79e-goog

Re: [PATCH 5/5] rust: fs: use READ_ONCE instead of read_volatile

[Boqun Feng]

On Wed, Dec 31, 2025 at 12:22:29PM +0000, Alice Ryhl wrote:
> Using `READ_ONCE` is the correct way to read the `f_flags` field.
> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/fs/file.rs | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/rust/kernel/fs/file.rs b/rust/kernel/fs/file.rs
> index 23ee689bd2400565223181645157d832a836589f..6b07f08e7012f512e53743266096ce0076d29e1c 100644
> --- a/rust/kernel/fs/file.rs
> +++ b/rust/kernel/fs/file.rs
> @@ -335,12 +335,8 @@ pub fn cred(&self) -> &Credential {
>      /// The flags are a combination of the constants in [`flags`].
>      #[inline]
>      pub fn flags(&self) -> u32 {
> -        // This `read_volatile` is intended to correspond to a READ_ONCE call.
> -        //
> -        // SAFETY: The file is valid because the shared reference guarantees a nonzero refcount.
> -        //
> -        // FIXME(read_once): Replace with `read_once` when available on the Rust side.
> -        unsafe { core::ptr::addr_of!((*self.as_ptr()).f_flags).read_volatile() }
> +        // SAFETY: The `f_flags` field of `struct file` is readable with `READ_ONCE`.
> +        unsafe { kernel::sync::READ_ONCE(&raw const (*self.as_ptr()).f_flags) }

Not a question directly to this patch, but for FS folks: I see we read
and write `f_flags` normally (i.e. without *_ONCE() or any atomic), and
I don't see any synchronization between these read and write (maybe I'm
missing something?), if read and write can happen at the same time, it's
data race. So I assume we must have some assumption on the atomicity of
these accesses to `f_flags`, could you may share or confirm this? Thanks

Regards,
Boqun

>      }
>  }
>  
> 
> -- 
> 2.52.0.351.gbe84eed79e-goog
> 

Re: [PATCH 0/5] Add READ_ONCE and WRITE_ONCE to Rust

[Gary Guo]

On Wed, 31 Dec 2025 12:22:24 +0000
Alice Ryhl <aliceryhl@google.com> wrote:

> There are currently a few places in the kernel where we use volatile
> reads when we really should be using `READ_ONCE`. To make it possible to
> replace these with proper `READ_ONCE` calls, introduce a Rust version of
> `READ_ONCE`.
> 
> A new config option CONFIG_ARCH_USE_CUSTOM_READ_ONCE is introduced so
> that Rust is able to use conditional compilation to implement READ_ONCE
> in terms of either a volatile read, or by calling into a C helper
> function, depending on the architecture.
> 
> This series is intended to be merged through ATOMIC INFRASTRUCTURE.

Hi Alice,

I would prefer not to expose the READ_ONCE/WRITE_ONCE functions, at
least not with their atomic semantics.

Both callsites that you have converted should be using

	Atomic::from_ptr().load(Relaxed)

Please refer to the documentation of `Atomic` about this. Fujita has a
series that expand the type to u8/u16 if you need narrower accesses.

Best,
Gary


> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> Alice Ryhl (5):
>       arch: add CONFIG_ARCH_USE_CUSTOM_READ_ONCE for arm64/alpha
>       rust: sync: add READ_ONCE and WRITE_ONCE
>       rust: sync: support using bool with READ_ONCE
>       rust: hrtimer: use READ_ONCE instead of read_volatile
>       rust: fs: use READ_ONCE instead of read_volatile
> 
>  MAINTAINERS                     |   2 +
>  arch/Kconfig                    |  11 +++
>  arch/alpha/Kconfig              |   1 +
>  arch/alpha/include/asm/rwonce.h |   4 +-
>  arch/arm64/Kconfig              |   1 +
>  arch/arm64/include/asm/rwonce.h |   4 +-
>  rust/helpers/helpers.c          |   1 +
>  rust/helpers/rwonce.c           |  34 +++++++
>  rust/kernel/fs/file.rs          |   8 +-
>  rust/kernel/sync.rs             |   2 +
>  rust/kernel/sync/rwonce.rs      | 207 ++++++++++++++++++++++++++++++++++++++++
>  rust/kernel/time/hrtimer.rs     |   8 +-
>  12 files changed, 268 insertions(+), 15 deletions(-)
> ---
> base-commit: f8f9c1f4d0c7a64600e2ca312dec824a0bc2f1da
> change-id: 20251230-rwonce-1e8d2ee0bcf9
> 
> Best regards,

Re: [PATCH 0/5] Add READ_ONCE and WRITE_ONCE to Rust

[Alice Ryhl]

On Wed, Dec 31, 2025 at 03:12:16PM +0000, Gary Guo wrote:
> On Wed, 31 Dec 2025 12:22:24 +0000
> Alice Ryhl <aliceryhl@google.com> wrote:
> 
> > There are currently a few places in the kernel where we use volatile
> > reads when we really should be using `READ_ONCE`. To make it possible to
> > replace these with proper `READ_ONCE` calls, introduce a Rust version of
> > `READ_ONCE`.
> > 
> > A new config option CONFIG_ARCH_USE_CUSTOM_READ_ONCE is introduced so
> > that Rust is able to use conditional compilation to implement READ_ONCE
> > in terms of either a volatile read, or by calling into a C helper
> > function, depending on the architecture.
> > 
> > This series is intended to be merged through ATOMIC INFRASTRUCTURE.
> 
> Hi Alice,
> 
> I would prefer not to expose the READ_ONCE/WRITE_ONCE functions, at
> least not with their atomic semantics.
> 
> Both callsites that you have converted should be using
> 
> 	Atomic::from_ptr().load(Relaxed)
> 
> Please refer to the documentation of `Atomic` about this. Fujita has a
> series that expand the type to u8/u16 if you need narrower accesses.

Why? If we say that we're using the LKMM, then it seems confusing to not
have a READ_ONCE() for cases where we interact with C code, and that C
code documents that READ_ONCE() should be used.

Alice

Re: [PATCH 0/5] Add READ_ONCE and WRITE_ONCE to Rust

[Boqun Feng]

On Thu, Jan 01, 2026 at 12:53:39AM +0000, Alice Ryhl wrote:
> On Wed, Dec 31, 2025 at 03:12:16PM +0000, Gary Guo wrote:
> > On Wed, 31 Dec 2025 12:22:24 +0000
> > Alice Ryhl <aliceryhl@google.com> wrote:
> > 
> > > There are currently a few places in the kernel where we use volatile
> > > reads when we really should be using `READ_ONCE`. To make it possible to
> > > replace these with proper `READ_ONCE` calls, introduce a Rust version of
> > > `READ_ONCE`.
> > > 
> > > A new config option CONFIG_ARCH_USE_CUSTOM_READ_ONCE is introduced so
> > > that Rust is able to use conditional compilation to implement READ_ONCE
> > > in terms of either a volatile read, or by calling into a C helper
> > > function, depending on the architecture.
> > > 
> > > This series is intended to be merged through ATOMIC INFRASTRUCTURE.
> > 
> > Hi Alice,
> > 
> > I would prefer not to expose the READ_ONCE/WRITE_ONCE functions, at
> > least not with their atomic semantics.
> > 
> > Both callsites that you have converted should be using
> > 
> > 	Atomic::from_ptr().load(Relaxed)
> > 
> > Please refer to the documentation of `Atomic` about this. Fujita has a
> > series that expand the type to u8/u16 if you need narrower accesses.
> 
> Why? If we say that we're using the LKMM, then it seems confusing to not
> have a READ_ONCE() for cases where we interact with C code, and that C
> code documents that READ_ONCE() should be used.
> 

The problem of READ_ONCE() and WRITE_ONCE() is that the semantics is
complicated. Sometimes they are used for atomicity, sometimes they are
used for preventing data race. So yes, we are using LKMM in Rust as
well, but whenever possible, we need to clarify the intentation of the
API, using Atomic::from_ptr().load(Relaxed) helps on that front.

IMO, READ_ONCE()/WRITE_ONCE() is like a "band aid" solution to a few
problems, having it would prevent us from developing a more clear view
for concurrent programming.

Regards,
Boqun

> Alice

Re: [PATCH 0/5] Add READ_ONCE and WRITE_ONCE to Rust

[Andreas Hindborg]

"Boqun Feng" <boqun.feng@gmail.com> writes:

> On Thu, Jan 01, 2026 at 12:53:39AM +0000, Alice Ryhl wrote:
>> On Wed, Dec 31, 2025 at 03:12:16PM +0000, Gary Guo wrote:
>> > On Wed, 31 Dec 2025 12:22:24 +0000
>> > Alice Ryhl <aliceryhl@google.com> wrote:
>> >
>> > > There are currently a few places in the kernel where we use volatile
>> > > reads when we really should be using `READ_ONCE`. To make it possible to
>> > > replace these with proper `READ_ONCE` calls, introduce a Rust version of
>> > > `READ_ONCE`.
>> > >
>> > > A new config option CONFIG_ARCH_USE_CUSTOM_READ_ONCE is introduced so
>> > > that Rust is able to use conditional compilation to implement READ_ONCE
>> > > in terms of either a volatile read, or by calling into a C helper
>> > > function, depending on the architecture.
>> > >
>> > > This series is intended to be merged through ATOMIC INFRASTRUCTURE.
>> >
>> > Hi Alice,
>> >
>> > I would prefer not to expose the READ_ONCE/WRITE_ONCE functions, at
>> > least not with their atomic semantics.
>> >
>> > Both callsites that you have converted should be using
>> >
>> > 	Atomic::from_ptr().load(Relaxed)
>> >
>> > Please refer to the documentation of `Atomic` about this. Fujita has a
>> > series that expand the type to u8/u16 if you need narrower accesses.
>>
>> Why? If we say that we're using the LKMM, then it seems confusing to not
>> have a READ_ONCE() for cases where we interact with C code, and that C
>> code documents that READ_ONCE() should be used.
>>
>
> The problem of READ_ONCE() and WRITE_ONCE() is that the semantics is
> complicated. Sometimes they are used for atomicity, sometimes they are
> used for preventing data race. So yes, we are using LKMM in Rust as
> well, but whenever possible, we need to clarify the intentation of the
> API, using Atomic::from_ptr().load(Relaxed) helps on that front.
>
> IMO, READ_ONCE()/WRITE_ONCE() is like a "band aid" solution to a few
> problems, having it would prevent us from developing a more clear view
> for concurrent programming.

What is the semantics of a non-atomic write in C code under lock racing
with a READ_ONCE/atomic relaxed read in Rust? That is the hrtimer case.


Best regards,
Andreas Hindborg

Re: [PATCH 0/5] Add READ_ONCE and WRITE_ONCE to Rust

[Boqun Feng]

On Tue, Jan 06, 2026 at 01:41:33PM +0100, Andreas Hindborg wrote:
> "Boqun Feng" <boqun.feng@gmail.com> writes:
> 
[...]
> >> > I would prefer not to expose the READ_ONCE/WRITE_ONCE functions, at
> >> > least not with their atomic semantics.
> >> >
> >> > Both callsites that you have converted should be using
> >> >
> >> > 	Atomic::from_ptr().load(Relaxed)
> >> >
> >> > Please refer to the documentation of `Atomic` about this. Fujita has a
> >> > series that expand the type to u8/u16 if you need narrower accesses.
> >>
> >> Why? If we say that we're using the LKMM, then it seems confusing to not
> >> have a READ_ONCE() for cases where we interact with C code, and that C
> >> code documents that READ_ONCE() should be used.
> >>
> >
> > The problem of READ_ONCE() and WRITE_ONCE() is that the semantics is
> > complicated. Sometimes they are used for atomicity, sometimes they are
> > used for preventing data race. So yes, we are using LKMM in Rust as
> > well, but whenever possible, we need to clarify the intentation of the
> > API, using Atomic::from_ptr().load(Relaxed) helps on that front.
> >
> > IMO, READ_ONCE()/WRITE_ONCE() is like a "band aid" solution to a few
> > problems, having it would prevent us from developing a more clear view
> > for concurrent programming.
> 
> What is the semantics of a non-atomic write in C code under lock racing
> with a READ_ONCE/atomic relaxed read in Rust? That is the hrtimer case.
> 

Some C code believes a plain write to a properly aligned location is
atomic (see KCSAN_ASSUME_PLAIN_WRITES_ATOMIC, and no, this doesn't mean
it's recommended to assume such), and I guess that's the case for
hrtimer, if it's not much a trouble you can replace the plain write with
WRITE_ONCE() on C side ;-)

Regards,
Boqun

> 
> Best regards,
> Andreas Hindborg
> 
> 
> 

Re: [PATCH 0/5] Add READ_ONCE and WRITE_ONCE to Rust

[Peter Zijlstra]

On Tue, Jan 06, 2026 at 09:09:37PM +0800, Boqun Feng wrote:

> Some C code believes a plain write to a properly aligned location is
> atomic (see KCSAN_ASSUME_PLAIN_WRITES_ATOMIC, and no, this doesn't mean
> it's recommended to assume such), and I guess that's the case for
> hrtimer, if it's not much a trouble you can replace the plain write with
> WRITE_ONCE() on C side ;-)

GCC used to provide this guarantee, some of the older code was written
on that. GCC no longer provides that guarantee (there are known cases
where it breaks and all that) and newer code should not rely on this.

All such places *SHOULD* be updated to use READ_ONCE/WRITE_ONCE.

Re: [PATCH 0/5] Add READ_ONCE and WRITE_ONCE to Rust

[Paul E. McKenney]

On Tue, Jan 06, 2026 at 03:56:22PM +0100, Peter Zijlstra wrote:
> On Tue, Jan 06, 2026 at 09:09:37PM +0800, Boqun Feng wrote:
> 
> > Some C code believes a plain write to a properly aligned location is
> > atomic (see KCSAN_ASSUME_PLAIN_WRITES_ATOMIC, and no, this doesn't mean
> > it's recommended to assume such), and I guess that's the case for
> > hrtimer, if it's not much a trouble you can replace the plain write with
> > WRITE_ONCE() on C side ;-)
> 
> GCC used to provide this guarantee, some of the older code was written
> on that. GCC no longer provides that guarantee (there are known cases
> where it breaks and all that) and newer code should not rely on this.
> 
> All such places *SHOULD* be updated to use READ_ONCE/WRITE_ONCE.

Agreed!

In that vein, any objections to the patch shown below?

							Thanx, Paul

------------------------------------------------------------------------

diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
index 4ce4b0c0109cb..e827e24ab5d42 100644
--- a/lib/Kconfig.kcsan
+++ b/lib/Kconfig.kcsan
@@ -199,7 +199,7 @@ config KCSAN_WEAK_MEMORY
 
 config KCSAN_REPORT_VALUE_CHANGE_ONLY
 	bool "Only report races where watcher observed a data value change"
-	default y
+	default n
 	depends on !KCSAN_STRICT
 	help
 	  If enabled and a conflicting write is observed via a watchpoint, but
@@ -208,7 +208,7 @@ config KCSAN_REPORT_VALUE_CHANGE_ONLY
 
 config KCSAN_ASSUME_PLAIN_WRITES_ATOMIC
 	bool "Assume that plain aligned writes up to word size are atomic"
-	default y
+	default n
 	depends on !KCSAN_STRICT
 	help
 	  Assume that plain aligned writes up to word size are atomic by

Re: [PATCH 0/5] Add READ_ONCE and WRITE_ONCE to Rust

[Marco Elver]

On Tue, 6 Jan 2026 at 19:18, 'Paul E. McKenney' via kasan-dev
<kasan-dev@googlegroups.com> wrote:
> On Tue, Jan 06, 2026 at 03:56:22PM +0100, Peter Zijlstra wrote:
> > On Tue, Jan 06, 2026 at 09:09:37PM +0800, Boqun Feng wrote:
> >
> > > Some C code believes a plain write to a properly aligned location is
> > > atomic (see KCSAN_ASSUME_PLAIN_WRITES_ATOMIC, and no, this doesn't mean
> > > it's recommended to assume such), and I guess that's the case for
> > > hrtimer, if it's not much a trouble you can replace the plain write with
> > > WRITE_ONCE() on C side ;-)
> >
> > GCC used to provide this guarantee, some of the older code was written
> > on that. GCC no longer provides that guarantee (there are known cases
> > where it breaks and all that) and newer code should not rely on this.
> >
> > All such places *SHOULD* be updated to use READ_ONCE/WRITE_ONCE.
>
> Agreed!
>
> In that vein, any objections to the patch shown below?

I'd be in favor, as that's what we did in the very initial version of
KCSAN (we started strict and then loosened things up).

However, the fallout will be even more perceived "noise", despite
being legitimate data races. These config knobs were added after much
discussion in 2019/2020, somewhere around this discussion (I think
that's the one that spawned KCSAN_REPORT_VALUE_CHANGE_ONLY, can't find
the source for KCSAN_ASSUME_PLAIN_WRITES_ATOMIC):
https://lore.kernel.org/all/CAHk-=wgu-QXU83ai4XBnh7JJUo2NBW41XhLWf=7wrydR4=ZP0g@mail.gmail.com/

While the situation has gotten better since 2020, we still have latent
data races that need some thought (given papering over things blindly
with *ONCE is not right either). My recommendation these days is to
just set CONFIG_KCSAN_STRICT=y for those who care (although I'd wish
everyone cared the same amount :-)).

Should you feel the below change is appropriate for 2026, feel free to
carry it (consider this my Ack).

However, I wasn't thinking of tightening the screws until the current
set of known data races has gotten to a manageable amount (say below
50)
https://syzkaller.appspot.com/upstream?manager=ci2-upstream-kcsan-gce
Then again, on syzbot the config can remain unchanged.

Thanks,
-- Marco

>                                                         Thanx, Paul
>
> ------------------------------------------------------------------------
>
> diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
> index 4ce4b0c0109cb..e827e24ab5d42 100644
> --- a/lib/Kconfig.kcsan
> +++ b/lib/Kconfig.kcsan
> @@ -199,7 +199,7 @@ config KCSAN_WEAK_MEMORY
>
>  config KCSAN_REPORT_VALUE_CHANGE_ONLY
>         bool "Only report races where watcher observed a data value change"
> -       default y
> +       default n
>         depends on !KCSAN_STRICT
>         help
>           If enabled and a conflicting write is observed via a watchpoint, but
> @@ -208,7 +208,7 @@ config KCSAN_REPORT_VALUE_CHANGE_ONLY
>
>  config KCSAN_ASSUME_PLAIN_WRITES_ATOMIC
>         bool "Assume that plain aligned writes up to word size are atomic"
> -       default y
> +       default n
>         depends on !KCSAN_STRICT
>         help
>           Assume that plain aligned writes up to word size are atomic by
>

Re: [PATCH 0/5] Add READ_ONCE and WRITE_ONCE to Rust

[Paul E. McKenney]

On Tue, Jan 06, 2026 at 08:28:41PM +0100, Marco Elver wrote:
> On Tue, 6 Jan 2026 at 19:18, 'Paul E. McKenney' via kasan-dev
> <kasan-dev@googlegroups.com> wrote:
> > On Tue, Jan 06, 2026 at 03:56:22PM +0100, Peter Zijlstra wrote:
> > > On Tue, Jan 06, 2026 at 09:09:37PM +0800, Boqun Feng wrote:
> > >
> > > > Some C code believes a plain write to a properly aligned location is
> > > > atomic (see KCSAN_ASSUME_PLAIN_WRITES_ATOMIC, and no, this doesn't mean
> > > > it's recommended to assume such), and I guess that's the case for
> > > > hrtimer, if it's not much a trouble you can replace the plain write with
> > > > WRITE_ONCE() on C side ;-)
> > >
> > > GCC used to provide this guarantee, some of the older code was written
> > > on that. GCC no longer provides that guarantee (there are known cases
> > > where it breaks and all that) and newer code should not rely on this.
> > >
> > > All such places *SHOULD* be updated to use READ_ONCE/WRITE_ONCE.
> >
> > Agreed!
> >
> > In that vein, any objections to the patch shown below?
> 
> I'd be in favor, as that's what we did in the very initial version of
> KCSAN (we started strict and then loosened things up).
> 
> However, the fallout will be even more perceived "noise", despite
> being legitimate data races. These config knobs were added after much
> discussion in 2019/2020, somewhere around this discussion (I think
> that's the one that spawned KCSAN_REPORT_VALUE_CHANGE_ONLY, can't find
> the source for KCSAN_ASSUME_PLAIN_WRITES_ATOMIC):
> https://lore.kernel.org/all/CAHk-=wgu-QXU83ai4XBnh7JJUo2NBW41XhLWf=7wrydR4=ZP0g@mail.gmail.com/

Fair point!

> While the situation has gotten better since 2020, we still have latent
> data races that need some thought (given papering over things blindly
> with *ONCE is not right either). My recommendation these days is to
> just set CONFIG_KCSAN_STRICT=y for those who care (although I'd wish
> everyone cared the same amount :-)).
> 
> Should you feel the below change is appropriate for 2026, feel free to
> carry it (consider this my Ack).
> 
> However, I wasn't thinking of tightening the screws until the current
> set of known data races has gotten to a manageable amount (say below
> 50)
> https://syzkaller.appspot.com/upstream?manager=ci2-upstream-kcsan-gce
> Then again, on syzbot the config can remain unchanged.

Is there an easy way to map from a report to the SHA-1 that the
corresponding test ran against?  Probably me being blind, but I am not
seeing it.  Though I do very much like the symbolic names in those
stack traces!

							Thanx, Paul

> Thanks,
> -- Marco
> 
> >                                                         Thanx, Paul
> >
> > ------------------------------------------------------------------------
> >
> > diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
> > index 4ce4b0c0109cb..e827e24ab5d42 100644
> > --- a/lib/Kconfig.kcsan
> > +++ b/lib/Kconfig.kcsan
> > @@ -199,7 +199,7 @@ config KCSAN_WEAK_MEMORY
> >
> >  config KCSAN_REPORT_VALUE_CHANGE_ONLY
> >         bool "Only report races where watcher observed a data value change"
> > -       default y
> > +       default n
> >         depends on !KCSAN_STRICT
> >         help
> >           If enabled and a conflicting write is observed via a watchpoint, but
> > @@ -208,7 +208,7 @@ config KCSAN_REPORT_VALUE_CHANGE_ONLY
> >
> >  config KCSAN_ASSUME_PLAIN_WRITES_ATOMIC
> >         bool "Assume that plain aligned writes up to word size are atomic"
> > -       default y
> > +       default n
> >         depends on !KCSAN_STRICT
> >         help
> >           Assume that plain aligned writes up to word size are atomic by
> >

Re: [PATCH 0/5] Add READ_ONCE and WRITE_ONCE to Rust

[Marco Elver]

On Fri, 9 Jan 2026 at 03:09, Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Tue, Jan 06, 2026 at 08:28:41PM +0100, Marco Elver wrote:
> > On Tue, 6 Jan 2026 at 19:18, 'Paul E. McKenney' via kasan-dev
> > <kasan-dev@googlegroups.com> wrote:
> > > On Tue, Jan 06, 2026 at 03:56:22PM +0100, Peter Zijlstra wrote:
> > > > On Tue, Jan 06, 2026 at 09:09:37PM +0800, Boqun Feng wrote:
> > > >
> > > > > Some C code believes a plain write to a properly aligned location is
> > > > > atomic (see KCSAN_ASSUME_PLAIN_WRITES_ATOMIC, and no, this doesn't mean
> > > > > it's recommended to assume such), and I guess that's the case for
> > > > > hrtimer, if it's not much a trouble you can replace the plain write with
> > > > > WRITE_ONCE() on C side ;-)
> > > >
> > > > GCC used to provide this guarantee, some of the older code was written
> > > > on that. GCC no longer provides that guarantee (there are known cases
> > > > where it breaks and all that) and newer code should not rely on this.
> > > >
> > > > All such places *SHOULD* be updated to use READ_ONCE/WRITE_ONCE.
> > >
> > > Agreed!
> > >
> > > In that vein, any objections to the patch shown below?
> >
> > I'd be in favor, as that's what we did in the very initial version of
> > KCSAN (we started strict and then loosened things up).
> >
> > However, the fallout will be even more perceived "noise", despite
> > being legitimate data races. These config knobs were added after much
> > discussion in 2019/2020, somewhere around this discussion (I think
> > that's the one that spawned KCSAN_REPORT_VALUE_CHANGE_ONLY, can't find
> > the source for KCSAN_ASSUME_PLAIN_WRITES_ATOMIC):
> > https://lore.kernel.org/all/CAHk-=wgu-QXU83ai4XBnh7JJUo2NBW41XhLWf=7wrydR4=ZP0g@mail.gmail.com/
>
> Fair point!
>
> > While the situation has gotten better since 2020, we still have latent
> > data races that need some thought (given papering over things blindly
> > with *ONCE is not right either). My recommendation these days is to
> > just set CONFIG_KCSAN_STRICT=y for those who care (although I'd wish
> > everyone cared the same amount :-)).
> >
> > Should you feel the below change is appropriate for 2026, feel free to
> > carry it (consider this my Ack).
> >
> > However, I wasn't thinking of tightening the screws until the current
> > set of known data races has gotten to a manageable amount (say below
> > 50)
> > https://syzkaller.appspot.com/upstream?manager=ci2-upstream-kcsan-gce
> > Then again, on syzbot the config can remain unchanged.
>
> Is there an easy way to map from a report to the SHA-1 that the
> corresponding test ran against?  Probably me being blind, but I am not
> seeing it.  Though I do very much like the symbolic names in those
> stack traces!

When viewing a report page, at the bottom in the "Crashes" table it's
in the "Commit" column.

Re: [PATCH 0/5] Add READ_ONCE and WRITE_ONCE to Rust

[Peter Zijlstra]

On Tue, Jan 06, 2026 at 10:18:35AM -0800, Paul E. McKenney wrote:
> On Tue, Jan 06, 2026 at 03:56:22PM +0100, Peter Zijlstra wrote:
> > On Tue, Jan 06, 2026 at 09:09:37PM +0800, Boqun Feng wrote:
> > 
> > > Some C code believes a plain write to a properly aligned location is
> > > atomic (see KCSAN_ASSUME_PLAIN_WRITES_ATOMIC, and no, this doesn't mean
> > > it's recommended to assume such), and I guess that's the case for
> > > hrtimer, if it's not much a trouble you can replace the plain write with
> > > WRITE_ONCE() on C side ;-)
> > 
> > GCC used to provide this guarantee, some of the older code was written
> > on that. GCC no longer provides that guarantee (there are known cases
> > where it breaks and all that) and newer code should not rely on this.
> > 
> > All such places *SHOULD* be updated to use READ_ONCE/WRITE_ONCE.
> 
> Agreed!
> 
> In that vein, any objections to the patch shown below?

Not really; although it would of course be nice if that were accompanied
with a pile of cleanup patches taking out the worst offenders or
somesuch ;-)

> ------------------------------------------------------------------------
> 
> diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
> index 4ce4b0c0109cb..e827e24ab5d42 100644
> --- a/lib/Kconfig.kcsan
> +++ b/lib/Kconfig.kcsan
> @@ -199,7 +199,7 @@ config KCSAN_WEAK_MEMORY
>  
>  config KCSAN_REPORT_VALUE_CHANGE_ONLY
>  	bool "Only report races where watcher observed a data value change"
> -	default y
> +	default n
>  	depends on !KCSAN_STRICT
>  	help
>  	  If enabled and a conflicting write is observed via a watchpoint, but
> @@ -208,7 +208,7 @@ config KCSAN_REPORT_VALUE_CHANGE_ONLY
>  
>  config KCSAN_ASSUME_PLAIN_WRITES_ATOMIC
>  	bool "Assume that plain aligned writes up to word size are atomic"
> -	default y
> +	default n
>  	depends on !KCSAN_STRICT
>  	help
>  	  Assume that plain aligned writes up to word size are atomic by

Re: [PATCH 0/5] Add READ_ONCE and WRITE_ONCE to Rust

[Paul E. McKenney]

On Wed, Jan 07, 2026 at 09:43:22AM +0100, Peter Zijlstra wrote:
> On Tue, Jan 06, 2026 at 10:18:35AM -0800, Paul E. McKenney wrote:
> > On Tue, Jan 06, 2026 at 03:56:22PM +0100, Peter Zijlstra wrote:
> > > On Tue, Jan 06, 2026 at 09:09:37PM +0800, Boqun Feng wrote:
> > > 
> > > > Some C code believes a plain write to a properly aligned location is
> > > > atomic (see KCSAN_ASSUME_PLAIN_WRITES_ATOMIC, and no, this doesn't mean
> > > > it's recommended to assume such), and I guess that's the case for
> > > > hrtimer, if it's not much a trouble you can replace the plain write with
> > > > WRITE_ONCE() on C side ;-)
> > > 
> > > GCC used to provide this guarantee, some of the older code was written
> > > on that. GCC no longer provides that guarantee (there are known cases
> > > where it breaks and all that) and newer code should not rely on this.
> > > 
> > > All such places *SHOULD* be updated to use READ_ONCE/WRITE_ONCE.
> > 
> > Agreed!
> > 
> > In that vein, any objections to the patch shown below?
> 
> Not really; although it would of course be nice if that were accompanied
> with a pile of cleanup patches taking out the worst offenders or
> somesuch ;-)

Careful what you ask for.  You might get it...  ;-)

							Thanx, Paul

> > ------------------------------------------------------------------------
> > 
> > diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
> > index 4ce4b0c0109cb..e827e24ab5d42 100644
> > --- a/lib/Kconfig.kcsan
> > +++ b/lib/Kconfig.kcsan
> > @@ -199,7 +199,7 @@ config KCSAN_WEAK_MEMORY
> >  
> >  config KCSAN_REPORT_VALUE_CHANGE_ONLY
> >  	bool "Only report races where watcher observed a data value change"
> > -	default y
> > +	default n
> >  	depends on !KCSAN_STRICT
> >  	help
> >  	  If enabled and a conflicting write is observed via a watchpoint, but
> > @@ -208,7 +208,7 @@ config KCSAN_REPORT_VALUE_CHANGE_ONLY
> >  
> >  config KCSAN_ASSUME_PLAIN_WRITES_ATOMIC
> >  	bool "Assume that plain aligned writes up to word size are atomic"
> > -	default y
> > +	default n
> >  	depends on !KCSAN_STRICT
> >  	help
> >  	  Assume that plain aligned writes up to word size are atomic by