[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87ldiaj560.fsf@t14s.mail-host-address-is-not-set>
Date: Tue, 06 Jan 2026 13:29:11 +0100
From: Andreas Hindborg <a.hindborg@...nel.org>
To: Alice Ryhl <aliceryhl@...gle.com>, Boqun Feng <boqun.feng@...il.com>,
Will Deacon <will@...nel.org>, Peter Zijlstra <peterz@...radead.org>
Cc: Richard Henderson <richard.henderson@...aro.org>, Matt Turner
<mattst88@...il.com>, Magnus Lindholm <linmag7@...il.com>, Catalin
Marinas <catalin.marinas@....com>, Miguel Ojeda <ojeda@...nel.org>, Gary
Guo <gary@...yguo.net>, Björn Roy Baron
<bjorn3_gh@...tonmail.com>, Benno
Lossin <lossin@...nel.org>, Trevor Gross <tmgross@...ch.edu>, Danilo
Krummrich <dakr@...nel.org>, Mark Rutland <mark.rutland@....com>, FUJITA
Tomonori <fujita.tomonori@...il.com>, Frederic Weisbecker
<frederic@...nel.org>, Lyude Paul <lyude@...hat.com>, Thomas Gleixner
<tglx@...utronix.de>, Anna-Maria
Behnsen <anna-maria@...utronix.de>, John Stultz <jstultz@...gle.com>,
Stephen Boyd <sboyd@...nel.org>, Alexander Viro <viro@...iv.linux.org.uk>,
Christian Brauner <brauner@...nel.org>, Jan
Kara <jack@...e.cz>, linux-kernel@...r.kernel.org,
linux-alpha@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
rust-for-linux@...r.kernel.org, linux-fsdevel@...r.kernel.org, Alice
Ryhl <aliceryhl@...gle.com>
Subject: Re: [PATCH 2/5] rust: sync: add READ_ONCE and WRITE_ONCE
"Alice Ryhl" <aliceryhl@...gle.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@...gle.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@...otech.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
Powered by blists - more mailing lists