diff mbox series

[WIP,1/3] rust: Introduce atomic module

Message ID 20240322233838.868874-2-boqun.feng@gmail.com (mailing list archive)
State New, archived
Headers show
Series Memory model and atomic API in Rust | expand

Commit Message

Boqun Feng March 22, 2024, 11:38 p.m. UTC
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

Comments

Andrew Lunn March 22, 2024, 11:52 p.m. UTC | #1
> +//! 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
Boqun Feng March 23, 2024, 12:03 a.m. UTC | #2
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
Alice Ryhl March 23, 2024, 9:58 a.m. UTC | #3
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
Andrew Lunn March 23, 2024, 2:10 p.m. UTC | #4
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
Miguel Ojeda March 23, 2024, 7:09 p.m. UTC | #5
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
Miguel Ojeda March 23, 2024, 7:13 p.m. UTC | #6
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
Boqun Feng March 23, 2024, 7:30 p.m. UTC | #7
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
Trevor Gross March 26, 2024, 5:56 a.m. UTC | #8
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 mbox series

Patch

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) }
+}