| Message ID | 20240322233838.868874-2-boqun.feng@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Memory model and atomic API in Rust | expand |
> +//! These primitives should have the same semantics as their C counterparts, for precise definitions > +//! of the semantics, please refer to tools/memory-model. Note that Linux Kernel Memory > +//! (Consistency) Model is the only model for Rust development in kernel right now, please avoid to > +//! use Rust's own atomics. Is it possible to somehow poison rusts own atomics? I would not be too surprised if somebody with good Rust knowledge but new to the kernel tries using Rusts atomics. Either getting the compiler to fail the build, or it throws an Opps on first invocation would be good. Andrew
On Sat, Mar 23, 2024 at 12:52:08AM +0100, Andrew Lunn wrote: > > +//! These primitives should have the same semantics as their C counterparts, for precise definitions > > +//! of the semantics, please refer to tools/memory-model. Note that Linux Kernel Memory > > +//! (Consistency) Model is the only model for Rust development in kernel right now, please avoid to > > +//! use Rust's own atomics. > > Is it possible to somehow poison rusts own atomics? I would not be I can continue to look an elegant way, now since we compile our own `core` crate (where Rust atomic library locates), we can certain do a sed trick to exclude the atomic code from Rust. It's pretty hacky, but maybe others know how to teach linter to help. Regards, Boqun > too surprised if somebody with good Rust knowledge but new to the > kernel tries using Rusts atomics. Either getting the compiler to fail > the build, or it throws an Opps on first invocation would be good. > > Andrew
On Sat, Mar 23, 2024 at 12:52 AM Andrew Lunn <andrew@lunn.ch> wrote: > > > +//! These primitives should have the same semantics as their C counterparts, for precise definitions > > +//! of the semantics, please refer to tools/memory-model. Note that Linux Kernel Memory > > +//! (Consistency) Model is the only model for Rust development in kernel right now, please avoid to > > +//! use Rust's own atomics. > > Is it possible to somehow poison rusts own atomics? I would not be > too surprised if somebody with good Rust knowledge but new to the > kernel tries using Rusts atomics. Either getting the compiler to fail > the build, or it throws an Opps on first invocation would be good. We could try to get a flag added to the Rust standard library that removes the core::sync::atomic module entirely, then pass that flag. Alice
On Sat, Mar 23, 2024 at 10:58:16AM +0100, Alice Ryhl wrote: > On Sat, Mar 23, 2024 at 12:52 AM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > +//! These primitives should have the same semantics as their C counterparts, for precise definitions > > > +//! of the semantics, please refer to tools/memory-model. Note that Linux Kernel Memory > > > +//! (Consistency) Model is the only model for Rust development in kernel right now, please avoid to > > > +//! use Rust's own atomics. > > > > Is it possible to somehow poison rusts own atomics? I would not be > > too surprised if somebody with good Rust knowledge but new to the > > kernel tries using Rusts atomics. Either getting the compiler to fail > > the build, or it throws an Opps on first invocation would be good. > > We could try to get a flag added to the Rust standard library that > removes the core::sync::atomic module entirely, then pass that flag. Just looking down the road a bit, are there other features in the standard library which are not applicable to Linux kernel space? Ideally we want a solution not just for atomics but a generic solution which can disable a collection of features? Maybe one by one? And i assume somebody will try to use Rust in uboot/barebox. It probably has similar requirements to the Linux kernel? But what about Zephyr? Or VxWorks? Darwin? Andrew
On Sat, Mar 23, 2024 at 3:10 PM Andrew Lunn <andrew@lunn.ch> wrote: > > Just looking down the road a bit, are there other features in the > standard library which are not applicable to Linux kernel space? > Ideally we want a solution not just for atomics but a generic solution > which can disable a collection of features? Maybe one by one? We requested a few of these in the past for both `core` [1] and `alloc` [2], and we got some which we use (see the `cfg(no_*)`s). It is what we called "modularization of `core`" too. [1] https://github.com/Rust-for-Linux/linux/issues/514 [2] https://github.com/Rust-for-Linux/linux/issues/408 Cheers, Miguel
On Sat, Mar 23, 2024 at 1:03 AM Boqun Feng <boqun.feng@gmail.com> wrote: > > I can continue to look an elegant way, now since we compile our own > `core` crate (where Rust atomic library locates), we can certain do a > sed trick to exclude the atomic code from Rust. It's pretty hacky, but > maybe others know how to teach linter to help. Yeah, but it requires copying the source and so on, like we did for `rusttest`. I would prefer to avoid another hack like that though (and the plan is to get rid of the existing hack anyway). Cheers, Miguel
On Sat, Mar 23, 2024 at 08:13:56PM +0100, Miguel Ojeda wrote: > On Sat, Mar 23, 2024 at 1:03 AM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > I can continue to look an elegant way, now since we compile our own > > `core` crate (where Rust atomic library locates), we can certain do a > > sed trick to exclude the atomic code from Rust. It's pretty hacky, but > > maybe others know how to teach linter to help. > > Yeah, but it requires copying the source and so on, like we did for > `rusttest`. I would prefer to avoid another hack like that though (and > the plan is to get rid of the existing hack anyway). Agreed! The problem is better resolved via modularization of `core`. Regards, Boqun > > Cheers, > Miguel
On Sat, Mar 23, 2024 at 10:10 AM Andrew Lunn <andrew@lunn.ch> wrote: > > > Is it possible to somehow poison rusts own atomics? I would not be > > > too surprised if somebody with good Rust knowledge but new to the > > > kernel tries using Rusts atomics. Either getting the compiler to fail > > > the build, or it throws an Opps on first invocation would be good. > > > > We could try to get a flag added to the Rust standard library that > > removes the core::sync::atomic module entirely, then pass that flag. > > Just looking down the road a bit, are there other features in the > standard library which are not applicable to Linux kernel space? > Ideally we want a solution not just for atomics but a generic solution > which can disable a collection of features? Maybe one by one? Clippy is an easy way to do this via the disallowed_* lints. disallowed_types [1] would be applicable here to forbid `core::atomic::Atomic*`. I don't think KCI currently checks clippy, but we probably want that at some point. - Trevor [1]: https://rust-lang.github.io/rust-clippy/master/index.html#/disallowed_types > And i assume somebody will try to use Rust in uboot/barebox. It > probably has similar requirements to the Linux kernel? But what about > Zephyr? Or VxWorks? Darwin? > > Andrew >
diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs index c983f63fd56e..dc2d26712f26 100644 --- a/rust/kernel/sync.rs +++ b/rust/kernel/sync.rs @@ -8,6 +8,7 @@ use crate::types::Opaque; mod arc; +pub mod atomic; mod condvar; pub mod lock; mod locked_by; diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs new file mode 100644 index 000000000000..280040705fb0 --- /dev/null +++ b/rust/kernel/sync/atomic.rs @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Atomic and barrier primitives. +//! +//! These primitives should have the same semantics as their C counterparts, for precise definitions +//! of the semantics, please refer to tools/memory-model. Note that Linux Kernel Memory +//! (Consistency) Model is the only model for Rust development in kernel right now, please avoid to +//! use Rust's own atomics. + +use core::cell::UnsafeCell; + +mod arch; + +/// An atomic `i32`. +pub struct AtomicI32(pub(crate) UnsafeCell<i32>); + +impl AtomicI32 { + /// Creates a new atomic value. + pub fn new(v: i32) -> Self { + Self(UnsafeCell::new(v)) + } + + /// Adds `i` to the atomic variable with RELAXED ordering. + /// + /// Returns the old value before the add. + /// + /// # Example + /// + /// ```rust + /// use kernel::sync::atomic::AtomicI32; + /// + /// let a = AtomicI32::new(0); + /// let b = a.fetch_add_relaxed(1); + /// let c = a.fetch_add_relaxed(2); + /// + /// assert_eq!(b, 0); + /// assert_eq!(c, 1); + /// ``` + pub fn fetch_add_relaxed(&self, i: i32) -> i32 { + arch::i32_fetch_add_relaxed(&self.0, i) + } +} diff --git a/rust/kernel/sync/atomic/arch.rs b/rust/kernel/sync/atomic/arch.rs new file mode 100644 index 000000000000..3eb5a103a69a --- /dev/null +++ b/rust/kernel/sync/atomic/arch.rs @@ -0,0 +1,9 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Architectural atomic and barrier primitives. + +#[cfg(CONFIG_X86)] +pub(crate) use x86::*; + +#[cfg(CONFIG_X86)] +pub(crate) mod x86; diff --git a/rust/kernel/sync/atomic/arch/x86.rs b/rust/kernel/sync/atomic/arch/x86.rs new file mode 100644 index 000000000000..2d715f740b22 --- /dev/null +++ b/rust/kernel/sync/atomic/arch/x86.rs @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! x86 implementation for atomic and barrier primitives. + +use core::arch::asm; +use core::cell::UnsafeCell; + +/// Generates an instruction with "lock" prefix. +#[cfg(CONFIG_SMP)] +macro_rules! lock_instr { + ($i:literal) => { concat!("lock; ", $i) } +} + +#[cfg(not(CONFIG_SMP))] +macro_rules! lock_instr { + ($i:literal) => { $i } +} + +/// Atomically exchanges and adds `i` to `*v` in a wrapping way. +/// +/// Return the old value before the addition. +/// +/// # Safety +/// +/// The caller need to make sure `v` points to a valid `i32`. +unsafe fn i32_xadd(v: *mut i32, mut i: i32) -> i32 { + // SAFETY: Per function safety requirement, the address of `v` is valid for "xadd". + unsafe { + asm!( + lock_instr!("xaddl {i:e}, ({v})"), + i = inout(reg) i, + v = in(reg) v, + options(att_syntax, preserves_flags), + ); + } + + i +} + +pub(crate) fn i32_fetch_add_relaxed(v: &UnsafeCell<i32>, i: i32) -> i32 { + // SAFETY: `v.get()` points to a valid `i32`. + unsafe { i32_xadd(v.get(), i) } +}
Although Rust has its own memory ordering model (in the standard C++ memory model), having two models is not wise to start with: it increases the difficulty for correctness reasoning. Since we use Linux Kernel Memory Model for C code in kernel, it makes sense that Rust code also uses LKMM, therefore introduce a module to provide LKMM atomic primitives. Signed-off-by: Boqun Feng <boqun.feng@gmail.com> --- rust/kernel/sync.rs | 1 + rust/kernel/sync/atomic.rs | 42 ++++++++++++++++++++++++++++ rust/kernel/sync/atomic/arch.rs | 9 ++++++ rust/kernel/sync/atomic/arch/x86.rs | 43 +++++++++++++++++++++++++++++ 4 files changed, 95 insertions(+) create mode 100644 rust/kernel/sync/atomic.rs create mode 100644 rust/kernel/sync/atomic/arch.rs create mode 100644 rust/kernel/sync/atomic/arch/x86.rs