From: Jens Korinth <jens.korinth@tuta.io>
Similar to `Once` in Rust's standard library, but with the same
non-blocking behavior as the kernel's `DO_ONCE_LITE` macro. Abstraction
allows easy replacement of the underlying sync mechanisms, see
https://lore.kernel.org/rust-for-linux/20241109-pr_once_macros-v3-0-6beb24e0cac8@tuta.io/.
Suggested-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: Jens Korinth <jens.korinth@tuta.io>
---
rust/kernel/lib.rs | 1 +
rust/kernel/once_lite.rs | 127 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 128 insertions(+)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index bf8d7f841f9425d19a24f3910929839cfe705c7f..2b0a80435d24f5e168679ec2e25bd68cd970dcdd 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -44,6 +44,7 @@
pub mod list;
#[cfg(CONFIG_NET)]
pub mod net;
+pub mod once_lite;
pub mod page;
pub mod prelude;
pub mod print;
diff --git a/rust/kernel/once_lite.rs b/rust/kernel/once_lite.rs
new file mode 100644
index 0000000000000000000000000000000000000000..723c3244fc856fe974ddd33de5415e7ced37f315
--- /dev/null
+++ b/rust/kernel/once_lite.rs
@@ -0,0 +1,127 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! A one-time only global execution primitive.
+//!
+//! This primitive is meant to be used to execute code only once. It is
+//! similar in design to Rust's
+//! [`std::sync:Once`](https://doc.rust-lang.org/std/sync/struct.Once.html),
+//! but borrowing the non-blocking mechanism used in the kernel's
+//! [`DO_ONCE_LITE`] macro.
+//!
+//! An example use case would be to print a message only the first time.
+//!
+//! [`DO_ONCE_LITE`]: srctree/include/linux/once_lite.h
+//!
+//! C header: [`include/linux/once_lite.h`](srctree/include/linux/once_lite.h)
+//!
+//! Reference: <https://doc.rust-lang.org/std/sync/struct.Once.html>
+
+use core::sync::atomic::{AtomicBool, Ordering::Relaxed};
+
+/// A low-level synchronization primitive for one-time global execution.
+///
+/// Based on the
+/// [`std::sync:Once`](https://doc.rust-lang.org/std/sync/struct.Once.html)
+/// interface, but internally equivalent the kernel's [`DO_ONCE_LITE`]
+/// macro. The Rust macro `do_once_lite` replacing it uses `OnceLite`
+/// internally.
+///
+/// [`DO_ONCE_LITE`]: srctree/include/linux/once_lite.h
+///
+/// # Examples
+///
+/// ```rust
+/// static START: kernel::once_lite::OnceLite =
+/// kernel::once_lite::OnceLite::new();
+///
+/// let mut x: i32 = 0;
+///
+/// START.call_once(|| {
+/// // run initialization here
+/// x = 42;
+/// });
+/// while !START.is_completed() { /* busy wait */ }
+/// assert_eq!(x, 42);
+/// ```
+///
+pub struct OnceLite(AtomicBool, AtomicBool);
+
+impl OnceLite {
+ /// Creates a new `OnceLite` value.
+ #[inline(always)]
+ pub const fn new() -> Self {
+ Self(AtomicBool::new(false), AtomicBool::new(false))
+ }
+
+ /// Performs an initialization routine once and only once. The given
+ /// closure will be executed if this is the first time `call_once` has
+ /// been called, and otherwise the routine will not be invoked.
+ ///
+ /// This method will _not_ block the calling thread if another
+ /// initialization is currently running. It is _not_ guaranteed that the
+ /// initialization routine will have completed by the time the calling
+ /// thread continues execution.
+ ///
+ /// Note that this is different from the guarantees made by
+ /// [`std::sync::Once`], but identical to the way the implementation of
+ /// the kernel's [`DO_ONCE_LITE_IF`] macro is behaving at the time of
+ /// writing.
+ ///
+ /// [`std::sync::Once`]: https://doc.rust-lang.org/std/sync/struct.Once.html
+ /// [`DO_ONCE_LITE_IF`]: srctree/include/once_lite.h
+ #[inline(always)]
+ pub fn call_once<F: FnOnce()>(&self, f: F) {
+ if !self.0.load(Relaxed) && !self.0.swap(true, Relaxed) {
+ f()
+ };
+ self.1.store(true, Relaxed);
+ }
+
+ /// Returns `true` if some `call_once` call has completed successfully.
+ /// Specifically, `is_completed` will return `false` in the following
+ /// situations:
+ ///
+ /// 1. `call_once()` was not called at all,
+ /// 2. `call_once()` was called, but has not yet completed.
+ ///
+ /// # Examples
+ ///
+ /// ```rust
+ /// static INIT: kernel::once_lite::OnceLite =
+ /// kernel::once_lite::OnceLite::new();
+ ///
+ /// assert_eq!(INIT.is_completed(), false);
+ /// INIT.call_once(|| {
+ /// assert_eq!(INIT.is_completed(), false);
+ /// });
+ /// assert_eq!(INIT.is_completed(), true);
+ /// ```
+ #[inline(always)]
+ pub fn is_completed(&self) -> bool {
+ self.1.load(Relaxed)
+ }
+}
+
+/// Executes code only once.
+///
+/// Equivalent to the kernel's [`DO_ONCE_LITE`] macro: Expression is
+/// evaluated at most once by the first thread, other threads will not be
+/// blocked while executing in first thread, nor are there any guarantees
+/// regarding the visibility of side-effects of the called expression.
+///
+/// [`DO_ONCE_LITE`]: srctree/include/linux/once_lite.h
+///
+/// # Examples
+///
+/// ```
+/// let mut x: i32 = 0;
+/// kernel::do_once_lite!((||{x = 42;})());
+/// ```
+#[macro_export]
+macro_rules! do_once_lite {
+ ($e: expr) => {{
+ #[link_section = ".data.once"]
+ static ONCE: $crate::once_lite::OnceLite = $crate::once_lite::OnceLite::new();
+ ONCE.call_once(|| $e);
+ }};
+}
--
2.47.0"Jens Korinth via B4 Relay" <devnull+jens.korinth.tuta.io@kernel.org> writes:
> From: Jens Korinth <jens.korinth@tuta.io>
>
> Similar to `Once` in Rust's standard library, but with the same
> non-blocking behavior as the kernel's `DO_ONCE_LITE` macro. Abstraction
> allows easy replacement of the underlying sync mechanisms, see
>
> https://lore.kernel.org/rust-for-linux/20241109-pr_once_macros-v3-0-6beb24e0cac8@tuta.io/.
>
> Suggested-by: Boqun Feng <boqun.feng@gmail.com>
> Signed-off-by: Jens Korinth <jens.korinth@tuta.io>
Thanks for the patch!
I was using the series for `pr_warn_once!` elsewhere, and clippy gave me
this suggestion:
CLIPPY L rust/kernel.o
error: you should consider adding a `Default` implementation for `OnceLite`
--> /home/aeh/src/linux-rust/module-params/rust/kernel/once_lite.rs:52:5
|
52 | / pub const fn new() -> Self {
53 | | Self(AtomicBool::new(false), AtomicBool::new(false))
54 | | }
| |_____^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#new_without_default
= note: `-D clippy::new-without-default` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::new_without_default)]`
help: try adding this
|
49 + impl Default for OnceLite {
50 + fn default() -> Self {
51 + Self::new()
52 + }
53 + }
|
Best regards,
Andreas Hindborg
On Tue, Nov 26, 2024 at 5:41 PM Jens Korinth via B4 Relay
<devnull+jens.korinth.tuta.io@kernel.org> wrote:
>
> From: Jens Korinth <jens.korinth@tuta.io>
>
> Similar to `Once` in Rust's standard library, but with the same
> non-blocking behavior as the kernel's `DO_ONCE_LITE` macro. Abstraction
> allows easy replacement of the underlying sync mechanisms, see
>
> https://lore.kernel.org/rust-for-linux/20241109-pr_once_macros-v3-0-6beb24e0cac8@tuta.io/.
>
> Suggested-by: Boqun Feng <boqun.feng@gmail.com>
> Signed-off-by: Jens Korinth <jens.korinth@tuta.io>
> + pub fn is_completed(&self) -> bool {
> + self.1.load(Relaxed)
> + }
What is the use-case for this function? Why not just have one atomic?
> What is the use-case for this function? `DO_ONCE_LITE` has very few uses in C; frankly, the only other use I could think of was initialization. But since `OnceLite` cannot block or guarantee visibility of the side-effects of the `call_once` expression in other threads, it can't be used for this case, either. _Unless_ there is some mechanism to wait voluntarily when this is required, hence `is_completed`. (And it also exists in `std::sync::Once`.) `DO_ONCE_LITE_IF` has more uses in C, but this is a bit harder to do well with `OnceLite`: A `do_once_lite_if` Rust macro can't short-circuit the condition to avoid the evaluation if the atomic load already shows that it has been done / is being done rn. Maybe a `pub fn call_once<C: FnOnce() -> bool, F: FnOnce()>(cond: C, f: F)` could be used to simulate the effect. Thoughts? > Why not just have one atomic? Do you also have an `AtomicU32` state var in mind, as Daniel suggested? Jens
On Wed, Nov 27, 2024 at 9:12 PM <jens.korinth@tuta.io> wrote: > > > What is the use-case for this function? > > `DO_ONCE_LITE` has very few uses in C; frankly, the only other use I could think > of was initialization. But since `OnceLite` cannot block or guarantee visibility > of the side-effects of the `call_once` expression in other threads, it can't be > used for this case, either. _Unless_ there is some mechanism to wait > voluntarily when this is required, hence `is_completed`. (And it also exists in > `std::sync::Once`.) > > `DO_ONCE_LITE_IF` has more uses in C, but this is a bit harder to do well with > `OnceLite`: A `do_once_lite_if` Rust macro can't short-circuit the condition to > avoid the evaluation if the atomic load already shows that it has been done / is > being done rn. Maybe a > `pub fn call_once<C: FnOnce() -> bool, F: FnOnce()>(cond: C, f: F)` could be > used to simulate the effect. Thoughts? > > > Why not just have one atomic? > > Do you also have an `AtomicU32` state var in mind, as Daniel suggested? What I had in mind was to use a single AtomicBool and get rid of the `is_completed` logic entirely. The purpose is to use this for printing once, and printing doesn't need `is_completed`. We can have another helper for other purposes. Alice
I'm afraid you lost me. You wrote: > Using a Once type for this seems like a good idea to me. and > One advantage of using a Once type is that we can use core::sync::atomic > until we have working LKMM atomics and then we just swap out the Once > type without having to modify the warn_once abstractions. That made sense to me, so I started in this direction. `std::sync::Once` has `is_completed` [1], which made particular sense to implement in my mind to increase the utility of `OnceLite`. [1]: https://doc.rust-lang.org/std/sync/struct.Once.html#method.is_completed > The purpose is to use this for printing once, and printing doesn't need > `is_completed`. We can have another helper for other purposes. Why have `OnceLite` then at all, instead of the hidden Rust macro that was proposed initially? Jens
On Thu, Dec 5, 2024 at 7:49 AM <jens.korinth@tuta.io> wrote: > > I'm afraid you lost me. You wrote: > > > Using a Once type for this seems like a good idea to me. > > and > > > One advantage of using a Once type is that we can use core::sync::atomic > > until we have working LKMM atomics and then we just swap out the Once > > type without having to modify the warn_once abstractions. > > That made sense to me, so I started in this direction. `std::sync::Once` > has `is_completed` [1], which made particular sense to implement in my > mind to increase the utility of `OnceLite`. > > [1]: https://doc.rust-lang.org/std/sync/struct.Once.html#method.is_completed The stdlib Once type guarantees that when call_once has returned, then the closure has finished running *even* if you are not the caller who is running the closure. It achieves this by having other callers go to sleep until the main caller completes. If we actually want to mirror Once, then we should have that logic too, and I really don't think we want such complicated logic in our pr_warn_once macro. > > The purpose is to use this for printing once, and printing doesn't need > > `is_completed`. We can have another helper for other purposes. > > Why have `OnceLite` then at all, instead of the hidden Rust macro that was > proposed initially? Well, one advantage is that when Boqun manages to add support for LKMM atomics, we can switch them out without having to modify macro code. Moving logic out of macro code is usually a good idea. It's also possible that there are other user-cases of an OnceLite that doesn't have is_completed. Alice
On Tue, Nov 26, 2024 at 5:41 PM Jens Korinth via B4 Relay
<devnull+jens.korinth.tuta.io@kernel.org> wrote:
>
> Similar to `Once` in Rust's standard library, but with the same
> non-blocking behavior as the kernel's `DO_ONCE_LITE` macro. Abstraction
> allows easy replacement of the underlying sync mechanisms, see
>
> https://lore.kernel.org/rust-for-linux/20241109-pr_once_macros-v3-0-6beb24e0cac8@tuta.io/.
Nit: you could use a Link tag for this, e.g.
Link: https://lore.kernel.org/rust-for-linux/20241109-pr_once_macros-v3-0-6beb24e0cac8@tuta.io/
[1]
And then you can refer to it using [1], like "see [1].".
> +//! This primitive is meant to be used to execute code only once. It is
> +//! similar in design to Rust's
> +//! [`std::sync:Once`](https://doc.rust-lang.org/std/sync/struct.Once.html),
In one case you used a shortcut reference link for this -- perhaps you
could do that here and below.
> +//! but borrowing the non-blocking mechanism used in the kernel's
> +//! [`DO_ONCE_LITE`] macro.
I would suggest mentioning C here, e.g. C macro, to reduce ambiguity
(it is true we have just used "kernel's" in some cases).
> +//! An example use case would be to print a message only the first time.
Ideally we would mention one or two concrete use cases here, e.g. you
could grep to see a couple common C use cases.
Also, since we are going to have the `pr_..._once` macros, it may not
be the best example use case, since the developer would use those
instead, right?
The docs look well-formatted etc., so thanks for taking the time writing them :)
> +/// A low-level synchronization primitive for one-time global execution.
I wonder if we should move part of the docs from the module level here
to avoid duplication.
> +/// macro. The Rust macro `do_once_lite` replacing it uses `OnceLite`
Please link these with intra-doc links.
> +/// ```rust
You can remove `rust` from this one, like in the others.
> +/// static START: kernel::once_lite::OnceLite =
> +/// kernel::once_lite::OnceLite::new();
I would have a hidden `use` line to simplify the example -- since we
are reading the example about this item, it is OK to shorten it here,
e.g.
static START: OnceLite = OnceLite::new();
> +/// // run initialization here
Please use the usual comment style: "Run initialization here.".
> +/// while !START.is_completed() { /* busy wait */ }
Hmm... without threads this looks a bit strange.
> + /// Creates a new `OnceLite` value.
Please use intra-doc links wherever possible.
> + /// This method will _not_ block the calling thread if another
Should we save "does _not_ ... regardless ..."? i.e. it never blocks.
> + /// [`std::sync::Once`], but identical to the way the implementation of
> + /// the kernel's [`DO_ONCE_LITE_IF`] macro is behaving at the time of
> + /// writing.
"at the time of writing" is implicit, so we don't need to say it.
(Of course, it would be nice to have someone making sure we don't get
out of sync!
> +/// Executes code only once.
"an expression" or "a Rust expression" could perhaps be more precise
and hints what the argument is (which may help those not accustomed to
macro signatures).
> +/// kernel::do_once_lite!((||{x = 42;})());
Please format the examples if possible. Not a big deal, but it is always nicer.
Can we add an `assert` here too like in the other examples, so that
this doubles as a test?
By the way, does this need to be an immediately called closure? i.e.
the macro takes an expression, can't we do e.g.
do_once_lite!(x = 42);
?
> + #[link_section = ".data.once"]
> + static ONCE: $crate::once_lite::OnceLite = $crate::once_lite::OnceLite::new();
I let Boqun et al. review the sync parts, but a few questions: Do we
want/need two `AtomicBool`s? Should the docs for `OnceLite`
mention/suggest `link_section`? Should we have a macro to declare them
instead?
By the way, please test with `CLIPPY=1`, I got `new_without_default`.
Cheers,
Miguel
On 11/26/24 5:40 PM, Jens Korinth via B4 Relay wrote:
> From: Jens Korinth <jens.korinth@tuta.io>
>
> Similar to `Once` in Rust's standard library, but with the same
> non-blocking behavior as the kernel's `DO_ONCE_LITE` macro. Abstraction
> allows easy replacement of the underlying sync mechanisms, see
>
> https://lore.kernel.org/rust-for-linux/20241109-pr_once_macros-v3-0-6beb24e0cac8@tuta.io/.
>
> Suggested-by: Boqun Feng <boqun.feng@gmail.com>
> Signed-off-by: Jens Korinth <jens.korinth@tuta.io>
> ---
> rust/kernel/lib.rs | 1 +
> rust/kernel/once_lite.rs | 127 +++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 128 insertions(+)
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index bf8d7f841f9425d19a24f3910929839cfe705c7f..2b0a80435d24f5e168679ec2e25bd68cd970dcdd 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -44,6 +44,7 @@
> pub mod list;
> #[cfg(CONFIG_NET)]
> pub mod net;
> +pub mod once_lite;
> pub mod page;
> pub mod prelude;
> pub mod print;
> diff --git a/rust/kernel/once_lite.rs b/rust/kernel/once_lite.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..723c3244fc856fe974ddd33de5415e7ced37f315
> --- /dev/null
> +++ b/rust/kernel/once_lite.rs
> @@ -0,0 +1,127 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! A one-time only global execution primitive.
> +//!
> +//! This primitive is meant to be used to execute code only once. It is
> +//! similar in design to Rust's
> +//! [`std::sync:Once`](https://doc.rust-lang.org/std/sync/struct.Once.html),
> +//! but borrowing the non-blocking mechanism used in the kernel's
> +//! [`DO_ONCE_LITE`] macro.
> +//!
> +//! An example use case would be to print a message only the first time.
> +//!
> +//! [`DO_ONCE_LITE`]: srctree/include/linux/once_lite.h
> +//!
> +//! C header: [`include/linux/once_lite.h`](srctree/include/linux/once_lite.h)
> +//!
> +//! Reference: <https://doc.rust-lang.org/std/sync/struct.Once.html>
> +
> +use core::sync::atomic::{AtomicBool, Ordering::Relaxed};
> +
> +/// A low-level synchronization primitive for one-time global execution.
> +///
> +/// Based on the
> +/// [`std::sync:Once`](https://doc.rust-lang.org/std/sync/struct.Once.html)
> +/// interface, but internally equivalent the kernel's [`DO_ONCE_LITE`]
> +/// macro. The Rust macro `do_once_lite` replacing it uses `OnceLite`
> +/// internally.
> +///
> +/// [`DO_ONCE_LITE`]: srctree/include/linux/once_lite.h
> +///
> +/// # Examples
> +///
> +/// ```rust
The `rust` part should be default value for rustdoc tests, can we please
omit that?
> +/// static START: kernel::once_lite::OnceLite =
> +/// kernel::once_lite::OnceLite::new();
> +///
> +/// let mut x: i32 = 0;
> +///
> +/// START.call_once(|| {
> +/// // run initialization here
> +/// x = 42;
> +/// });
> +/// while !START.is_completed() { /* busy wait */ }
> +/// assert_eq!(x, 42);
> +/// ```
> +///
> +pub struct OnceLite(AtomicBool, AtomicBool);
Have you considered it to be implemented like `AtomicU32`? I think™ that
one atomic variable is more than enough.
> +
> +impl OnceLite {
> + /// Creates a new `OnceLite` value.
> + #[inline(always)]
> + pub const fn new() -> Self {
> + Self(AtomicBool::new(false), AtomicBool::new(false))
> + }
> +
> + /// Performs an initialization routine once and only once. The given
> + /// closure will be executed if this is the first time `call_once` has
> + /// been called, and otherwise the routine will not be invoked.
> + ///
> + /// This method will _not_ block the calling thread if another
> + /// initialization is currently running. It is _not_ guaranteed that the
> + /// initialization routine will have completed by the time the calling
> + /// thread continues execution.
> + ///
> + /// Note that this is different from the guarantees made by
> + /// [`std::sync::Once`], but identical to the way the implementation of
> + /// the kernel's [`DO_ONCE_LITE_IF`] macro is behaving at the time of
> + /// writing.
> + ///
> + /// [`std::sync::Once`]: https://doc.rust-lang.org/std/sync/struct.Once.html
> + /// [`DO_ONCE_LITE_IF`]: srctree/include/once_lite.h
> + #[inline(always)]
> + pub fn call_once<F: FnOnce()>(&self, f: F) {
> + if !self.0.load(Relaxed) && !self.0.swap(true, Relaxed) {
> + f()
> + };
> + self.1.store(true, Relaxed);
I think the memory ordering is wrong here. Since it is `Relaxed`, the
`self.1` and `self.0` can get reordered during execution. So `self.0`
can be false, even though `self.1` will be true. Not on x86, but on
architectures with weaker memory models it can happen.
Even the implementation in the Rust std, uses at least
`Acquire`/`Release`, where the reordering shouldn't be possible see [1].
[1]:
https://github.com/rust-lang/rust/blob/master/library/std/src/sys/sync/once/futex.rs
> + }
> +
> + /// Returns `true` if some `call_once` call has completed successfully.
> + /// Specifically, `is_completed` will return `false` in the following
> + /// situations:
> + ///
> + /// 1. `call_once()` was not called at all,
> + /// 2. `call_once()` was called, but has not yet completed.
> + ///
> + /// # Examples
> + ///
> + /// ```rust
Also here the `rust` part should be the default value.
> + /// static INIT: kernel::once_lite::OnceLite =
> + /// kernel::once_lite::OnceLite::new();
> + ///
> + /// assert_eq!(INIT.is_completed(), false);
> + /// INIT.call_once(|| {
> + /// assert_eq!(INIT.is_completed(), false);
> + /// });
> + /// assert_eq!(INIT.is_completed(), true);
> + /// ```
> + #[inline(always)]
> + pub fn is_completed(&self) -> bool {
> + self.1.load(Relaxed)
> + }
> +}
> +
> +/// Executes code only once.
> +///
> +/// Equivalent to the kernel's [`DO_ONCE_LITE`] macro: Expression is
> +/// evaluated at most once by the first thread, other threads will not be
> +/// blocked while executing in first thread, nor are there any guarantees
> +/// regarding the visibility of side-effects of the called expression.
> +///
> +/// [`DO_ONCE_LITE`]: srctree/include/linux/once_lite.h
> +///
> +/// # Examples
> +///
> +/// ```
> +/// let mut x: i32 = 0;
> +/// kernel::do_once_lite!((||{x = 42;})());
> +/// ```
> +#[macro_export]
> +macro_rules! do_once_lite {
> + ($e: expr) => {{
> + #[link_section = ".data.once"]
> + static ONCE: $crate::once_lite::OnceLite = $crate::once_lite::OnceLite::new();
> + ONCE.call_once(|| $e);
> + }};
> +}
>
Thanks for working on that!
Daniel
> Have you considered it to be implemented like `AtomicU32`? I think™ that
> one atomic variable is more than enough.
Just to clarify - you mean something like this? Nevermind the magic numbers,
I'd replace them, of course.
diff --git a/rust/kernel/once_lite.rs b/rust/kernel/once_lite.rs
index 723c3244fc85..0622ecbfced5 100644
--- a/rust/kernel/once_lite.rs
+++ b/rust/kernel/once_lite.rs
@@ -16,7 +16,7 @@
//!
//! Reference: <https://doc.rust-lang.org/std/sync/struct.Once.html>
-use core::sync::atomic::{AtomicBool, Ordering::Relaxed};
+use core::sync::atomic::{AtomicU32, Ordering::Acquire, Ordering::Relaxed};
/// A low-level synchronization primitive for one-time global execution.
///
@@ -44,13 +44,13 @@
/// assert_eq!(x, 42);
/// ```
///
-pub struct OnceLite(AtomicBool, AtomicBool);
+pub struct OnceLite(AtomicU32);
impl OnceLite {
/// Creates a new `OnceLite` value.
#[inline(always)]
pub const fn new() -> Self {
- Self(AtomicBool::new(false), AtomicBool::new(false))
+ Self(AtomicU32::new(0))
}
/// Performs an initialization routine once and only once. The given
@@ -71,10 +71,10 @@ pub const fn new() -> Self {
/// [`DO_ONCE_LITE_IF`]: srctree/include/once_lite.h
#[inline(always)]
pub fn call_once<F: FnOnce()>(&self, f: F) {
- if !self.0.load(Relaxed) && !self.0.swap(true, Relaxed) {
- f()
+ if self.0.load(Relaxed) == 0 && self.0.compare_exchange(0, 1, Acquire, Relaxed) == Ok(0) {
+ f();
+ self.0.store(2, Relaxed);
};
- self.1.store(true, Relaxed);
}
/// Returns `true` if some `call_once` call has completed successfully.
@@ -98,7 +98,7 @@ pub fn call_once<F: FnOnce()>(&self, f: F) {
/// ```
#[inline(always)]
pub fn is_completed(&self) -> bool {
- self.1.load(Relaxed)
+ self.0.load(Relaxed) > 1
}
}
> The `rust` part should be default value for rustdoc tests, can we please
> omit that?
Will do!
Jens
On 11/27/24 8:46 PM, jens.korinth@tuta.io wrote:
>> Have you considered it to be implemented like `AtomicU32`? I think™ that
>> one atomic variable is more than enough.
>
> Just to clarify - you mean something like this? Nevermind the magic numbers,
> I'd replace them, of course.
Yes, that's what I meant. But as Alice pointed out, you _may_ be able to
reduce it to the one AtomicBool.
Daniel
>
> diff --git a/rust/kernel/once_lite.rs b/rust/kernel/once_lite.rs
> index 723c3244fc85..0622ecbfced5 100644
> --- a/rust/kernel/once_lite.rs
> +++ b/rust/kernel/once_lite.rs
> @@ -16,7 +16,7 @@
> //!
> //! Reference: <https://doc.rust-lang.org/std/sync/struct.Once.html>
>
> -use core::sync::atomic::{AtomicBool, Ordering::Relaxed};
> +use core::sync::atomic::{AtomicU32, Ordering::Acquire, Ordering::Relaxed};
>
> /// A low-level synchronization primitive for one-time global execution.
> ///
> @@ -44,13 +44,13 @@
> /// assert_eq!(x, 42);
> /// ```
> ///
> -pub struct OnceLite(AtomicBool, AtomicBool);
> +pub struct OnceLite(AtomicU32);
>
> impl OnceLite {
> /// Creates a new `OnceLite` value.
> #[inline(always)]
> pub const fn new() -> Self {
> - Self(AtomicBool::new(false), AtomicBool::new(false))
> + Self(AtomicU32::new(0))
> }
>
> /// Performs an initialization routine once and only once. The given
> @@ -71,10 +71,10 @@ pub const fn new() -> Self {
> /// [`DO_ONCE_LITE_IF`]: srctree/include/once_lite.h
> #[inline(always)]
> pub fn call_once<F: FnOnce()>(&self, f: F) {
> - if !self.0.load(Relaxed) && !self.0.swap(true, Relaxed) {
> - f()
> + if self.0.load(Relaxed) == 0 && self.0.compare_exchange(0, 1, Acquire, Relaxed) == Ok(0) {
> + f();
> + self.0.store(2, Relaxed);
> };
> - self.1.store(true, Relaxed);
> }
>
> /// Returns `true` if some `call_once` call has completed successfully.
> @@ -98,7 +98,7 @@ pub fn call_once<F: FnOnce()>(&self, f: F) {
> /// ```
> #[inline(always)]
> pub fn is_completed(&self) -> bool {
> - self.1.load(Relaxed)
> + self.0.load(Relaxed) > 1
> }
> }
>
>> The `rust` part should be default value for rustdoc tests, can we please
>> omit that?
>
> Will do!
>
> Jens
© 2016 - 2026 Red Hat, Inc.