* [PATCH v15 1/7] rust: sync: add `SetOnce`
2025-07-07 13:29 [PATCH v15 0/7] rust: extend `module!` macro with integer parameter support Andreas Hindborg
@ 2025-07-07 13:29 ` Andreas Hindborg
2025-07-07 13:35 ` Andreas Hindborg
` (2 more replies)
2025-07-07 13:29 ` [PATCH v15 2/7] rust: str: add radix prefixed integer parsing functions Andreas Hindborg
` (6 subsequent siblings)
7 siblings, 3 replies; 28+ messages in thread
From: Andreas Hindborg @ 2025-07-07 13:29 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Benno Lossin, Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules, Andreas Hindborg
Introduce the `SetOnce` type, a container that can only be written once.
The container uses an internal atomic to synchronize writes to the internal
value.
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/sync.rs | 2 +
rust/kernel/sync/set_once.rs | 125 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 127 insertions(+)
diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index 81e3a806e57e2..13e6bc7fa87ac 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -18,6 +18,7 @@
mod locked_by;
pub mod poll;
pub mod rcu;
+mod set_once;
pub use arc::{Arc, ArcBorrow, UniqueArc};
pub use completion::Completion;
@@ -26,6 +27,7 @@
pub use lock::mutex::{new_mutex, Mutex, MutexGuard};
pub use lock::spinlock::{new_spinlock, SpinLock, SpinLockGuard};
pub use locked_by::LockedBy;
+pub use set_once::SetOnce;
/// Represents a lockdep class. It's a wrapper around C's `lock_class_key`.
#[repr(transparent)]
diff --git a/rust/kernel/sync/set_once.rs b/rust/kernel/sync/set_once.rs
new file mode 100644
index 0000000000000..e1e31f5faed09
--- /dev/null
+++ b/rust/kernel/sync/set_once.rs
@@ -0,0 +1,125 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! A container that can be initialized at most once.
+
+use super::atomic::ordering::Acquire;
+use super::atomic::ordering::Relaxed;
+use super::atomic::ordering::Release;
+use super::atomic::Atomic;
+use core::ptr::drop_in_place;
+use kernel::types::Opaque;
+
+/// A container that can be populated at most once. Thread safe.
+///
+/// Once the a [`SetOnce`] is populated, it remains populated by the same object for the
+/// lifetime `Self`.
+///
+/// # Invariants
+///
+/// - `init` may only increase in value.
+/// - `init` may only assume values in the range `0..=2`.
+/// - `init == 0` if and only if the container is empty.
+/// - `init == 1` if and only if being initialized.
+/// - `init == 2` if and only if the container is populated and valid for shared access.
+///
+/// # Example
+///
+/// ```
+/// # use kernel::sync::SetOnce;
+/// let value = SetOnce::new();
+/// assert_eq!(None, value.as_ref());
+///
+/// let status = value.populate(42u8);
+/// assert_eq!(true, status);
+/// assert_eq!(Some(&42u8), value.as_ref());
+/// assert_eq!(Some(42u8), value.copy());
+///
+/// let status = value.populate(101u8);
+/// assert_eq!(false, status);
+/// assert_eq!(Some(&42u8), value.as_ref());
+/// assert_eq!(Some(42u8), value.copy());
+/// ```
+pub struct SetOnce<T> {
+ init: Atomic<u32>,
+ value: Opaque<T>,
+}
+
+impl<T> Default for SetOnce<T> {
+ fn default() -> Self {
+ Self::new()
+ }
+}
+
+// TODO: change names
+
+impl<T> SetOnce<T> {
+ /// Create a new [`SetOnce`].
+ ///
+ /// The returned instance will be empty.
+ pub const fn new() -> Self {
+ // INVARIANT: The container is empty and we initialize `init` to `0`.
+ Self {
+ value: Opaque::uninit(),
+ init: Atomic::new(0),
+ }
+ }
+
+ /// Get a reference to the contained object.
+ ///
+ /// Returns [`None`] if this [`SetOnce`] is empty.
+ pub fn as_ref(&self) -> Option<&T> {
+ if self.init.load(Acquire) == 2 {
+ // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
+ // contains a valid value.
+ Some(unsafe { &*self.value.get() })
+ } else {
+ None
+ }
+ }
+
+ /// Populate the [`SetOnce`].
+ ///
+ /// Returns `true` if the [`SetOnce`] was successfully populated.
+ pub fn populate(&self, value: T) -> bool {
+ // INVARIANT: If the swap succeeds:
+ // - We increase `init`.
+ // - We write the valid value `1` to `init`.
+ // - Only one thread can succeed in this write, so we have exclusive access after the
+ // write.
+ if let Ok(0) = self.init.cmpxchg(0, 1, Relaxed) {
+ // SAFETY: By the type invariants of `Self`, the fact that we succeeded in writing `1`
+ // to `self.init` means we obtained exclusive access to the contained object.
+ unsafe { core::ptr::write(self.value.get(), value) };
+ // INVARIANT:
+ // - We increase `init`.
+ // - We write the valid value `2` to `init`.
+ // - We release our exclusive access to the contained object and the object is now
+ // valid for shared access.
+ self.init.store(2, Release);
+ true
+ } else {
+ false
+ }
+ }
+
+ /// Get a copy of the contained object.
+ ///
+ /// Returns [`None`] if the [`SetOnce`] is empty.
+ pub fn copy(&self) -> Option<T>
+ where
+ T: Copy,
+ {
+ self.as_ref().copied()
+ }
+}
+
+impl<T> Drop for SetOnce<T> {
+ fn drop(&mut self) {
+ if self.init.load(Acquire) == 2 {
+ // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
+ // contains a valid value. We have exclusive access, as we hold a `mut` reference to
+ // `self`.
+ unsafe { drop_in_place(self.value.get()) };
+ }
+ }
+}
--
2.47.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
2025-07-07 13:29 ` [PATCH v15 1/7] rust: sync: add `SetOnce` Andreas Hindborg
@ 2025-07-07 13:35 ` Andreas Hindborg
2025-07-07 13:38 ` Alice Ryhl
2025-07-08 9:02 ` Benno Lossin
2 siblings, 0 replies; 28+ messages in thread
From: Andreas Hindborg @ 2025-07-07 13:35 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Alice Ryhl, Masahiro Yamada, Nathan Chancellor, Luis Chamberlain,
Danilo Krummrich, Benno Lossin, Nicolas Schier, Trevor Gross,
Adam Bratschi-Kaye, rust-for-linux, linux-kernel, linux-kbuild,
Petr Pavlu, Sami Tolvanen, Daniel Gomez, Simona Vetter, Greg KH,
Fiona Behrens, Daniel Almeida, linux-modules
Andreas Hindborg <a.hindborg@kernel.org> writes:
> Introduce the `SetOnce` type, a container that can only be written once.
> The container uses an internal atomic to synchronize writes to the internal
> value.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> rust/kernel/sync.rs | 2 +
> rust/kernel/sync/set_once.rs | 125 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 127 insertions(+)
>
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index 81e3a806e57e2..13e6bc7fa87ac 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -18,6 +18,7 @@
> mod locked_by;
> pub mod poll;
> pub mod rcu;
> +mod set_once;
>
> pub use arc::{Arc, ArcBorrow, UniqueArc};
> pub use completion::Completion;
> @@ -26,6 +27,7 @@
> pub use lock::mutex::{new_mutex, Mutex, MutexGuard};
> pub use lock::spinlock::{new_spinlock, SpinLock, SpinLockGuard};
> pub use locked_by::LockedBy;
> +pub use set_once::SetOnce;
>
> /// Represents a lockdep class. It's a wrapper around C's `lock_class_key`.
> #[repr(transparent)]
> diff --git a/rust/kernel/sync/set_once.rs b/rust/kernel/sync/set_once.rs
> new file mode 100644
> index 0000000000000..e1e31f5faed09
> --- /dev/null
> +++ b/rust/kernel/sync/set_once.rs
> @@ -0,0 +1,125 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! A container that can be initialized at most once.
> +
> +use super::atomic::ordering::Acquire;
> +use super::atomic::ordering::Relaxed;
> +use super::atomic::ordering::Release;
> +use super::atomic::Atomic;
> +use core::ptr::drop_in_place;
> +use kernel::types::Opaque;
> +
> +/// A container that can be populated at most once. Thread safe.
> +///
> +/// Once the a [`SetOnce`] is populated, it remains populated by the same object for the
> +/// lifetime `Self`.
> +///
> +/// # Invariants
> +///
> +/// - `init` may only increase in value.
> +/// - `init` may only assume values in the range `0..=2`.
> +/// - `init == 0` if and only if the container is empty.
> +/// - `init == 1` if and only if being initialized.
> +/// - `init == 2` if and only if the container is populated and valid for shared access.
> +///
> +/// # Example
> +///
> +/// ```
> +/// # use kernel::sync::SetOnce;
> +/// let value = SetOnce::new();
> +/// assert_eq!(None, value.as_ref());
> +///
> +/// let status = value.populate(42u8);
> +/// assert_eq!(true, status);
> +/// assert_eq!(Some(&42u8), value.as_ref());
> +/// assert_eq!(Some(42u8), value.copy());
> +///
> +/// let status = value.populate(101u8);
> +/// assert_eq!(false, status);
> +/// assert_eq!(Some(&42u8), value.as_ref());
> +/// assert_eq!(Some(42u8), value.copy());
> +/// ```
> +pub struct SetOnce<T> {
> + init: Atomic<u32>,
> + value: Opaque<T>,
> +}
> +
> +impl<T> Default for SetOnce<T> {
> + fn default() -> Self {
> + Self::new()
> + }
> +}
> +
> +// TODO: change names
I just saw that this line decided to stick around, that was obviously
not intentional. Just disregard this line.
Best regards,
Andreas Hindborg
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
2025-07-07 13:29 ` [PATCH v15 1/7] rust: sync: add `SetOnce` Andreas Hindborg
2025-07-07 13:35 ` Andreas Hindborg
@ 2025-07-07 13:38 ` Alice Ryhl
2025-07-07 15:13 ` Boqun Feng
2025-07-08 8:47 ` Andreas Hindborg
2025-07-08 9:02 ` Benno Lossin
2 siblings, 2 replies; 28+ messages in thread
From: Alice Ryhl @ 2025-07-07 13:38 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Masahiro Yamada, Nathan Chancellor,
Luis Chamberlain, Danilo Krummrich, Benno Lossin, Nicolas Schier,
Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules
On Mon, Jul 7, 2025 at 3:32 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Introduce the `SetOnce` type, a container that can only be written once.
> The container uses an internal atomic to synchronize writes to the internal
> value.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
LGTM:
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> +impl<T> Drop for SetOnce<T> {
> + fn drop(&mut self) {
> + if self.init.load(Acquire) == 2 {
> + // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
> + // contains a valid value. We have exclusive access, as we hold a `mut` reference to
> + // `self`.
> + unsafe { drop_in_place(self.value.get()) };
This load does not need to be Acquire. It can be a Relaxed load or
even an unsynchronized one since the access is exclusive.
Alice
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
2025-07-07 13:38 ` Alice Ryhl
@ 2025-07-07 15:13 ` Boqun Feng
2025-07-08 8:54 ` Andreas Hindborg
2025-07-08 8:47 ` Andreas Hindborg
1 sibling, 1 reply; 28+ messages in thread
From: Boqun Feng @ 2025-07-07 15:13 UTC (permalink / raw)
To: Alice Ryhl
Cc: Andreas Hindborg, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Masahiro Yamada, Nathan Chancellor,
Luis Chamberlain, Danilo Krummrich, Benno Lossin, Nicolas Schier,
Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules
On Mon, Jul 07, 2025 at 03:38:58PM +0200, Alice Ryhl wrote:
> On Mon, Jul 7, 2025 at 3:32 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
> >
> > Introduce the `SetOnce` type, a container that can only be written once.
> > The container uses an internal atomic to synchronize writes to the internal
> > value.
> >
> > Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> LGTM:
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>
> > +impl<T> Drop for SetOnce<T> {
> > + fn drop(&mut self) {
> > + if self.init.load(Acquire) == 2 {
> > + // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
> > + // contains a valid value. We have exclusive access, as we hold a `mut` reference to
> > + // `self`.
> > + unsafe { drop_in_place(self.value.get()) };
>
> This load does not need to be Acquire. It can be a Relaxed load or
> even an unsynchronized one since the access is exclusive.
Right, I think we can do the similar as Revocable here:
if *self.init.get_mut() == 2 { }
Further, with my following Benno's suggestion and making `Atomic<T>` an
`UnsafeCell<T>:
https://lore.kernel.org/rust-for-linux/aGhh-TvNOWhkt0JG@Mac.home/
compiler can generate a noalias reference here, which allows further
optimization.
Regards,
Boqun
>
> Alice
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
2025-07-07 15:13 ` Boqun Feng
@ 2025-07-08 8:54 ` Andreas Hindborg
2025-07-08 9:07 ` Benno Lossin
0 siblings, 1 reply; 28+ messages in thread
From: Andreas Hindborg @ 2025-07-08 8:54 UTC (permalink / raw)
To: Boqun Feng
Cc: Alice Ryhl, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Masahiro Yamada, Nathan Chancellor,
Luis Chamberlain, Danilo Krummrich, Benno Lossin, Nicolas Schier,
Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules
"Boqun Feng" <boqun.feng@gmail.com> writes:
> On Mon, Jul 07, 2025 at 03:38:58PM +0200, Alice Ryhl wrote:
>> On Mon, Jul 7, 2025 at 3:32 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>> >
>> > Introduce the `SetOnce` type, a container that can only be written once.
>> > The container uses an internal atomic to synchronize writes to the internal
>> > value.
>> >
>> > Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>>
>> LGTM:
>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>>
>> > +impl<T> Drop for SetOnce<T> {
>> > + fn drop(&mut self) {
>> > + if self.init.load(Acquire) == 2 {
>> > + // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
>> > + // contains a valid value. We have exclusive access, as we hold a `mut` reference to
>> > + // `self`.
>> > + unsafe { drop_in_place(self.value.get()) };
>>
>> This load does not need to be Acquire. It can be a Relaxed load or
>> even an unsynchronized one since the access is exclusive.
>
> Right, I think we can do the similar as Revocable here:
>
> if *self.init.get_mut() == 2 { }
>
> Further, with my following Benno's suggestion and making `Atomic<T>` an
> `UnsafeCell<T>:
>
> https://lore.kernel.org/rust-for-linux/aGhh-TvNOWhkt0JG@Mac.home/
>
> compiler can generate a noalias reference here, which allows further
> optimization.
>
You would like to remove `PhantomPinned` to enable noalias? I guess that
makes sense in this case. I'll fix that for next spin.
Best regards,
Andreas Hindborg
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
2025-07-08 8:54 ` Andreas Hindborg
@ 2025-07-08 9:07 ` Benno Lossin
2025-07-09 10:34 ` Andreas Hindborg
0 siblings, 1 reply; 28+ messages in thread
From: Benno Lossin @ 2025-07-08 9:07 UTC (permalink / raw)
To: Andreas Hindborg, Boqun Feng
Cc: Alice Ryhl, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Masahiro Yamada, Nathan Chancellor,
Luis Chamberlain, Danilo Krummrich, Nicolas Schier, Trevor Gross,
Adam Bratschi-Kaye, rust-for-linux, linux-kernel, linux-kbuild,
Petr Pavlu, Sami Tolvanen, Daniel Gomez, Simona Vetter, Greg KH,
Fiona Behrens, Daniel Almeida, linux-modules
On Tue Jul 8, 2025 at 10:54 AM CEST, Andreas Hindborg wrote:
> "Boqun Feng" <boqun.feng@gmail.com> writes:
>
>> On Mon, Jul 07, 2025 at 03:38:58PM +0200, Alice Ryhl wrote:
>>> On Mon, Jul 7, 2025 at 3:32 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>> >
>>> > Introduce the `SetOnce` type, a container that can only be written once.
>>> > The container uses an internal atomic to synchronize writes to the internal
>>> > value.
>>> >
>>> > Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>>>
>>> LGTM:
>>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>>>
>>> > +impl<T> Drop for SetOnce<T> {
>>> > + fn drop(&mut self) {
>>> > + if self.init.load(Acquire) == 2 {
>>> > + // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
>>> > + // contains a valid value. We have exclusive access, as we hold a `mut` reference to
>>> > + // `self`.
>>> > + unsafe { drop_in_place(self.value.get()) };
>>>
>>> This load does not need to be Acquire. It can be a Relaxed load or
>>> even an unsynchronized one since the access is exclusive.
>>
>> Right, I think we can do the similar as Revocable here:
>>
>> if *self.init.get_mut() == 2 { }
>>
>> Further, with my following Benno's suggestion and making `Atomic<T>` an
>> `UnsafeCell<T>:
>>
>> https://lore.kernel.org/rust-for-linux/aGhh-TvNOWhkt0JG@Mac.home/
>>
>> compiler can generate a noalias reference here, which allows further
>> optimization.
>>
>
> You would like to remove `PhantomPinned` to enable noalias? I guess that
> makes sense in this case. I'll fix that for next spin.
I think you two are talking about different things. Boqun is saying that
the `Atomic<T>` will use `UnsafeCell` rather than `Opaque`, which will
potentially allow more optimizations.
But you are talking about `SetOnce`, right? I think it makes more sense
for `SetOnce` to use `UnsafeCell<MaybeUninit<T>>` rather than `Opaque`
too. So feel free to change it in the next version.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
2025-07-08 9:07 ` Benno Lossin
@ 2025-07-09 10:34 ` Andreas Hindborg
2025-07-09 18:22 ` Benno Lossin
0 siblings, 1 reply; 28+ messages in thread
From: Andreas Hindborg @ 2025-07-09 10:34 UTC (permalink / raw)
To: Benno Lossin
Cc: Boqun Feng, Alice Ryhl, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Masahiro Yamada, Nathan Chancellor,
Luis Chamberlain, Danilo Krummrich, Nicolas Schier, Trevor Gross,
Adam Bratschi-Kaye, rust-for-linux, linux-kernel, linux-kbuild,
Petr Pavlu, Sami Tolvanen, Daniel Gomez, Simona Vetter, Greg KH,
Fiona Behrens, Daniel Almeida, linux-modules
"Benno Lossin" <lossin@kernel.org> writes:
> On Tue Jul 8, 2025 at 10:54 AM CEST, Andreas Hindborg wrote:
>> "Boqun Feng" <boqun.feng@gmail.com> writes:
>>
>>> On Mon, Jul 07, 2025 at 03:38:58PM +0200, Alice Ryhl wrote:
>>>> On Mon, Jul 7, 2025 at 3:32 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>>> >
>>>> > Introduce the `SetOnce` type, a container that can only be written once.
>>>> > The container uses an internal atomic to synchronize writes to the internal
>>>> > value.
>>>> >
>>>> > Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>>>>
>>>> LGTM:
>>>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>>>>
>>>> > +impl<T> Drop for SetOnce<T> {
>>>> > + fn drop(&mut self) {
>>>> > + if self.init.load(Acquire) == 2 {
>>>> > + // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
>>>> > + // contains a valid value. We have exclusive access, as we hold a `mut` reference to
>>>> > + // `self`.
>>>> > + unsafe { drop_in_place(self.value.get()) };
>>>>
>>>> This load does not need to be Acquire. It can be a Relaxed load or
>>>> even an unsynchronized one since the access is exclusive.
>>>
>>> Right, I think we can do the similar as Revocable here:
>>>
>>> if *self.init.get_mut() == 2 { }
Ok, now I got it. You are saying I don't need to use the atomic load
method, because I have mutable access. Sounds good.
But I guess a relaxed load and access through a mutable reference should
result in the same code generation on most (all?) platforms?
>>>
>>> Further, with my following Benno's suggestion and making `Atomic<T>` an
>>> `UnsafeCell<T>:
>>>
>>> https://lore.kernel.org/rust-for-linux/aGhh-TvNOWhkt0JG@Mac.home/
>>>
>>> compiler can generate a noalias reference here, which allows further
>>> optimization.
>>>
>>
>> You would like to remove `PhantomPinned` to enable noalias? I guess that
>> makes sense in this case. I'll fix that for next spin.
>
> I think you two are talking about different things. Boqun is saying that
> the `Atomic<T>` will use `UnsafeCell` rather than `Opaque`, which will
> potentially allow more optimizations.
>
> But you are talking about `SetOnce`, right? I think it makes more sense
> for `SetOnce` to use `UnsafeCell<MaybeUninit<T>>` rather than `Opaque`
> too. So feel free to change it in the next version.
Exactly. We don't need `UnsafePinned` mechanics here.
Best regards,
Andreas Hindborg
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
2025-07-09 10:34 ` Andreas Hindborg
@ 2025-07-09 18:22 ` Benno Lossin
2025-07-09 20:12 ` Boqun Feng
0 siblings, 1 reply; 28+ messages in thread
From: Benno Lossin @ 2025-07-09 18:22 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Boqun Feng, Alice Ryhl, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Masahiro Yamada, Nathan Chancellor,
Luis Chamberlain, Danilo Krummrich, Nicolas Schier, Trevor Gross,
Adam Bratschi-Kaye, rust-for-linux, linux-kernel, linux-kbuild,
Petr Pavlu, Sami Tolvanen, Daniel Gomez, Simona Vetter, Greg KH,
Fiona Behrens, Daniel Almeida, linux-modules
On Wed Jul 9, 2025 at 12:34 PM CEST, Andreas Hindborg wrote:
> "Benno Lossin" <lossin@kernel.org> writes:
>> On Tue Jul 8, 2025 at 10:54 AM CEST, Andreas Hindborg wrote:
>>> "Boqun Feng" <boqun.feng@gmail.com> writes:
>>>> On Mon, Jul 07, 2025 at 03:38:58PM +0200, Alice Ryhl wrote:
>>>>> On Mon, Jul 7, 2025 at 3:32 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>>>> >
>>>>> > Introduce the `SetOnce` type, a container that can only be written once.
>>>>> > The container uses an internal atomic to synchronize writes to the internal
>>>>> > value.
>>>>> >
>>>>> > Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>>>>>
>>>>> LGTM:
>>>>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>>>>>
>>>>> > +impl<T> Drop for SetOnce<T> {
>>>>> > + fn drop(&mut self) {
>>>>> > + if self.init.load(Acquire) == 2 {
>>>>> > + // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
>>>>> > + // contains a valid value. We have exclusive access, as we hold a `mut` reference to
>>>>> > + // `self`.
>>>>> > + unsafe { drop_in_place(self.value.get()) };
>>>>>
>>>>> This load does not need to be Acquire. It can be a Relaxed load or
>>>>> even an unsynchronized one since the access is exclusive.
>>>>
>>>> Right, I think we can do the similar as Revocable here:
>>>>
>>>> if *self.init.get_mut() == 2 { }
>
> Ok, now I got it. You are saying I don't need to use the atomic load
> method, because I have mutable access. Sounds good.
>
> But I guess a relaxed load and access through a mutable reference should
> result in the same code generation on most (all?) platforms?
AFAIK it is not the same on arm.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
2025-07-09 18:22 ` Benno Lossin
@ 2025-07-09 20:12 ` Boqun Feng
2025-07-09 20:22 ` Benno Lossin
0 siblings, 1 reply; 28+ messages in thread
From: Boqun Feng @ 2025-07-09 20:12 UTC (permalink / raw)
To: Benno Lossin
Cc: Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Masahiro Yamada, Nathan Chancellor,
Luis Chamberlain, Danilo Krummrich, Nicolas Schier, Trevor Gross,
Adam Bratschi-Kaye, rust-for-linux, linux-kernel, linux-kbuild,
Petr Pavlu, Sami Tolvanen, Daniel Gomez, Simona Vetter, Greg KH,
Fiona Behrens, Daniel Almeida, linux-modules
On Wed, Jul 09, 2025 at 08:22:16PM +0200, Benno Lossin wrote:
> On Wed Jul 9, 2025 at 12:34 PM CEST, Andreas Hindborg wrote:
> > "Benno Lossin" <lossin@kernel.org> writes:
> >> On Tue Jul 8, 2025 at 10:54 AM CEST, Andreas Hindborg wrote:
> >>> "Boqun Feng" <boqun.feng@gmail.com> writes:
> >>>> On Mon, Jul 07, 2025 at 03:38:58PM +0200, Alice Ryhl wrote:
> >>>>> On Mon, Jul 7, 2025 at 3:32 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
> >>>>> >
> >>>>> > Introduce the `SetOnce` type, a container that can only be written once.
> >>>>> > The container uses an internal atomic to synchronize writes to the internal
> >>>>> > value.
> >>>>> >
> >>>>> > Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> >>>>>
> >>>>> LGTM:
> >>>>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> >>>>>
> >>>>> > +impl<T> Drop for SetOnce<T> {
> >>>>> > + fn drop(&mut self) {
> >>>>> > + if self.init.load(Acquire) == 2 {
> >>>>> > + // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
> >>>>> > + // contains a valid value. We have exclusive access, as we hold a `mut` reference to
> >>>>> > + // `self`.
> >>>>> > + unsafe { drop_in_place(self.value.get()) };
> >>>>>
> >>>>> This load does not need to be Acquire. It can be a Relaxed load or
> >>>>> even an unsynchronized one since the access is exclusive.
> >>>>
> >>>> Right, I think we can do the similar as Revocable here:
> >>>>
> >>>> if *self.init.get_mut() == 2 { }
> >
> > Ok, now I got it. You are saying I don't need to use the atomic load
> > method, because I have mutable access. Sounds good.
> >
> > But I guess a relaxed load and access through a mutable reference should
> > result in the same code generation on most (all?) platforms?
>
> AFAIK it is not the same on arm.
>
Right, when LTO=y, arm64 use acquire load to implement
READ_ONCE()/atomic_read().
Regards,
Boqun
> ---
> Cheers,
> Benno
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
2025-07-09 20:12 ` Boqun Feng
@ 2025-07-09 20:22 ` Benno Lossin
2025-07-09 21:05 ` Boqun Feng
0 siblings, 1 reply; 28+ messages in thread
From: Benno Lossin @ 2025-07-09 20:22 UTC (permalink / raw)
To: Boqun Feng
Cc: Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Masahiro Yamada, Nathan Chancellor,
Luis Chamberlain, Danilo Krummrich, Nicolas Schier, Trevor Gross,
Adam Bratschi-Kaye, rust-for-linux, linux-kernel, linux-kbuild,
Petr Pavlu, Sami Tolvanen, Daniel Gomez, Simona Vetter, Greg KH,
Fiona Behrens, Daniel Almeida, linux-modules
On Wed Jul 9, 2025 at 10:12 PM CEST, Boqun Feng wrote:
> On Wed, Jul 09, 2025 at 08:22:16PM +0200, Benno Lossin wrote:
>> On Wed Jul 9, 2025 at 12:34 PM CEST, Andreas Hindborg wrote:
>> > "Benno Lossin" <lossin@kernel.org> writes:
>> >> On Tue Jul 8, 2025 at 10:54 AM CEST, Andreas Hindborg wrote:
>> >>> "Boqun Feng" <boqun.feng@gmail.com> writes:
>> >>>> On Mon, Jul 07, 2025 at 03:38:58PM +0200, Alice Ryhl wrote:
>> >>>>> On Mon, Jul 7, 2025 at 3:32 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>> >>>>> >
>> >>>>> > Introduce the `SetOnce` type, a container that can only be written once.
>> >>>>> > The container uses an internal atomic to synchronize writes to the internal
>> >>>>> > value.
>> >>>>> >
>> >>>>> > Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> >>>>>
>> >>>>> LGTM:
>> >>>>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>> >>>>>
>> >>>>> > +impl<T> Drop for SetOnce<T> {
>> >>>>> > + fn drop(&mut self) {
>> >>>>> > + if self.init.load(Acquire) == 2 {
>> >>>>> > + // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
>> >>>>> > + // contains a valid value. We have exclusive access, as we hold a `mut` reference to
>> >>>>> > + // `self`.
>> >>>>> > + unsafe { drop_in_place(self.value.get()) };
>> >>>>>
>> >>>>> This load does not need to be Acquire. It can be a Relaxed load or
>> >>>>> even an unsynchronized one since the access is exclusive.
>> >>>>
>> >>>> Right, I think we can do the similar as Revocable here:
>> >>>>
>> >>>> if *self.init.get_mut() == 2 { }
>> >
>> > Ok, now I got it. You are saying I don't need to use the atomic load
>> > method, because I have mutable access. Sounds good.
>> >
>> > But I guess a relaxed load and access through a mutable reference should
>> > result in the same code generation on most (all?) platforms?
>>
>> AFAIK it is not the same on arm.
>>
>
> Right, when LTO=y, arm64 use acquire load to implement
> READ_ONCE()/atomic_read().
But Andreas was talking about relaxed load vs mutable reference (=
normal unsynchronized write)?
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
2025-07-09 20:22 ` Benno Lossin
@ 2025-07-09 21:05 ` Boqun Feng
0 siblings, 0 replies; 28+ messages in thread
From: Boqun Feng @ 2025-07-09 21:05 UTC (permalink / raw)
To: Benno Lossin
Cc: Andreas Hindborg, Alice Ryhl, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Masahiro Yamada, Nathan Chancellor,
Luis Chamberlain, Danilo Krummrich, Nicolas Schier, Trevor Gross,
Adam Bratschi-Kaye, rust-for-linux, linux-kernel, linux-kbuild,
Petr Pavlu, Sami Tolvanen, Daniel Gomez, Simona Vetter, Greg KH,
Fiona Behrens, Daniel Almeida, linux-modules
On Wed, Jul 09, 2025 at 10:22:04PM +0200, Benno Lossin wrote:
[...]
> >> >>>>> > +impl<T> Drop for SetOnce<T> {
> >> >>>>> > + fn drop(&mut self) {
> >> >>>>> > + if self.init.load(Acquire) == 2 {
> >> >>>>> > + // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
> >> >>>>> > + // contains a valid value. We have exclusive access, as we hold a `mut` reference to
> >> >>>>> > + // `self`.
> >> >>>>> > + unsafe { drop_in_place(self.value.get()) };
> >> >>>>>
> >> >>>>> This load does not need to be Acquire. It can be a Relaxed load or
> >> >>>>> even an unsynchronized one since the access is exclusive.
> >> >>>>
> >> >>>> Right, I think we can do the similar as Revocable here:
> >> >>>>
> >> >>>> if *self.init.get_mut() == 2 { }
> >> >
> >> > Ok, now I got it. You are saying I don't need to use the atomic load
> >> > method, because I have mutable access. Sounds good.
> >> >
> >> > But I guess a relaxed load and access through a mutable reference should
> >> > result in the same code generation on most (all?) platforms?
> >>
> >> AFAIK it is not the same on arm.
> >>
> >
> > Right, when LTO=y, arm64 use acquire load to implement
> > READ_ONCE()/atomic_read().
>
> But Andreas was talking about relaxed load vs mutable reference (=
> normal unsynchronized write)?
>
No, I think it was a relaxed load (self.init.load(Relaxed)) vs a normal
unsynchronized *load* (*self.init.get_mut()). Yes, there is a mutable
reference, but we never use it for write.
Regards,
Boqun
> ---
> Cheers,
> Benno
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
2025-07-07 13:38 ` Alice Ryhl
2025-07-07 15:13 ` Boqun Feng
@ 2025-07-08 8:47 ` Andreas Hindborg
2025-07-08 9:00 ` Alice Ryhl
1 sibling, 1 reply; 28+ messages in thread
From: Andreas Hindborg @ 2025-07-08 8:47 UTC (permalink / raw)
To: Alice Ryhl
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Masahiro Yamada, Nathan Chancellor,
Luis Chamberlain, Danilo Krummrich, Benno Lossin, Nicolas Schier,
Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules
"Alice Ryhl" <aliceryhl@google.com> writes:
> On Mon, Jul 7, 2025 at 3:32 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> Introduce the `SetOnce` type, a container that can only be written once.
>> The container uses an internal atomic to synchronize writes to the internal
>> value.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> LGTM:
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>
>> +impl<T> Drop for SetOnce<T> {
>> + fn drop(&mut self) {
>> + if self.init.load(Acquire) == 2 {
>> + // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
>> + // contains a valid value. We have exclusive access, as we hold a `mut` reference to
>> + // `self`.
>> + unsafe { drop_in_place(self.value.get()) };
>
> This load does not need to be Acquire. It can be a Relaxed load or
> even an unsynchronized one since the access is exclusive.
Right, that is actually very cool. My rationale was that if a reference
has been shared to another thread of execution, we would need to
synchronize here to see a possible initialization from that other
thread. But I guess it is impossible to end the lifetime of a reference
without doing a synchronization somewhere else.
Best regards,
Andreas Hindborg
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
2025-07-08 8:47 ` Andreas Hindborg
@ 2025-07-08 9:00 ` Alice Ryhl
0 siblings, 0 replies; 28+ messages in thread
From: Alice Ryhl @ 2025-07-08 9:00 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Masahiro Yamada, Nathan Chancellor,
Luis Chamberlain, Danilo Krummrich, Benno Lossin, Nicolas Schier,
Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules
On Tue, Jul 8, 2025 at 10:48 AM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> "Alice Ryhl" <aliceryhl@google.com> writes:
>
> > On Mon, Jul 7, 2025 at 3:32 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
> >>
> >> Introduce the `SetOnce` type, a container that can only be written once.
> >> The container uses an internal atomic to synchronize writes to the internal
> >> value.
> >>
> >> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> >
> > LGTM:
> > Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> >
> >> +impl<T> Drop for SetOnce<T> {
> >> + fn drop(&mut self) {
> >> + if self.init.load(Acquire) == 2 {
> >> + // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
> >> + // contains a valid value. We have exclusive access, as we hold a `mut` reference to
> >> + // `self`.
> >> + unsafe { drop_in_place(self.value.get()) };
> >
> > This load does not need to be Acquire. It can be a Relaxed load or
> > even an unsynchronized one since the access is exclusive.
>
> Right, that is actually very cool. My rationale was that if a reference
> has been shared to another thread of execution, we would need to
> synchronize here to see a possible initialization from that other
> thread. But I guess it is impossible to end the lifetime of a reference
> without doing a synchronization somewhere else.
Yup, a mutable reference generally implies synchronization.
Alice
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
2025-07-07 13:29 ` [PATCH v15 1/7] rust: sync: add `SetOnce` Andreas Hindborg
2025-07-07 13:35 ` Andreas Hindborg
2025-07-07 13:38 ` Alice Ryhl
@ 2025-07-08 9:02 ` Benno Lossin
2025-07-08 13:06 ` Andreas Hindborg
2 siblings, 1 reply; 28+ messages in thread
From: Benno Lossin @ 2025-07-08 9:02 UTC (permalink / raw)
To: Andreas Hindborg, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules
On Mon Jul 7, 2025 at 3:29 PM CEST, Andreas Hindborg wrote:
> Introduce the `SetOnce` type, a container that can only be written once.
> The container uses an internal atomic to synchronize writes to the internal
> value.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
One nit and a safety comment fix below. (feel free to ignore the nit)
With the safety comment fixed:
Reviewed-by: Benno Lossin <lossin@kernel.org>
> ---
> rust/kernel/sync.rs | 2 +
> rust/kernel/sync/set_once.rs | 125 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 127 insertions(+)
>
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index 81e3a806e57e2..13e6bc7fa87ac 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -18,6 +18,7 @@
> mod locked_by;
> pub mod poll;
> pub mod rcu;
> +mod set_once;
I would have named this `once`.
> pub use arc::{Arc, ArcBorrow, UniqueArc};
> pub use completion::Completion;
> + /// Get a reference to the contained object.
> + ///
> + /// Returns [`None`] if this [`SetOnce`] is empty.
> + pub fn as_ref(&self) -> Option<&T> {
> + if self.init.load(Acquire) == 2 {
> + // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
> + // contains a valid value.
And the type invariants also ensure that the value of `self.init`
doesn't change.
So probably
// SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
// contains a valid value. They also guarantee that `self.init` doesn't change.
If you come up with something better, feel free to use it.
---
Cheers,
Benno
> + Some(unsafe { &*self.value.get() })
> + } else {
> + None
> + }
> + }
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
2025-07-08 9:02 ` Benno Lossin
@ 2025-07-08 13:06 ` Andreas Hindborg
2025-07-08 14:19 ` Benno Lossin
0 siblings, 1 reply; 28+ messages in thread
From: Andreas Hindborg @ 2025-07-08 13:06 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Nicolas Schier, Trevor Gross, Adam Bratschi-Kaye, rust-for-linux,
linux-kernel, linux-kbuild, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, Simona Vetter, Greg KH, Fiona Behrens,
Daniel Almeida, linux-modules
"Benno Lossin" <lossin@kernel.org> writes:
> On Mon Jul 7, 2025 at 3:29 PM CEST, Andreas Hindborg wrote:
>> Introduce the `SetOnce` type, a container that can only be written once.
>> The container uses an internal atomic to synchronize writes to the internal
>> value.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> One nit and a safety comment fix below. (feel free to ignore the nit)
> With the safety comment fixed:
>
> Reviewed-by: Benno Lossin <lossin@kernel.org>
>
>> ---
>> rust/kernel/sync.rs | 2 +
>> rust/kernel/sync/set_once.rs | 125 +++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 127 insertions(+)
>>
>> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
>> index 81e3a806e57e2..13e6bc7fa87ac 100644
>> --- a/rust/kernel/sync.rs
>> +++ b/rust/kernel/sync.rs
>> @@ -18,6 +18,7 @@
>> mod locked_by;
>> pub mod poll;
>> pub mod rcu;
>> +mod set_once;
>
> I would have named this `once`.
So module `once` and struct `SetOnce`? Struct name `Once` would lead
thoughts to `std::sync::Once`, which is a different thing.
>
>> pub use arc::{Arc, ArcBorrow, UniqueArc};
>> pub use completion::Completion;
>
>> + /// Get a reference to the contained object.
>> + ///
>> + /// Returns [`None`] if this [`SetOnce`] is empty.
>> + pub fn as_ref(&self) -> Option<&T> {
>> + if self.init.load(Acquire) == 2 {
>> + // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
>> + // contains a valid value.
>
> And the type invariants also ensure that the value of `self.init`
> doesn't change.
>
> So probably
>
> // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
> // contains a valid value. They also guarantee that `self.init` doesn't change.
>
Sure 👍
Best regards,
Andreas Hindborg
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
2025-07-08 13:06 ` Andreas Hindborg
@ 2025-07-08 14:19 ` Benno Lossin
2025-07-09 8:56 ` Andreas Hindborg
0 siblings, 1 reply; 28+ messages in thread
From: Benno Lossin @ 2025-07-08 14:19 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Nicolas Schier, Trevor Gross, Adam Bratschi-Kaye, rust-for-linux,
linux-kernel, linux-kbuild, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, Simona Vetter, Greg KH, Fiona Behrens,
Daniel Almeida, linux-modules
On Tue Jul 8, 2025 at 3:06 PM CEST, Andreas Hindborg wrote:
> "Benno Lossin" <lossin@kernel.org> writes:
>> On Mon Jul 7, 2025 at 3:29 PM CEST, Andreas Hindborg wrote:
>>> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
>>> index 81e3a806e57e2..13e6bc7fa87ac 100644
>>> --- a/rust/kernel/sync.rs
>>> +++ b/rust/kernel/sync.rs
>>> @@ -18,6 +18,7 @@
>>> mod locked_by;
>>> pub mod poll;
>>> pub mod rcu;
>>> +mod set_once;
>>
>> I would have named this `once`.
>
> So module `once` and struct `SetOnce`? Struct name `Once` would lead
> thoughts to `std::sync::Once`, which is a different thing.
Hmm I thought that `Once` and `SetOnce` would live in the same module,
but if they don't then I think it's better to keep the `set_once`
module as-is.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
2025-07-08 14:19 ` Benno Lossin
@ 2025-07-09 8:56 ` Andreas Hindborg
2025-07-09 9:10 ` Benno Lossin
0 siblings, 1 reply; 28+ messages in thread
From: Andreas Hindborg @ 2025-07-09 8:56 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Nicolas Schier, Trevor Gross, Adam Bratschi-Kaye, rust-for-linux,
linux-kernel, linux-kbuild, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, Simona Vetter, Greg KH, Fiona Behrens,
Daniel Almeida, linux-modules
"Benno Lossin" <lossin@kernel.org> writes:
> On Tue Jul 8, 2025 at 3:06 PM CEST, Andreas Hindborg wrote:
>> "Benno Lossin" <lossin@kernel.org> writes:
>>> On Mon Jul 7, 2025 at 3:29 PM CEST, Andreas Hindborg wrote:
>>>> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
>>>> index 81e3a806e57e2..13e6bc7fa87ac 100644
>>>> --- a/rust/kernel/sync.rs
>>>> +++ b/rust/kernel/sync.rs
>>>> @@ -18,6 +18,7 @@
>>>> mod locked_by;
>>>> pub mod poll;
>>>> pub mod rcu;
>>>> +mod set_once;
>>>
>>> I would have named this `once`.
>>
>> So module `once` and struct `SetOnce`? Struct name `Once` would lead
>> thoughts to `std::sync::Once`, which is a different thing.
>
> Hmm I thought that `Once` and `SetOnce` would live in the same module,
> but if they don't then I think it's better to keep the `set_once`
> module as-is.
I guess they could live together. I was thinking a module for each. We
can always move it, the module name is not part of a public API.
Let's go with `set_once` for now and we can change it later, if we
decide it is for the better?
Best regards,
Andreas Hindborg
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
2025-07-09 8:56 ` Andreas Hindborg
@ 2025-07-09 9:10 ` Benno Lossin
0 siblings, 0 replies; 28+ messages in thread
From: Benno Lossin @ 2025-07-09 9:10 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Nicolas Schier, Trevor Gross, Adam Bratschi-Kaye, rust-for-linux,
linux-kernel, linux-kbuild, Petr Pavlu, Sami Tolvanen,
Daniel Gomez, Simona Vetter, Greg KH, Fiona Behrens,
Daniel Almeida, linux-modules
On Wed Jul 9, 2025 at 10:56 AM CEST, Andreas Hindborg wrote:
> "Benno Lossin" <lossin@kernel.org> writes:
>
>> On Tue Jul 8, 2025 at 3:06 PM CEST, Andreas Hindborg wrote:
>>> "Benno Lossin" <lossin@kernel.org> writes:
>>>> On Mon Jul 7, 2025 at 3:29 PM CEST, Andreas Hindborg wrote:
>>>>> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
>>>>> index 81e3a806e57e2..13e6bc7fa87ac 100644
>>>>> --- a/rust/kernel/sync.rs
>>>>> +++ b/rust/kernel/sync.rs
>>>>> @@ -18,6 +18,7 @@
>>>>> mod locked_by;
>>>>> pub mod poll;
>>>>> pub mod rcu;
>>>>> +mod set_once;
>>>>
>>>> I would have named this `once`.
>>>
>>> So module `once` and struct `SetOnce`? Struct name `Once` would lead
>>> thoughts to `std::sync::Once`, which is a different thing.
>>
>> Hmm I thought that `Once` and `SetOnce` would live in the same module,
>> but if they don't then I think it's better to keep the `set_once`
>> module as-is.
>
> I guess they could live together. I was thinking a module for each. We
> can always move it, the module name is not part of a public API.
>
> Let's go with `set_once` for now and we can change it later, if we
> decide it is for the better?
Sure.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v15 2/7] rust: str: add radix prefixed integer parsing functions
2025-07-07 13:29 [PATCH v15 0/7] rust: extend `module!` macro with integer parameter support Andreas Hindborg
2025-07-07 13:29 ` [PATCH v15 1/7] rust: sync: add `SetOnce` Andreas Hindborg
@ 2025-07-07 13:29 ` Andreas Hindborg
2025-07-08 8:55 ` Benno Lossin
2025-07-07 13:29 ` [PATCH v15 3/7] rust: introduce module_param module Andreas Hindborg
` (5 subsequent siblings)
7 siblings, 1 reply; 28+ messages in thread
From: Andreas Hindborg @ 2025-07-07 13:29 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Benno Lossin, Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules, Andreas Hindborg
Add the trait `ParseInt` for parsing string representations of integers
where the string representations are optionally prefixed by a radix
specifier. Implement the trait for the primitive integer types.
Tested-by: Daniel Gomez <da.gomez@samsung.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Suggested-by: Benno Lossin <benno.lossin@proton.me>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/str.rs | 2 +
rust/kernel/str/parse_int.rs | 148 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 150 insertions(+)
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index a927db8e079c3..2b6c8b4a0ae4b 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -8,6 +8,8 @@
use crate::prelude::*;
+pub mod parse_int;
+
/// Byte string without UTF-8 validity guarantee.
#[repr(transparent)]
pub struct BStr([u8]);
diff --git a/rust/kernel/str/parse_int.rs b/rust/kernel/str/parse_int.rs
new file mode 100644
index 0000000000000..48eb4c202984c
--- /dev/null
+++ b/rust/kernel/str/parse_int.rs
@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Integer parsing functions.
+//!
+//! Integer parsing functions for parsing signed and unsigned integers
+//! potentially prefixed with `0x`, `0o`, or `0b`.
+
+use crate::prelude::*;
+use crate::str::BStr;
+use core::ops::Deref;
+
+// Make `FromStrRadix` a public type with a private name. This seals
+// `ParseInt`, that is, prevents downstream users from implementing the
+// trait.
+mod private {
+ use crate::prelude::*;
+ use crate::str::BStr;
+
+ /// Trait that allows parsing a [`&BStr`] to an integer with a radix.
+ pub trait FromStrRadix: Sized {
+ /// Parse `src` to [`Self`] using radix `radix`.
+ fn from_str_radix(src: &BStr, radix: u32) -> Result<Self>;
+
+ /// Tries to convert `value` into [`Self`] and negates the resulting value.
+ fn from_u64_negated(value: u64) -> Result<Self>;
+ }
+}
+
+/// Extract the radix from an integer literal optionally prefixed with
+/// one of `0x`, `0X`, `0o`, `0O`, `0b`, `0B`, `0`.
+fn strip_radix(src: &BStr) -> (u32, &BStr) {
+ match src.deref() {
+ [b'0', b'x' | b'X', rest @ ..] => (16, rest.as_ref()),
+ [b'0', b'o' | b'O', rest @ ..] => (8, rest.as_ref()),
+ [b'0', b'b' | b'B', rest @ ..] => (2, rest.as_ref()),
+ // NOTE: We are including the leading zero to be able to parse
+ // literal `0` here. If we removed it as a radix prefix, we would
+ // not be able to parse `0`.
+ [b'0', ..] => (8, src),
+ _ => (10, src),
+ }
+}
+
+/// Trait for parsing string representations of integers.
+///
+/// Strings beginning with `0x`, `0o`, or `0b` are parsed as hex, octal, or
+/// binary respectively. Strings beginning with `0` otherwise are parsed as
+/// octal. Anything else is parsed as decimal. A leading `+` or `-` is also
+/// permitted. Any string parsed by [`kstrtol()`] or [`kstrtoul()`] will be
+/// successfully parsed.
+///
+/// [`kstrtol()`]: https://docs.kernel.org/core-api/kernel-api.html#c.kstrtol
+/// [`kstrtoul()`]: https://docs.kernel.org/core-api/kernel-api.html#c.kstrtoul
+///
+/// # Examples
+///
+/// ```
+/// # use kernel::str::parse_int::ParseInt;
+/// # use kernel::b_str;
+///
+/// assert_eq!(Ok(0u8), u8::from_str(b_str!("0")));
+///
+/// assert_eq!(Ok(0xa2u8), u8::from_str(b_str!("0xa2")));
+/// assert_eq!(Ok(-0xa2i32), i32::from_str(b_str!("-0xa2")));
+///
+/// assert_eq!(Ok(-0o57i8), i8::from_str(b_str!("-0o57")));
+/// assert_eq!(Ok(0o57i8), i8::from_str(b_str!("057")));
+///
+/// assert_eq!(Ok(0b1001i16), i16::from_str(b_str!("0b1001")));
+/// assert_eq!(Ok(-0b1001i16), i16::from_str(b_str!("-0b1001")));
+///
+/// assert_eq!(Ok(127i8), i8::from_str(b_str!("127")));
+/// assert!(i8::from_str(b_str!("128")).is_err());
+/// assert_eq!(Ok(-128i8), i8::from_str(b_str!("-128")));
+/// assert!(i8::from_str(b_str!("-129")).is_err());
+/// assert_eq!(Ok(255u8), u8::from_str(b_str!("255")));
+/// assert!(u8::from_str(b_str!("256")).is_err());
+/// ```
+pub trait ParseInt: private::FromStrRadix + TryFrom<u64> {
+ /// Parse a string according to the description in [`Self`].
+ fn from_str(src: &BStr) -> Result<Self> {
+ match src.deref() {
+ [b'-', rest @ ..] => {
+ let (radix, digits) = strip_radix(rest.as_ref());
+ // 2's complement values range from -2^(b-1) to 2^(b-1)-1.
+ // So if we want to parse negative numbers as positive and
+ // later multiply by -1, we have to parse into a larger
+ // integer. We choose `u64` as sufficiently large.
+ //
+ // NOTE: 128 bit integers are not available on all
+ // platforms, hence the choice of 64 bits.
+ let val =
+ u64::from_str_radix(core::str::from_utf8(digits).map_err(|_| EINVAL)?, radix)
+ .map_err(|_| EINVAL)?;
+ Self::from_u64_negated(val)
+ }
+ _ => {
+ let (radix, digits) = strip_radix(src);
+ Self::from_str_radix(digits, radix).map_err(|_| EINVAL)
+ }
+ }
+ }
+}
+
+macro_rules! impl_parse_int {
+ ($($ty:ty),*) => {
+ $(
+ impl private::FromStrRadix for $ty {
+ fn from_str_radix(src: &BStr, radix: u32) -> Result<Self> {
+ <$ty>::from_str_radix(core::str::from_utf8(src).map_err(|_| EINVAL)?, radix)
+ .map_err(|_| EINVAL)
+ }
+
+ fn from_u64_negated(value: u64) -> Result<Self> {
+ const ABS_MIN: u64 = {
+ #[allow(unused_comparisons)]
+ if <$ty>::MIN < 0 {
+ 1u64 << (<$ty>::BITS - 1)
+ } else {
+ 0
+ }
+ };
+
+ if value > ABS_MIN {
+ return Err(EINVAL);
+ }
+
+ if value == ABS_MIN {
+ return Ok(<$ty>::MIN);
+ }
+
+ // SAFETY: The above checks guarantee that `value` fits into `Self`:
+ // - if `Self` is unsigned, then `ABS_MIN == 0` and thus we have returned above
+ // (either `EINVAL` or `MIN`).
+ // - if `Self` is signed, then we have that `0 <= value < ABS_MIN`. And since
+ // `ABS_MIN - 1` fits into `Self` by construction, `value` also does.
+ let value: Self = unsafe { value.try_into().unwrap_unchecked() };
+
+ Ok((!value).wrapping_add(1))
+ }
+ }
+
+ impl ParseInt for $ty {}
+ )*
+ };
+}
+
+impl_parse_int![i8, u8, i16, u16, i32, u32, i64, u64, isize, usize];
--
2.47.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v15 2/7] rust: str: add radix prefixed integer parsing functions
2025-07-07 13:29 ` [PATCH v15 2/7] rust: str: add radix prefixed integer parsing functions Andreas Hindborg
@ 2025-07-08 8:55 ` Benno Lossin
0 siblings, 0 replies; 28+ messages in thread
From: Benno Lossin @ 2025-07-08 8:55 UTC (permalink / raw)
To: Andreas Hindborg, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules
On Mon Jul 7, 2025 at 3:29 PM CEST, Andreas Hindborg wrote:
> Add the trait `ParseInt` for parsing string representations of integers
> where the string representations are optionally prefixed by a radix
> specifier. Implement the trait for the primitive integer types.
>
> Tested-by: Daniel Gomez <da.gomez@samsung.com>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Suggested-by: Benno Lossin <benno.lossin@proton.me>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
Reviewed-by: Benno Lossin <lossin@kernel.org>
---
Cheers,
Benno
> ---
> rust/kernel/str.rs | 2 +
> rust/kernel/str/parse_int.rs | 148 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 150 insertions(+)
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v15 3/7] rust: introduce module_param module
2025-07-07 13:29 [PATCH v15 0/7] rust: extend `module!` macro with integer parameter support Andreas Hindborg
2025-07-07 13:29 ` [PATCH v15 1/7] rust: sync: add `SetOnce` Andreas Hindborg
2025-07-07 13:29 ` [PATCH v15 2/7] rust: str: add radix prefixed integer parsing functions Andreas Hindborg
@ 2025-07-07 13:29 ` Andreas Hindborg
2025-07-07 13:29 ` [PATCH v15 4/7] rust: module: use a reference in macros::module::module Andreas Hindborg
` (4 subsequent siblings)
7 siblings, 0 replies; 28+ messages in thread
From: Andreas Hindborg @ 2025-07-07 13:29 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Benno Lossin, Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules, Andreas Hindborg
Add types and traits for interfacing the C moduleparam API.
Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/lib.rs | 1 +
rust/kernel/module_param.rs | 181 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 182 insertions(+)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 6b4774b2b1c37..2b439ea061850 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -87,6 +87,7 @@
pub mod list;
pub mod miscdevice;
pub mod mm;
+pub mod module_param;
#[cfg(CONFIG_NET)]
pub mod net;
pub mod of;
diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
new file mode 100644
index 0000000000000..9b187ed1d3513
--- /dev/null
+++ b/rust/kernel/module_param.rs
@@ -0,0 +1,181 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Support for module parameters.
+//!
+//! C header: [`include/linux/moduleparam.h`](srctree/include/linux/moduleparam.h)
+
+use crate::prelude::*;
+use crate::str::BStr;
+use bindings;
+use kernel::sync::SetOnce;
+
+/// Newtype to make `bindings::kernel_param` [`Sync`].
+#[repr(transparent)]
+#[doc(hidden)]
+pub struct KernelParam(bindings::kernel_param);
+
+impl KernelParam {
+ #[doc(hidden)]
+ pub const fn new(val: bindings::kernel_param) -> Self {
+ Self(val)
+ }
+}
+
+// SAFETY: C kernel handles serializing access to this type. We never access it
+// from Rust module.
+unsafe impl Sync for KernelParam {}
+
+/// Types that can be used for module parameters.
+// NOTE: This trait is `Copy` because drop could produce unsoundness during teardown.
+pub trait ModuleParam: Sized + Copy {
+ /// Parse a parameter argument into the parameter value.
+ fn try_from_param_arg(arg: &BStr) -> Result<Self>;
+}
+
+/// Set the module parameter from a string.
+///
+/// Used to set the parameter value at kernel initialization, when loading
+/// the module or when set through `sysfs`.
+///
+/// See `struct kernel_param_ops.set`.
+///
+/// # Safety
+///
+/// - If `val` is non-null then it must point to a valid null-terminated string that must be valid
+/// for reads for the duration of the call.
+/// - `param` must be a pointer to a `bindings::kernel_param` initialized by the rust module macro.
+/// The pointee must be valid for reads for the duration of the call.
+///
+/// # Note
+///
+/// - The safety requirements are satisfied by C API contract when this function is invoked by the
+/// module subsystem C code.
+/// - Currently, we only support read-only parameters that are not readable from `sysfs`. Thus, this
+/// function is only called at kernel initialization time, or at module load time, and we have
+/// exclusive access to the parameter for the duration of the function.
+///
+/// [`module!`]: macros::module
+unsafe extern "C" fn set_param<T>(val: *const c_char, param: *const bindings::kernel_param) -> c_int
+where
+ T: ModuleParam,
+{
+ // NOTE: If we start supporting arguments without values, val _is_ allowed
+ // to be null here.
+ if val.is_null() {
+ // TODO: Use pr_warn_once available.
+ crate::pr_warn!("Null pointer passed to `module_param::set_param`");
+ return EINVAL.to_errno();
+ }
+
+ // SAFETY: By function safety requirement, val is non-null, null-terminated
+ // and valid for reads for the duration of this function.
+ let arg = unsafe { CStr::from_char_ptr(val) };
+
+ crate::error::from_result(|| {
+ let new_value = T::try_from_param_arg(arg)?;
+
+ // SAFETY: By function safety requirements, this access is safe.
+ let container = unsafe { &*((*param).__bindgen_anon_1.arg as *mut SetOnce<T>) };
+
+ container
+ .populate(new_value)
+ .then_some(0)
+ .ok_or(kernel::error::code::EEXIST)
+ })
+}
+
+macro_rules! impl_int_module_param {
+ ($ty:ident) => {
+ impl ModuleParam for $ty {
+ fn try_from_param_arg(arg: &BStr) -> Result<Self> {
+ <$ty as crate::str::parse_int::ParseInt>::from_str(arg)
+ }
+ }
+ };
+}
+
+impl_int_module_param!(i8);
+impl_int_module_param!(u8);
+impl_int_module_param!(i16);
+impl_int_module_param!(u16);
+impl_int_module_param!(i32);
+impl_int_module_param!(u32);
+impl_int_module_param!(i64);
+impl_int_module_param!(u64);
+impl_int_module_param!(isize);
+impl_int_module_param!(usize);
+
+/// A wrapper for kernel parameters.
+///
+/// This type is instantiated by the [`module!`] macro when module parameters are
+/// defined. You should never need to instantiate this type directly.
+///
+/// Note: This type is `pub` because it is used by module crates to access
+/// parameter values.
+pub struct ModuleParamAccess<T> {
+ value: SetOnce<T>,
+ default: T,
+}
+
+// SAFETY: We only create shared references to the contents of this container,
+// so if `T` is `Sync`, so is `ModuleParamAccess`.
+unsafe impl<T: Sync> Sync for ModuleParamAccess<T> {}
+
+impl<T> ModuleParamAccess<T> {
+ #[doc(hidden)]
+ pub const fn new(default: T) -> Self {
+ Self {
+ value: SetOnce::new(),
+ default,
+ }
+ }
+
+ /// Get a shared reference to the parameter value.
+ // Note: When sysfs access to parameters are enabled, we have to pass in a
+ // held lock guard here.
+ pub fn value(&self) -> &T {
+ self.value.as_ref().unwrap_or(&self.default)
+ }
+
+ /// Get a mutable pointer to `self`.
+ ///
+ /// NOTE: In most cases it is not safe deref the returned pointer.
+ pub const fn as_void_ptr(&self) -> *mut c_void {
+ core::ptr::from_ref(self).cast_mut().cast()
+ }
+}
+
+#[doc(hidden)]
+/// Generate a static [`kernel_param_ops`](srctree/include/linux/moduleparam.h) struct.
+///
+/// # Examples
+///
+/// ```ignore
+/// make_param_ops!(
+/// /// Documentation for new param ops.
+/// PARAM_OPS_MYTYPE, // Name for the static.
+/// MyType // A type which implements [`ModuleParam`].
+/// );
+/// ```
+macro_rules! make_param_ops {
+ ($ops:ident, $ty:ty) => {
+ #[doc(hidden)]
+ pub static $ops: $crate::bindings::kernel_param_ops = $crate::bindings::kernel_param_ops {
+ flags: 0,
+ set: Some(set_param::<$ty>),
+ get: None,
+ free: None,
+ };
+ };
+}
+
+make_param_ops!(PARAM_OPS_I8, i8);
+make_param_ops!(PARAM_OPS_U8, u8);
+make_param_ops!(PARAM_OPS_I16, i16);
+make_param_ops!(PARAM_OPS_U16, u16);
+make_param_ops!(PARAM_OPS_I32, i32);
+make_param_ops!(PARAM_OPS_U32, u32);
+make_param_ops!(PARAM_OPS_I64, i64);
+make_param_ops!(PARAM_OPS_U64, u64);
+make_param_ops!(PARAM_OPS_ISIZE, isize);
+make_param_ops!(PARAM_OPS_USIZE, usize);
--
2.47.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v15 4/7] rust: module: use a reference in macros::module::module
2025-07-07 13:29 [PATCH v15 0/7] rust: extend `module!` macro with integer parameter support Andreas Hindborg
` (2 preceding siblings ...)
2025-07-07 13:29 ` [PATCH v15 3/7] rust: introduce module_param module Andreas Hindborg
@ 2025-07-07 13:29 ` Andreas Hindborg
2025-07-07 13:29 ` [PATCH v15 5/7] rust: module: update the module macro with module parameter support Andreas Hindborg
` (3 subsequent siblings)
7 siblings, 0 replies; 28+ messages in thread
From: Andreas Hindborg @ 2025-07-07 13:29 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Benno Lossin, Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules, Andreas Hindborg
When we add parameter support to the module macro, we want to be able to
pass a reference to `ModuleInfo` to a helper function. That is not possible
when we move out of the local `modinfo`. So change the function to access
the local via reference rather than value.
Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/macros/module.rs | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index 2ddd2eeb28521..1a867a1e787ed 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -179,26 +179,26 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
// Rust does not allow hyphens in identifiers, use underscore instead.
let ident = info.name.replace('-', "_");
let mut modinfo = ModInfoBuilder::new(ident.as_ref());
- if let Some(author) = info.author {
- modinfo.emit("author", &author);
+ if let Some(author) = &info.author {
+ modinfo.emit("author", author);
}
- if let Some(authors) = info.authors {
+ if let Some(authors) = &info.authors {
for author in authors {
- modinfo.emit("author", &author);
+ modinfo.emit("author", author);
}
}
- if let Some(description) = info.description {
- modinfo.emit("description", &description);
+ if let Some(description) = &info.description {
+ modinfo.emit("description", description);
}
modinfo.emit("license", &info.license);
- if let Some(aliases) = info.alias {
+ if let Some(aliases) = &info.alias {
for alias in aliases {
- modinfo.emit("alias", &alias);
+ modinfo.emit("alias", alias);
}
}
- if let Some(firmware) = info.firmware {
+ if let Some(firmware) = &info.firmware {
for fw in firmware {
- modinfo.emit("firmware", &fw);
+ modinfo.emit("firmware", fw);
}
}
--
2.47.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v15 5/7] rust: module: update the module macro with module parameter support
2025-07-07 13:29 [PATCH v15 0/7] rust: extend `module!` macro with integer parameter support Andreas Hindborg
` (3 preceding siblings ...)
2025-07-07 13:29 ` [PATCH v15 4/7] rust: module: use a reference in macros::module::module Andreas Hindborg
@ 2025-07-07 13:29 ` Andreas Hindborg
2025-07-07 13:29 ` [PATCH v15 6/7] rust: samples: add a module parameter to the rust_minimal sample Andreas Hindborg
` (2 subsequent siblings)
7 siblings, 0 replies; 28+ messages in thread
From: Andreas Hindborg @ 2025-07-07 13:29 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Benno Lossin, Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules, Andreas Hindborg
Allow module parameters to be declared in the rust `module!` macro.
Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/macros/helpers.rs | 25 +++++++
rust/macros/lib.rs | 31 +++++++++
rust/macros/module.rs | 178 ++++++++++++++++++++++++++++++++++++++++++++++---
3 files changed, 224 insertions(+), 10 deletions(-)
diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs
index e2602be402c10..365d7eb499c08 100644
--- a/rust/macros/helpers.rs
+++ b/rust/macros/helpers.rs
@@ -10,6 +10,17 @@ pub(crate) fn try_ident(it: &mut token_stream::IntoIter) -> Option<String> {
}
}
+pub(crate) fn try_sign(it: &mut token_stream::IntoIter) -> Option<char> {
+ let peek = it.clone().next();
+ match peek {
+ Some(TokenTree::Punct(punct)) if punct.as_char() == '-' => {
+ let _ = it.next();
+ Some(punct.as_char())
+ }
+ _ => None,
+ }
+}
+
pub(crate) fn try_literal(it: &mut token_stream::IntoIter) -> Option<String> {
if let Some(TokenTree::Literal(literal)) = it.next() {
Some(literal.to_string())
@@ -103,3 +114,17 @@ pub(crate) fn file() -> String {
proc_macro::Span::call_site().file()
}
}
+
+/// Parse a token stream of the form `expected_name: "value",` and return the
+/// string in the position of "value".
+///
+/// # Panics
+///
+/// - On parse error.
+pub(crate) fn expect_string_field(it: &mut token_stream::IntoIter, expected_name: &str) -> String {
+ assert_eq!(expect_ident(it), expected_name);
+ assert_eq!(expect_punct(it), ':');
+ let string = expect_string(it);
+ assert_eq!(expect_punct(it), ',');
+ string
+}
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index fa847cf3a9b5f..2fb520dc930af 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -28,6 +28,30 @@
/// The `type` argument should be a type which implements the [`Module`]
/// trait. Also accepts various forms of kernel metadata.
///
+/// The `params` field describe module parameters. Each entry has the form
+///
+/// ```ignore
+/// parameter_name: type {
+/// default: default_value,
+/// description: "Description",
+/// }
+/// ```
+///
+/// `type` may be one of
+///
+/// - [`i8`]
+/// - [`u8`]
+/// - [`i8`]
+/// - [`u8`]
+/// - [`i16`]
+/// - [`u16`]
+/// - [`i32`]
+/// - [`u32`]
+/// - [`i64`]
+/// - [`u64`]
+/// - [`isize`]
+/// - [`usize`]
+///
/// C header: [`include/linux/moduleparam.h`](srctree/include/linux/moduleparam.h)
///
/// [`Module`]: ../kernel/trait.Module.html
@@ -44,6 +68,12 @@
/// description: "My very own kernel module!",
/// license: "GPL",
/// alias: ["alternate_module_name"],
+/// params: {
+/// my_parameter: i64 {
+/// default: 1,
+/// description: "This parameter has a default of 1",
+/// },
+/// },
/// }
///
/// struct MyModule(i32);
@@ -52,6 +82,7 @@
/// fn init(_module: &'static ThisModule) -> Result<Self> {
/// let foo: i32 = 42;
/// pr_info!("I contain: {}\n", foo);
+/// pr_info!("i32 param is: {}\n", module_parameters::my_parameter.read());
/// Ok(Self(foo))
/// }
/// }
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index 1a867a1e787ed..c1400597774a5 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -26,6 +26,7 @@ struct ModInfoBuilder<'a> {
module: &'a str,
counter: usize,
buffer: String,
+ param_buffer: String,
}
impl<'a> ModInfoBuilder<'a> {
@@ -34,10 +35,11 @@ fn new(module: &'a str) -> Self {
module,
counter: 0,
buffer: String::new(),
+ param_buffer: String::new(),
}
}
- fn emit_base(&mut self, field: &str, content: &str, builtin: bool) {
+ fn emit_base(&mut self, field: &str, content: &str, builtin: bool, param: bool) {
let string = if builtin {
// Built-in modules prefix their modinfo strings by `module.`.
format!(
@@ -51,8 +53,14 @@ fn emit_base(&mut self, field: &str, content: &str, builtin: bool) {
format!("{field}={content}\0")
};
+ let buffer = if param {
+ &mut self.param_buffer
+ } else {
+ &mut self.buffer
+ };
+
write!(
- &mut self.buffer,
+ buffer,
"
{cfg}
#[doc(hidden)]
@@ -75,20 +83,119 @@ fn emit_base(&mut self, field: &str, content: &str, builtin: bool) {
self.counter += 1;
}
- fn emit_only_builtin(&mut self, field: &str, content: &str) {
- self.emit_base(field, content, true)
+ fn emit_only_builtin(&mut self, field: &str, content: &str, param: bool) {
+ self.emit_base(field, content, true, param)
}
- fn emit_only_loadable(&mut self, field: &str, content: &str) {
- self.emit_base(field, content, false)
+ fn emit_only_loadable(&mut self, field: &str, content: &str, param: bool) {
+ self.emit_base(field, content, false, param)
}
fn emit(&mut self, field: &str, content: &str) {
- self.emit_only_builtin(field, content);
- self.emit_only_loadable(field, content);
+ self.emit_internal(field, content, false);
+ }
+
+ fn emit_internal(&mut self, field: &str, content: &str, param: bool) {
+ self.emit_only_builtin(field, content, param);
+ self.emit_only_loadable(field, content, param);
+ }
+
+ fn emit_param(&mut self, field: &str, param: &str, content: &str) {
+ let content = format!("{param}:{content}", param = param, content = content);
+ self.emit_internal(field, &content, true);
+ }
+
+ fn emit_params(&mut self, info: &ModuleInfo) {
+ let Some(params) = &info.params else {
+ return;
+ };
+
+ for param in params {
+ let ops = param_ops_path(¶m.ptype);
+
+ // Note: The spelling of these fields is dictated by the user space
+ // tool `modinfo`.
+ self.emit_param("parmtype", ¶m.name, ¶m.ptype);
+ self.emit_param("parm", ¶m.name, ¶m.description);
+
+ write!(
+ self.param_buffer,
+ "
+ pub(crate) static {param_name}:
+ ::kernel::module_param::ModuleParamAccess<{param_type}> =
+ ::kernel::module_param::ModuleParamAccess::new({param_default});
+
+ const _: () = {{
+ #[link_section = \"__param\"]
+ #[used]
+ static __{module_name}_{param_name}_struct:
+ ::kernel::module_param::KernelParam =
+ ::kernel::module_param::KernelParam::new(
+ ::kernel::bindings::kernel_param {{
+ name: if ::core::cfg!(MODULE) {{
+ ::kernel::c_str!(\"{param_name}\").as_bytes_with_nul()
+ }} else {{
+ ::kernel::c_str!(\"{module_name}.{param_name}\")
+ .as_bytes_with_nul()
+ }}.as_ptr(),
+ // SAFETY: `__this_module` is constructed by the kernel at load
+ // time and will not be freed until the module is unloaded.
+ #[cfg(MODULE)]
+ mod_: unsafe {{
+ core::ptr::from_ref(&::kernel::bindings::__this_module)
+ .cast_mut()
+ }},
+ #[cfg(not(MODULE))]
+ mod_: ::core::ptr::null_mut(),
+ ops: core::ptr::from_ref(&{ops}),
+ perm: 0, // Will not appear in sysfs
+ level: -1,
+ flags: 0,
+ __bindgen_anon_1: ::kernel::bindings::kernel_param__bindgen_ty_1 {{
+ arg: {param_name}.as_void_ptr()
+ }},
+ }}
+ );
+ }};
+ ",
+ module_name = info.name,
+ param_type = param.ptype,
+ param_default = param.default,
+ param_name = param.name,
+ ops = ops,
+ )
+ .unwrap();
+ }
+ }
+}
+
+fn param_ops_path(param_type: &str) -> &'static str {
+ match param_type {
+ "i8" => "::kernel::module_param::PARAM_OPS_I8",
+ "u8" => "::kernel::module_param::PARAM_OPS_U8",
+ "i16" => "::kernel::module_param::PARAM_OPS_I16",
+ "u16" => "::kernel::module_param::PARAM_OPS_U16",
+ "i32" => "::kernel::module_param::PARAM_OPS_I32",
+ "u32" => "::kernel::module_param::PARAM_OPS_U32",
+ "i64" => "::kernel::module_param::PARAM_OPS_I64",
+ "u64" => "::kernel::module_param::PARAM_OPS_U64",
+ "isize" => "::kernel::module_param::PARAM_OPS_ISIZE",
+ "usize" => "::kernel::module_param::PARAM_OPS_USIZE",
+ t => panic!("Unsupported parameter type {}", t),
}
}
+fn expect_param_default(param_it: &mut token_stream::IntoIter) -> String {
+ assert_eq!(expect_ident(param_it), "default");
+ assert_eq!(expect_punct(param_it), ':');
+ let sign = try_sign(param_it);
+ let default = try_literal(param_it).expect("Expected default param value");
+ assert_eq!(expect_punct(param_it), ',');
+ let mut value = sign.map(String::from).unwrap_or_default();
+ value.push_str(&default);
+ value
+}
+
#[derive(Debug, Default)]
struct ModuleInfo {
type_: String,
@@ -99,6 +206,50 @@ struct ModuleInfo {
description: Option<String>,
alias: Option<Vec<String>>,
firmware: Option<Vec<String>>,
+ params: Option<Vec<Parameter>>,
+}
+
+#[derive(Debug)]
+struct Parameter {
+ name: String,
+ ptype: String,
+ default: String,
+ description: String,
+}
+
+fn expect_params(it: &mut token_stream::IntoIter) -> Vec<Parameter> {
+ let params = expect_group(it);
+ assert_eq!(params.delimiter(), Delimiter::Brace);
+ let mut it = params.stream().into_iter();
+ let mut parsed = Vec::new();
+
+ loop {
+ let param_name = match it.next() {
+ Some(TokenTree::Ident(ident)) => ident.to_string(),
+ Some(_) => panic!("Expected Ident or end"),
+ None => break,
+ };
+
+ assert_eq!(expect_punct(&mut it), ':');
+ let param_type = expect_ident(&mut it);
+ let group = expect_group(&mut it);
+ assert_eq!(group.delimiter(), Delimiter::Brace);
+ assert_eq!(expect_punct(&mut it), ',');
+
+ let mut param_it = group.stream().into_iter();
+ let param_default = expect_param_default(&mut param_it);
+ let param_description = expect_string_field(&mut param_it, "description");
+ expect_end(&mut param_it);
+
+ parsed.push(Parameter {
+ name: param_name,
+ ptype: param_type,
+ default: param_default,
+ description: param_description,
+ })
+ }
+
+ parsed
}
impl ModuleInfo {
@@ -114,6 +265,7 @@ fn parse(it: &mut token_stream::IntoIter) -> Self {
"license",
"alias",
"firmware",
+ "params",
];
const REQUIRED_KEYS: &[&str] = &["type", "name", "license"];
let mut seen_keys = Vec::new();
@@ -140,6 +292,7 @@ fn parse(it: &mut token_stream::IntoIter) -> Self {
"license" => info.license = expect_string_ascii(it),
"alias" => info.alias = Some(expect_string_array(it)),
"firmware" => info.firmware = Some(expect_string_array(it)),
+ "params" => info.params = Some(expect_params(it)),
_ => panic!("Unknown key \"{key}\". Valid keys are: {EXPECTED_KEYS:?}."),
}
@@ -205,7 +358,9 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
// Built-in modules also export the `file` modinfo string.
let file =
std::env::var("RUST_MODFILE").expect("Unable to fetch RUST_MODFILE environmental variable");
- modinfo.emit_only_builtin("file", &file);
+ modinfo.emit_only_builtin("file", &file, false);
+
+ modinfo.emit_params(&info);
format!(
"
@@ -369,15 +524,18 @@ unsafe fn __exit() {{
__MOD.assume_init_drop();
}}
}}
-
{modinfo}
}}
}}
+ mod module_parameters {{
+ {params}
+ }}
",
type_ = info.type_,
name = info.name,
ident = ident,
modinfo = modinfo.buffer,
+ params = modinfo.param_buffer,
initcall_section = ".initcall6.init"
)
.parse()
--
2.47.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v15 6/7] rust: samples: add a module parameter to the rust_minimal sample
2025-07-07 13:29 [PATCH v15 0/7] rust: extend `module!` macro with integer parameter support Andreas Hindborg
` (4 preceding siblings ...)
2025-07-07 13:29 ` [PATCH v15 5/7] rust: module: update the module macro with module parameter support Andreas Hindborg
@ 2025-07-07 13:29 ` Andreas Hindborg
2025-07-07 13:29 ` [PATCH v15 7/7] modules: add rust modules files to MAINTAINERS Andreas Hindborg
2025-07-07 14:05 ` [PATCH v15 0/7] rust: extend `module!` macro with integer parameter support Miguel Ojeda
7 siblings, 0 replies; 28+ messages in thread
From: Andreas Hindborg @ 2025-07-07 13:29 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Benno Lossin, Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules, Andreas Hindborg
Showcase the rust module parameter support by adding a module parameter to
the `rust_minimal` sample.
Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
samples/rust/rust_minimal.rs | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/samples/rust/rust_minimal.rs b/samples/rust/rust_minimal.rs
index 1fc7a1be6b6d7..8eb9583571d72 100644
--- a/samples/rust/rust_minimal.rs
+++ b/samples/rust/rust_minimal.rs
@@ -10,6 +10,12 @@
authors: ["Rust for Linux Contributors"],
description: "Rust minimal sample",
license: "GPL",
+ params: {
+ test_parameter: i64 {
+ default: 1,
+ description: "This parameter has a default of 1",
+ },
+ },
}
struct RustMinimal {
@@ -20,6 +26,10 @@ impl kernel::Module for RustMinimal {
fn init(_module: &'static ThisModule) -> Result<Self> {
pr_info!("Rust minimal sample (init)\n");
pr_info!("Am I built-in? {}\n", !cfg!(MODULE));
+ pr_info!(
+ "test_parameter: {}\n",
+ *module_parameters::test_parameter.value()
+ );
let mut numbers = KVec::new();
numbers.push(72, GFP_KERNEL)?;
--
2.47.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v15 7/7] modules: add rust modules files to MAINTAINERS
2025-07-07 13:29 [PATCH v15 0/7] rust: extend `module!` macro with integer parameter support Andreas Hindborg
` (5 preceding siblings ...)
2025-07-07 13:29 ` [PATCH v15 6/7] rust: samples: add a module parameter to the rust_minimal sample Andreas Hindborg
@ 2025-07-07 13:29 ` Andreas Hindborg
2025-07-07 14:05 ` [PATCH v15 0/7] rust: extend `module!` macro with integer parameter support Miguel Ojeda
7 siblings, 0 replies; 28+ messages in thread
From: Andreas Hindborg @ 2025-07-07 13:29 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Benno Lossin, Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules, Andreas Hindborg
The module subsystem people agreed to maintain rust support for modules
[1]. Thus, add entries for relevant files to modules entry in MAINTAINERS.
Link: https://lore.kernel.org/all/0d9e596a-5316-4e00-862b-fd77552ae4b5@suse.com/ [1]
Acked-by: Daniel Gomez <da.gomez@samsung.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
MAINTAINERS | 2 ++
1 file changed, 2 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index d431320ed3b2a..afa385ecc5c4c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16835,6 +16835,8 @@ F: include/linux/module*.h
F: kernel/module/
F: lib/test_kmod.c
F: lib/tests/module/
+F: rust/kernel/module_param.rs
+F: rust/macros/module.rs
F: scripts/module*
F: tools/testing/selftests/kmod/
F: tools/testing/selftests/module/
--
2.47.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v15 0/7] rust: extend `module!` macro with integer parameter support
2025-07-07 13:29 [PATCH v15 0/7] rust: extend `module!` macro with integer parameter support Andreas Hindborg
` (6 preceding siblings ...)
2025-07-07 13:29 ` [PATCH v15 7/7] modules: add rust modules files to MAINTAINERS Andreas Hindborg
@ 2025-07-07 14:05 ` Miguel Ojeda
2025-07-08 13:09 ` Andreas Hindborg
7 siblings, 1 reply; 28+ messages in thread
From: Miguel Ojeda @ 2025-07-07 14:05 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Nicolas Schier, Trevor Gross, Adam Bratschi-Kaye,
rust-for-linux, linux-kernel, linux-kbuild, Petr Pavlu,
Sami Tolvanen, Daniel Gomez, Simona Vetter, Greg KH,
Fiona Behrens, Daniel Almeida, linux-modules
On Mon, Jul 7, 2025 at 3:31 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Based on code by Adam Bratschi-Kaye lifted from the original `rust` branch [1].
I would suggest adding this sort of note to the commit messages,
especially since the commits have no Co-developed-by/Link tags
otherwise.
From our discussion in Zulip: the code itself is older than that the
merge above. I think you wanted an example sentence for this --
something simple could be e.g.
Based on the original module parameter support by Miguel [1],
later extended and generalized by Adam for more types [2][3].
Originally tracked at [4].
Link: https://github.com/Rust-for-Linux/linux/pull/7 [1]
Link: https://github.com/Rust-for-Linux/linux/pull/82 [2]
Link: https://github.com/Rust-for-Linux/linux/pull/87 [3]
Link: https://github.com/Rust-for-Linux/linux/issues/11 [4]
By the way, I guess you should inherit that issue in the last link :)
It had some details about things we may or may not want to support
etc. that I looked up back then. If you prefer that we close it or
that we create sub-issues, that is fine -- up to you!
Finally, if you end up adding strings, please link to Adam's
https://github.com/Rust-for-Linux/linux/pull/110.
Thanks!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v15 0/7] rust: extend `module!` macro with integer parameter support
2025-07-07 14:05 ` [PATCH v15 0/7] rust: extend `module!` macro with integer parameter support Miguel Ojeda
@ 2025-07-08 13:09 ` Andreas Hindborg
0 siblings, 0 replies; 28+ messages in thread
From: Andreas Hindborg @ 2025-07-08 13:09 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Nicolas Schier, Trevor Gross, Adam Bratschi-Kaye,
rust-for-linux, linux-kernel, linux-kbuild, Petr Pavlu,
Sami Tolvanen, Daniel Gomez, Simona Vetter, Greg KH,
Fiona Behrens, Daniel Almeida, linux-modules
"Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com> writes:
> On Mon, Jul 7, 2025 at 3:31 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> Based on code by Adam Bratschi-Kaye lifted from the original `rust` branch [1].
>
> I would suggest adding this sort of note to the commit messages,
> especially since the commits have no Co-developed-by/Link tags
> otherwise.
>
> From our discussion in Zulip: the code itself is older than that the
> merge above. I think you wanted an example sentence for this --
> something simple could be e.g.
>
> Based on the original module parameter support by Miguel [1],
> later extended and generalized by Adam for more types [2][3].
> Originally tracked at [4].
>
> Link: https://github.com/Rust-for-Linux/linux/pull/7 [1]
> Link: https://github.com/Rust-for-Linux/linux/pull/82 [2]
> Link: https://github.com/Rust-for-Linux/linux/pull/87 [3]
> Link: https://github.com/Rust-for-Linux/linux/issues/11 [4]
>
> By the way, I guess you should inherit that issue in the last link :)
> It had some details about things we may or may not want to support
> etc. that I looked up back then. If you prefer that we close it or
> that we create sub-issues, that is fine -- up to you!
>
> Finally, if you end up adding strings, please link to Adam's
> https://github.com/Rust-for-Linux/linux/pull/110.
Thanks, I'll add this!
Best regards,
Andreas Hindborg
^ permalink raw reply [flat|nested] 28+ messages in thread