* [PATCH v4 0/3] rust: Add pr_*_once macros
@ 2024-11-26 16:40 ` Jens Korinth via B4 Relay
2024-11-26 16:40 ` [PATCH v4 1/3] rust: Add `OnceLite` for executing code once Jens Korinth via B4 Relay
` (4 more replies)
0 siblings, 5 replies; 27+ messages in thread
From: Jens Korinth via B4 Relay @ 2024-11-26 16:40 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross
Cc: rust-for-linux, FUJITA Tomonori, Dirk Behme, linux-kernel,
Jens Korinth
Add Rust version of pr_[emerg|alert|crit|err|warn|notic|info]_once
functions, which print a message only once.
Introduces a `OnceLite` abstraction similar to Rust's
[`std::sync::Once`](https://doc.rust-lang.org/std/sync/struct.Once.html)
but using the non-blocking mechanism from the kernel's `DO_ONCE_LITE`
macro, which is used to define the `do_once_lite` Rust macro.
First use case are an implementation of `pr_*_once` message macros to
print a message only once.
v4:
- Move `do_once_lite` macro implementation to `OnceLite` abstraction
- Use `OnceLite` in `do_once_lite`
- More documentation, examples
v3: https://lore.kernel.org/rust-for-linux/20241109-pr_once_macros-v3-0-6beb24e0cac8@tuta.io/
- Fix rustdoc error, formatting issues
- Fix missing Signed-off-by
v2: https://lore.kernel.org/r/20241107-pr_once_macros-v2-0-dc0317ff301e@tuta.io
- Split patch into do_once_lite part and pr_*_once macros
- Add macro rule for call without condition => renamed to do_once_lite
- Used condition-less call in pr_*_once macros
- Added examples
- Removed TODO in kernel/error.rs using pr_warn_once
v1: https://lore.kernel.org/rust-for-linux/20241106.083113.356536037967804464.fujita.tomonori@gmail.com/
Co-developed-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
Co-developed-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: Jens Korinth <jens.korinth@tuta.io>
---
FUJITA Tomonori (1):
rust: print: Add pr_*_once macros
Jens Korinth (2):
rust: Add `OnceLite` for executing code once
rust: error: Replace pr_warn by pr_warn_once
rust/kernel/error.rs | 3 +-
rust/kernel/lib.rs | 1 +
rust/kernel/once_lite.rs | 127 +++++++++++++++++++++++++++++++++++++++++++++++
rust/kernel/print.rs | 126 ++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 255 insertions(+), 2 deletions(-)
---
base-commit: b2603f8ac8217bc59f5c7f248ac248423b9b99cb
change-id: 20241107-pr_once_macros-6438e6f5b923
Best regards,
--
Jens Korinth <jens.korinth@tuta.io>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v4 1/3] rust: Add `OnceLite` for executing code once
2024-11-26 16:40 ` [PATCH v4 0/3] rust: Add pr_*_once macros Jens Korinth via B4 Relay
@ 2024-11-26 16:40 ` Jens Korinth via B4 Relay
2024-11-26 18:57 ` Daniel Sedlak
` (3 more replies)
2024-11-26 16:40 ` [PATCH v4 2/3] rust: print: Add pr_*_once macros Jens Korinth via B4 Relay
` (3 subsequent siblings)
4 siblings, 4 replies; 27+ messages in thread
From: Jens Korinth via B4 Relay @ 2024-11-26 16:40 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross
Cc: rust-for-linux, FUJITA Tomonori, Dirk Behme, linux-kernel,
Jens Korinth
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
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v4 2/3] rust: print: Add pr_*_once macros
2024-11-26 16:40 ` [PATCH v4 0/3] rust: Add pr_*_once macros Jens Korinth via B4 Relay
2024-11-26 16:40 ` [PATCH v4 1/3] rust: Add `OnceLite` for executing code once Jens Korinth via B4 Relay
@ 2024-11-26 16:40 ` Jens Korinth via B4 Relay
2024-11-26 16:40 ` [PATCH v4 3/3] rust: error: Replace pr_warn by pr_warn_once Jens Korinth via B4 Relay
` (2 subsequent siblings)
4 siblings, 0 replies; 27+ messages in thread
From: Jens Korinth via B4 Relay @ 2024-11-26 16:40 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross
Cc: rust-for-linux, FUJITA Tomonori, Dirk Behme, linux-kernel,
Jens Korinth
From: FUJITA Tomonori <fujita.tomonori@gmail.com>
Add Rust version of pr_[emerg|alert|crit|err|warn|notic|info]_once
functions, which print a message only once.
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
Signed-off-by: Jens Korinth <jens.korinth@tuta.io>
---
rust/kernel/print.rs | 126 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 126 insertions(+)
diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs
index a28077a7cb301193dcca293befb096b2dd153202..f354b32684a2b3c6c5d9aca6ef9f68e52c870bcf 100644
--- a/rust/kernel/print.rs
+++ b/rust/kernel/print.rs
@@ -414,3 +414,129 @@ macro_rules! pr_cont (
$crate::print_macro!($crate::print::format_strings::CONT, true, $($arg)*)
)
);
+
+/// Prints an emergency-level message (level 0) only once.
+///
+/// Equivalent to the kernel's [`pr_emerg_once`] macro.
+///
+/// [`pr_emerg_once`]: srctree/include/linux/printk.h
+///
+/// # Examples
+///
+/// ```
+/// kernel::pr_emerg_once!("hello {}\n", "there");
+/// ```
+#[macro_export]
+macro_rules! pr_emerg_once (
+ ($($arg:tt)*) => (
+ $crate::do_once_lite!($crate::pr_emerg!($($arg)*))
+ )
+);
+
+/// Prints an alert-level message (level 1) only once.
+///
+/// Equivalent to the kernel's [`pr_alert_once`] macro.
+///
+/// [`pr_alert_once`]: srctree/include/linux/printk.h
+///
+/// # Examples
+///
+/// ```
+/// kernel::pr_alert_once!("hello {}\n", "there");
+/// ```
+#[macro_export]
+macro_rules! pr_alert_once (
+ ($($arg:tt)*) => (
+ $crate::do_once_lite!($crate::pr_alert!($($arg)*))
+ )
+);
+
+/// Prints a critical-level message (level 2) only once.
+///
+/// Equivalent to the kernel's [`pr_crit_once`] macro.
+///
+/// [`pr_crit_once`]: srctree/include/linux/printk.h
+///
+/// # Examples
+///
+/// ```
+/// kernel::pr_crit_once!("hello {}\n", "there");
+/// ```
+#[macro_export]
+macro_rules! pr_crit_once (
+ ($($arg:tt)*) => (
+ $crate::do_once_lite!($crate::pr_crit!($($arg)*))
+ )
+);
+
+/// Prints an error-level message (level 3) only once.
+///
+/// Equivalent to the kernel's [`pr_err_once`] macro.
+///
+/// # Examples
+///
+/// [`pr_err_once`]: srctree/include/linux/printk.h
+///
+/// ```
+/// kernel::pr_err_once!("hello {}\n", "there");
+/// ```
+#[macro_export]
+macro_rules! pr_err_once (
+ ($($arg:tt)*) => (
+ $crate::do_once_lite!($crate::pr_err!($($arg)*))
+ )
+);
+
+/// Prints a warning-level message (level 4) only once.
+///
+/// Equivalent to the kernel's [`pr_warn_once`] macro.
+///
+/// [`pr_warn_once`]: srctree/include/linux/printk.h
+///
+/// # Examples
+///
+/// ```
+/// kernel::pr_warn_once!("hello {}\n", "there");
+/// ```
+#[macro_export]
+macro_rules! pr_warn_once (
+ ($($arg:tt)*) => (
+ $crate::do_once_lite!($crate::pr_warn!($($arg)*))
+ )
+);
+
+/// Prints a notice-level message (level 5) only once.
+///
+/// Equivalent to the kernel's [`pr_notice_once`] macro.
+///
+/// [`pr_notice_once`]: srctree/include/linux/printk.h
+///
+/// # Examples
+///
+/// ```
+/// kernel::pr_notice_once!("hello {}\n", "there");
+/// ```
+#[macro_export]
+macro_rules! pr_notice_once (
+ ($($arg:tt)*) => (
+ $crate::do_once_lite!($crate::pr_notice!($($arg)*))
+ )
+);
+
+/// Prints an info-level message (level 6) only once.
+///
+/// Equivalent to the kernel's [`pr_info_once`] macro.
+///
+/// [`pr_info_once`]: srctree/include/linux/printk.h
+///
+/// # Examples
+///
+/// ```
+/// kernel::pr_info_once!("hello {}\n", "there");
+/// ```
+#[macro_export]
+macro_rules! pr_info_once (
+ ($($arg:tt)*) => (
+ $crate::do_once_lite!($crate::pr_info!($($arg)*))
+ )
+);
--
2.47.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v4 3/3] rust: error: Replace pr_warn by pr_warn_once
2024-11-26 16:40 ` [PATCH v4 0/3] rust: Add pr_*_once macros Jens Korinth via B4 Relay
2024-11-26 16:40 ` [PATCH v4 1/3] rust: Add `OnceLite` for executing code once Jens Korinth via B4 Relay
2024-11-26 16:40 ` [PATCH v4 2/3] rust: print: Add pr_*_once macros Jens Korinth via B4 Relay
@ 2024-11-26 16:40 ` Jens Korinth via B4 Relay
2024-11-26 17:07 ` Miguel Ojeda
2024-11-26 16:59 ` [PATCH v4 0/3] rust: Add pr_*_once macros Miguel Ojeda
2025-02-11 15:04 ` Andreas Hindborg
4 siblings, 1 reply; 27+ messages in thread
From: Jens Korinth via B4 Relay @ 2024-11-26 16:40 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross
Cc: rust-for-linux, FUJITA Tomonori, Dirk Behme, linux-kernel,
Jens Korinth
From: Jens Korinth <jens.korinth@tuta.io>
Use new pr_warn_once macro to resolve TODO in error.rs.
Signed-off-by: Jens Korinth <jens.korinth@tuta.io>
---
rust/kernel/error.rs | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index 52c5024324474fc1306047f3fd7516f0023d0313..f6813dace1128b7ef91f64e79cd83bb64995bf97 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -102,8 +102,7 @@ impl Error {
/// be returned in such a case.
pub fn from_errno(errno: crate::ffi::c_int) -> Error {
if errno < -(bindings::MAX_ERRNO as i32) || errno >= 0 {
- // TODO: Make it a `WARN_ONCE` once available.
- crate::pr_warn!(
+ crate::pr_warn_once!(
"attempted to create `Error` with out of range `errno`: {}",
errno
);
--
2.47.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v4 0/3] rust: Add pr_*_once macros
2024-11-26 16:40 ` [PATCH v4 0/3] rust: Add pr_*_once macros Jens Korinth via B4 Relay
` (2 preceding siblings ...)
2024-11-26 16:40 ` [PATCH v4 3/3] rust: error: Replace pr_warn by pr_warn_once Jens Korinth via B4 Relay
@ 2024-11-26 16:59 ` Miguel Ojeda
2025-02-11 15:04 ` Andreas Hindborg
4 siblings, 0 replies; 27+ messages in thread
From: Miguel Ojeda @ 2024-11-26 16:59 UTC (permalink / raw)
To: jens.korinth
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, rust-for-linux, FUJITA Tomonori, Dirk Behme,
linux-kernel
On Tue, Nov 26, 2024 at 5:41 PM Jens Korinth via B4 Relay
<devnull+jens.korinth.tuta.io@kernel.org> wrote:
>
> Co-developed-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> Co-developed-by: Boqun Feng <boqun.feng@gmail.com>
> Signed-off-by: Jens Korinth <jens.korinth@tuta.io>
I guess these tags are meant to be informative, but just in case, I
thought I would ask if they are here for some other reason, i.e. if
you somehow want them to end in the kernel log.
(It may be less confusing to simply mention in the letter who worked on what.)
Cheers,
Miguel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 3/3] rust: error: Replace pr_warn by pr_warn_once
2024-11-26 16:40 ` [PATCH v4 3/3] rust: error: Replace pr_warn by pr_warn_once Jens Korinth via B4 Relay
@ 2024-11-26 17:07 ` Miguel Ojeda
2024-11-27 20:39 ` jens.korinth
0 siblings, 1 reply; 27+ messages in thread
From: Miguel Ojeda @ 2024-11-26 17:07 UTC (permalink / raw)
To: jens.korinth
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, rust-for-linux, FUJITA Tomonori, Dirk Behme,
linux-kernel
On Tue, Nov 26, 2024 at 5:41 PM Jens Korinth via B4 Relay
<devnull+jens.korinth.tuta.io@kernel.org> wrote:
>
> Use new pr_warn_once macro to resolve TODO in error.rs.
Thanks for keeping the work on this!
I would mention here the merits between `pr_warn_once` vs. `WARN_ONCE`
and why the former was picked in this patch (especially since the
`TODO` suggests the latter).
Cheers,
Miguel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/3] rust: Add `OnceLite` for executing code once
2024-11-26 16:40 ` [PATCH v4 1/3] rust: Add `OnceLite` for executing code once Jens Korinth via B4 Relay
@ 2024-11-26 18:57 ` Daniel Sedlak
2024-11-27 19:46 ` jens.korinth
2024-11-26 19:00 ` Miguel Ojeda
` (2 subsequent siblings)
3 siblings, 1 reply; 27+ messages in thread
From: Daniel Sedlak @ 2024-11-26 18:57 UTC (permalink / raw)
To: jens.korinth, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross
Cc: rust-for-linux, FUJITA Tomonori, Dirk Behme, linux-kernel
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/3] rust: Add `OnceLite` for executing code once
2024-11-26 16:40 ` [PATCH v4 1/3] rust: Add `OnceLite` for executing code once Jens Korinth via B4 Relay
2024-11-26 18:57 ` Daniel Sedlak
@ 2024-11-26 19:00 ` Miguel Ojeda
2024-11-27 12:26 ` Alice Ryhl
2025-02-04 11:11 ` Andreas Hindborg
3 siblings, 0 replies; 27+ messages in thread
From: Miguel Ojeda @ 2024-11-26 19:00 UTC (permalink / raw)
To: jens.korinth
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, rust-for-linux, FUJITA Tomonori, Dirk Behme,
linux-kernel
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/3] rust: Add `OnceLite` for executing code once
2024-11-26 16:40 ` [PATCH v4 1/3] rust: Add `OnceLite` for executing code once Jens Korinth via B4 Relay
2024-11-26 18:57 ` Daniel Sedlak
2024-11-26 19:00 ` Miguel Ojeda
@ 2024-11-27 12:26 ` Alice Ryhl
2024-11-27 20:12 ` jens.korinth
2025-02-04 11:11 ` Andreas Hindborg
3 siblings, 1 reply; 27+ messages in thread
From: Alice Ryhl @ 2024-11-27 12:26 UTC (permalink / raw)
To: jens.korinth
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, rust-for-linux, FUJITA Tomonori, Dirk Behme,
linux-kernel
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?
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/3] rust: Add `OnceLite` for executing code once
2024-11-26 18:57 ` Daniel Sedlak
@ 2024-11-27 19:46 ` jens.korinth
2024-11-29 8:22 ` Daniel Sedlak
0 siblings, 1 reply; 27+ messages in thread
From: jens.korinth @ 2024-11-27 19:46 UTC (permalink / raw)
To: Daniel Sedlak
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Rust For Linux, FUJITA Tomonori, Dirk Behme,
Linux Kernel
> 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
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/3] rust: Add `OnceLite` for executing code once
2024-11-27 12:26 ` Alice Ryhl
@ 2024-11-27 20:12 ` jens.korinth
2024-11-27 20:18 ` Alice Ryhl
0 siblings, 1 reply; 27+ messages in thread
From: jens.korinth @ 2024-11-27 20:12 UTC (permalink / raw)
To: Alice Ryhl
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Rust For Linux, FUJITA Tomonori, Dirk Behme,
Linux Kernel
> 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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/3] rust: Add `OnceLite` for executing code once
2024-11-27 20:12 ` jens.korinth
@ 2024-11-27 20:18 ` Alice Ryhl
2024-12-05 6:49 ` jens.korinth
0 siblings, 1 reply; 27+ messages in thread
From: Alice Ryhl @ 2024-11-27 20:18 UTC (permalink / raw)
To: jens.korinth
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Rust For Linux, FUJITA Tomonori, Dirk Behme,
Linux Kernel
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 3/3] rust: error: Replace pr_warn by pr_warn_once
2024-11-26 17:07 ` Miguel Ojeda
@ 2024-11-27 20:39 ` jens.korinth
2024-11-30 18:18 ` Miguel Ojeda
0 siblings, 1 reply; 27+ messages in thread
From: jens.korinth @ 2024-11-27 20:39 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Rust For Linux, FUJITA Tomonori, Dirk Behme,
Linux Kernel
> I would mention here the merits between `pr_warn_once` vs. `WARN_ONCE`
> and why the former was picked in this patch (especially since the
> `TODO` suggests the latter).
Tbh, I am not 100% whether this should be here at all. The bug is not here, it's
at the call site. It should probably be a `try_from` instead, to raise the error
there?
Jens
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/3] rust: Add `OnceLite` for executing code once
2024-11-27 19:46 ` jens.korinth
@ 2024-11-29 8:22 ` Daniel Sedlak
0 siblings, 0 replies; 27+ messages in thread
From: Daniel Sedlak @ 2024-11-29 8:22 UTC (permalink / raw)
To: jens.korinth
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Rust For Linux, FUJITA Tomonori, Dirk Behme,
Linux Kernel
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 3/3] rust: error: Replace pr_warn by pr_warn_once
2024-11-27 20:39 ` jens.korinth
@ 2024-11-30 18:18 ` Miguel Ojeda
2024-12-05 6:30 ` jens.korinth
0 siblings, 1 reply; 27+ messages in thread
From: Miguel Ojeda @ 2024-11-30 18:18 UTC (permalink / raw)
To: jens.korinth
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Rust For Linux, FUJITA Tomonori, Dirk Behme,
Linux Kernel
On Wed, Nov 27, 2024 at 9:39 PM <jens.korinth@tuta.io> wrote:
>
> Tbh, I am not 100% whether this should be here at all. The bug is not here, it's
> at the call site. It should probably be a `try_from` instead, to raise the error
> there?
Do you mean removing the function altogether? i.e. migrating all
callers to `try_from_errno`?
Cheers,
Miguel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 3/3] rust: error: Replace pr_warn by pr_warn_once
2024-11-30 18:18 ` Miguel Ojeda
@ 2024-12-05 6:30 ` jens.korinth
2024-12-05 11:55 ` Miguel Ojeda
0 siblings, 1 reply; 27+ messages in thread
From: jens.korinth @ 2024-12-05 6:30 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Rust For Linux, FUJITA Tomonori, Dirk Behme,
Linux Kernel
Sorry for the late response, the usual madness at the end of the year is
setting in.
> Do you mean removing the function altogether? i.e. migrating all
> callers to `try_from_errno`?
I think it should be `TryFrom`. The `std::From` doc [1] says:
Note: This trait must not fail. The From trait is intended for perfect
conversions. If the conversion can fail or is not perfect, use TryFrom.
[1]: https://doc.rust-lang.org/std/convert/trait.From.html
Jens
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/3] rust: Add `OnceLite` for executing code once
2024-11-27 20:18 ` Alice Ryhl
@ 2024-12-05 6:49 ` jens.korinth
2024-12-05 8:47 ` Alice Ryhl
0 siblings, 1 reply; 27+ messages in thread
From: jens.korinth @ 2024-12-05 6:49 UTC (permalink / raw)
To: Alice Ryhl
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Rust For Linux, FUJITA Tomonori, Dirk Behme,
Linux Kernel
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/3] rust: Add `OnceLite` for executing code once
2024-12-05 6:49 ` jens.korinth
@ 2024-12-05 8:47 ` Alice Ryhl
0 siblings, 0 replies; 27+ messages in thread
From: Alice Ryhl @ 2024-12-05 8:47 UTC (permalink / raw)
To: jens.korinth
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Rust For Linux, FUJITA Tomonori, Dirk Behme,
Linux Kernel
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 3/3] rust: error: Replace pr_warn by pr_warn_once
2024-12-05 6:30 ` jens.korinth
@ 2024-12-05 11:55 ` Miguel Ojeda
2024-12-05 19:22 ` jens.korinth
0 siblings, 1 reply; 27+ messages in thread
From: Miguel Ojeda @ 2024-12-05 11:55 UTC (permalink / raw)
To: jens.korinth
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Rust For Linux, FUJITA Tomonori, Dirk Behme,
Linux Kernel
On Thu, Dec 5, 2024 at 7:30 AM <jens.korinth@tuta.io> wrote:
>
> Sorry for the late response, the usual madness at the end of the year is
> setting in.
No worries at all! I know the feeling... :)
> I think it should be `TryFrom`. The `std::From` doc [1] says:
>
> Note: This trait must not fail. The From trait is intended for perfect
> conversions. If the conversion can fail or is not perfect, use TryFrom.
>
> [1]: https://doc.rust-lang.org/std/convert/trait.From.html
Sorry, I am confused. This is not implementing the `From` trait, it is
an inherent implementation.
If what you mean is that this should not be an infallible operation,
then we are back at my previous reply, i.e. are you suggesting to
remove the method? Could you please clarify, perhaps with an example?
Or are you talking about something completely different?
Thanks!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 3/3] rust: error: Replace pr_warn by pr_warn_once
2024-12-05 11:55 ` Miguel Ojeda
@ 2024-12-05 19:22 ` jens.korinth
0 siblings, 0 replies; 27+ messages in thread
From: jens.korinth @ 2024-12-05 19:22 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Rust For Linux, FUJITA Tomonori, Dirk Behme,
Linux Kernel
> Sorry, I am confused. This is not implementing the `From` trait, it is
> an inherent implementation.
Hmm, you're right. I assume there is a reason why it doesn't implement
`From<c_int>` or `TryFrom<c_int>`?
> If what you mean is that this should not be an infallible operation,
> then we are back at my previous reply, i.e. are you suggesting to
> remove the method? Could you please clarify, perhaps with an example?
Yes, I meant to say it should not be infallible, because, well, it isn't. :)
I'd probably make `try_from_errno` public and remove `from_errno`.
If there's no other disadvantage I'd remove `try_from_errno` as well and
replace it by `TryFrom<c_int>`. Implementing `From<Error> for c_int` may
also make sense for the other direction?
Jens
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 1/3] rust: Add `OnceLite` for executing code once
2024-11-26 16:40 ` [PATCH v4 1/3] rust: Add `OnceLite` for executing code once Jens Korinth via B4 Relay
` (2 preceding siblings ...)
2024-11-27 12:26 ` Alice Ryhl
@ 2025-02-04 11:11 ` Andreas Hindborg
3 siblings, 0 replies; 27+ messages in thread
From: Andreas Hindborg @ 2025-02-04 11:11 UTC (permalink / raw)
To: Jens Korinth via B4 Relay
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
jens.korinth, rust-for-linux, FUJITA Tomonori, Dirk Behme,
linux-kernel
"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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 0/3] rust: Add pr_*_once macros
2024-11-26 16:40 ` [PATCH v4 0/3] rust: Add pr_*_once macros Jens Korinth via B4 Relay
` (3 preceding siblings ...)
2024-11-26 16:59 ` [PATCH v4 0/3] rust: Add pr_*_once macros Miguel Ojeda
@ 2025-02-11 15:04 ` Andreas Hindborg
2025-02-11 15:29 ` jens.korinth
4 siblings, 1 reply; 27+ messages in thread
From: Andreas Hindborg @ 2025-02-11 15:04 UTC (permalink / raw)
To: Jens Korinth via B4 Relay
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
jens.korinth, rust-for-linux, FUJITA Tomonori, Dirk Behme,
linux-kernel
Hi Jens,
"Jens Korinth via B4 Relay" <devnull+jens.korinth.tuta.io@kernel.org> writes:
> Add Rust version of pr_[emerg|alert|crit|err|warn|notic|info]_once
> functions, which print a message only once.
>
> Introduces a `OnceLite` abstraction similar to Rust's
> [`std::sync::Once`](https://doc.rust-lang.org/std/sync/struct.Once.html)
> but using the non-blocking mechanism from the kernel's `DO_ONCE_LITE`
> macro, which is used to define the `do_once_lite` Rust macro.
>
> First use case are an implementation of `pr_*_once` message macros to
> print a message only once.
>
Thanks for the patch! Do you plan on sending a new version? If not, do
you mind if I send v5?
Best regards,
Andreas Hindborg
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 0/3] rust: Add pr_*_once macros
2025-02-11 15:04 ` Andreas Hindborg
@ 2025-02-11 15:29 ` jens.korinth
2025-02-11 15:42 ` Andreas Hindborg
0 siblings, 1 reply; 27+ messages in thread
From: jens.korinth @ 2025-02-11 15:29 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Jens Korinth via B4 Relay, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Alice Ryhl,
Trevor Gross, Rust For Linux, FUJITA Tomonori, Dirk Behme,
Linux Kernel
Hi!
> Thanks for the patch! Do you plan on sending a new version? If not, do you mind if I send v5?
I think there is currently no consensus on how exactly it should be done (or at least I was confused about that). If you’re actively using the patch please go ahead! Active usage is always the best argument in such cases.
Jens
11 Feb 2025 at 16:05 by a.hindborg@kernel.org:
> Hi Jens,
>
> "Jens Korinth via B4 Relay" <devnull+jens.korinth.tuta.io@kernel.org> writes:
>
>> Add Rust version of pr_[emerg|alert|crit|err|warn|notic|info]_once
>> functions, which print a message only once.
>>
>> Introduces a `OnceLite` abstraction similar to Rust's
>> [`std::sync::Once`](https://doc.rust-lang.org/std/sync/struct.Once.html)
>> but using the non-blocking mechanism from the kernel's `DO_ONCE_LITE`
>> macro, which is used to define the `do_once_lite` Rust macro.
>>
>> First use case are an implementation of `pr_*_once` message macros to
>> print a message only once.
>>
>
> Thanks for the patch! Do you plan on sending a new version? If not, do
> you mind if I send v5?
>
>
> Best regards,
> Andreas Hindborg
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 0/3] rust: Add pr_*_once macros
2025-02-11 15:29 ` jens.korinth
@ 2025-02-11 15:42 ` Andreas Hindborg
2025-07-17 16:07 ` [PATCH v4 0/3] rust: Add pr_*_once macros134 Onur Özkan
0 siblings, 1 reply; 27+ messages in thread
From: Andreas Hindborg @ 2025-02-11 15:42 UTC (permalink / raw)
To: jens.korinth
Cc: Jens Korinth via B4 Relay, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Alice Ryhl,
Trevor Gross, Rust For Linux, FUJITA Tomonori, Dirk Behme,
Linux Kernel
<jens.korinth@tuta.io> writes:
> Hi!
>
>> Thanks for the patch! Do you plan on sending a new version? If not, do you mind if I send v5?
>
> I think there is currently no consensus on how exactly it should be done (or at least I was confused about that). If you’re actively using the patch please go ahead! Active usage is always the best argument in such cases.
I was informed this patch set is the correct way to emit a warning in
the module_params patch set [1].
I did not follow all the discussion so I am not sure either. But I'll
look into the discussion then.
Best regards,
Andreas Hindborg
[1] https://lore.kernel.org/rust-for-linux/20250204-module-params-v3-v5-0-bf5ec2041625@kernel.org/
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 0/3] rust: Add pr_*_once macros134
2025-02-11 15:42 ` Andreas Hindborg
@ 2025-07-17 16:07 ` Onur Özkan
2025-07-19 3:33 ` Boqun Feng
0 siblings, 1 reply; 27+ messages in thread
From: Onur Özkan @ 2025-07-17 16:07 UTC (permalink / raw)
To: Andreas Hindborg
Cc: jens.korinth, Jens Korinth via B4 Relay, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Alice Ryhl, Trevor Gross, Rust For Linux,
FUJITA Tomonori, Dirk Behme, Linux Kernel
On Tue, 11 Feb 2025 16:42:25 +0100
Andreas Hindborg <a.hindborg@kernel.org> wrote:
> <jens.korinth@tuta.io> writes:
>
> > Hi!
> >
> >> Thanks for the patch! Do you plan on sending a new version? If
> >>not, do you mind if I send v5?
> >
> > I think there is currently no consensus on how exactly it should be
> > done (or at least I was confused about that). If you’re actively
> > using the patch please go ahead! Active usage is always the best
> > argument in such cases.
>
> I was informed this patch set is the correct way to emit a warning in
> the module_params patch set [1].
>
> I did not follow all the discussion so I am not sure either. But I'll
> look into the discussion then.
>
>
> Best regards,
> Andreas Hindborg
>
>
>
> [1]
> https://lore.kernel.org/rust-for-linux/20250204-module-params-v3-v5-0-bf5ec2041625@kernel.org/
>
>
I have reviewed the patch series from start to finish. I am not
using or depending the patch actively but I can send v5 sometime
soon (I will have some space next week) if you would like.
Regards,
Onur
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 0/3] rust: Add pr_*_once macros134
2025-07-17 16:07 ` [PATCH v4 0/3] rust: Add pr_*_once macros134 Onur Özkan
@ 2025-07-19 3:33 ` Boqun Feng
2025-07-19 16:33 ` Onur
0 siblings, 1 reply; 27+ messages in thread
From: Boqun Feng @ 2025-07-19 3:33 UTC (permalink / raw)
To: Onur Özkan
Cc: Andreas Hindborg, jens.korinth, Jens Korinth via B4 Relay,
Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
Benno Lossin, Alice Ryhl, Trevor Gross, Rust For Linux,
FUJITA Tomonori, Dirk Behme, Linux Kernel
On Thu, Jul 17, 2025 at 07:07:13PM +0300, Onur Özkan wrote:
> On Tue, 11 Feb 2025 16:42:25 +0100
> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> > <jens.korinth@tuta.io> writes:
> >
> > > Hi!
> > >
> > >> Thanks for the patch! Do you plan on sending a new version? If
> > >>not, do you mind if I send v5?
> > >
> > > I think there is currently no consensus on how exactly it should be
> > > done (or at least I was confused about that). If you´re actively
> > > using the patch please go ahead! Active usage is always the best
> > > argument in such cases.
> >
> > I was informed this patch set is the correct way to emit a warning in
> > the module_params patch set [1].
> >
> > I did not follow all the discussion so I am not sure either. But I'll
> > look into the discussion then.
> >
> >
> > Best regards,
> > Andreas Hindborg
> >
> >
> >
> > [1]
> > https://lore.kernel.org/rust-for-linux/20250204-module-params-v3-v5-0-bf5ec2041625@kernel.org/
> >
> >
>
> I have reviewed the patch series from start to finish. I am not
> using or depending the patch actively but I can send v5 sometime
> soon (I will have some space next week) if you would like.
>
Note that you need to use LKMM atomics [1], and you should really use
a 32bit atomic at the beginning. Thanks!
There are a few explanations on why we want to avoid use Rust native
atomics:
https://lwn.net/Articles/993785/
https://lore.kernel.org/rust-for-linux/CAHk-=whkQk=zq5XiMcaU3xj4v69+jyoP-y6Sywhq-TvxSSvfEA@mail.gmail.com/
[1]: https://lore.kernel.org/rust-for-linux/20250719030827.61357-1-boqun.feng@gmail.com/
Regards,
Boqun
> Regards,
> Onur
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v4 0/3] rust: Add pr_*_once macros134
2025-07-19 3:33 ` Boqun Feng
@ 2025-07-19 16:33 ` Onur
0 siblings, 0 replies; 27+ messages in thread
From: Onur @ 2025-07-19 16:33 UTC (permalink / raw)
To: Boqun Feng
Cc: Andreas Hindborg, jens.korinth, Jens Korinth via B4 Relay,
Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
Benno Lossin, Alice Ryhl, Trevor Gross, Rust For Linux,
FUJITA Tomonori, Dirk Behme, Linux Kernel
On Fri, 18 Jul 2025 20:33:21 -0700
Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > I have reviewed the patch series from start to finish. I am not
> > using or depending the patch actively but I can send v5 sometime
> > soon (I will have some space next week) if you would like.
> >
>
> Note that you need to use LKMM atomics [1], and you should really use
> a 32bit atomic at the beginning. Thanks!
>
> There are a few explanations on why we want to avoid use Rust native
> atomics:
>
> https://lwn.net/Articles/993785/
> https://lore.kernel.org/rust-for-linux/CAHk-=whkQk=zq5XiMcaU3xj4v69+jyoP-y6Sywhq-TvxSSvfEA@mail.gmail.com/
>
These are excellent sources for digging into the details.
Thanks a lot! :)
Regards,
Onur
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2025-07-19 16:41 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <8gSHb0iuAT_Wz0rR1-qsnFIVndSpW229fA0lq-fslngTt5k1ooiMw5eAIc82F26Mdma4nkyU4X7_1RLZcnQf-Q==@protonmail.internalid>
2024-11-26 16:40 ` [PATCH v4 0/3] rust: Add pr_*_once macros Jens Korinth via B4 Relay
2024-11-26 16:40 ` [PATCH v4 1/3] rust: Add `OnceLite` for executing code once Jens Korinth via B4 Relay
2024-11-26 18:57 ` Daniel Sedlak
2024-11-27 19:46 ` jens.korinth
2024-11-29 8:22 ` Daniel Sedlak
2024-11-26 19:00 ` Miguel Ojeda
2024-11-27 12:26 ` Alice Ryhl
2024-11-27 20:12 ` jens.korinth
2024-11-27 20:18 ` Alice Ryhl
2024-12-05 6:49 ` jens.korinth
2024-12-05 8:47 ` Alice Ryhl
2025-02-04 11:11 ` Andreas Hindborg
2024-11-26 16:40 ` [PATCH v4 2/3] rust: print: Add pr_*_once macros Jens Korinth via B4 Relay
2024-11-26 16:40 ` [PATCH v4 3/3] rust: error: Replace pr_warn by pr_warn_once Jens Korinth via B4 Relay
2024-11-26 17:07 ` Miguel Ojeda
2024-11-27 20:39 ` jens.korinth
2024-11-30 18:18 ` Miguel Ojeda
2024-12-05 6:30 ` jens.korinth
2024-12-05 11:55 ` Miguel Ojeda
2024-12-05 19:22 ` jens.korinth
2024-11-26 16:59 ` [PATCH v4 0/3] rust: Add pr_*_once macros Miguel Ojeda
2025-02-11 15:04 ` Andreas Hindborg
2025-02-11 15:29 ` jens.korinth
2025-02-11 15:42 ` Andreas Hindborg
2025-07-17 16:07 ` [PATCH v4 0/3] rust: Add pr_*_once macros134 Onur Özkan
2025-07-19 3:33 ` Boqun Feng
2025-07-19 16:33 ` Onur
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).