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.2On 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
> + }
> + }
"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
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
"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
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
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
"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
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
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
"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
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
"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
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
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
>
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
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
>
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
© 2016 - 2025 Red Hat, Inc.