[PATCH] rust: sync: add lazy initialization methods to SetOnce

[PATCH] rust: sync: add lazy initialization methods to SetOnce

[Andreas Hindborg]

Add methods to get a reference to the contained value or populate the
SetOnce if empty. The new `as_ref_or_populate` method accepts a value
directly, while `as_ref_or_populate_with` accepts a fallible closure,
allowing for lazy initialization that may fail. Both methods spin-wait
if another thread is concurrently initializing the container.

Also add `populate_with` which takes a fallible closure and serves as
the implementation basis for the other populate methods.

Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/sync/set_once.rs | 53 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 46 insertions(+), 7 deletions(-)

diff --git a/rust/kernel/sync/set_once.rs b/rust/kernel/sync/set_once.rs
index bdba601807d8b..9e3c4be4047f8 100644
--- a/rust/kernel/sync/set_once.rs
+++ b/rust/kernel/sync/set_once.rs
@@ -2,11 +2,14 @@
 
 //! A container that can be initialized at most once.
 
-use super::atomic::{
-    ordering::{Acquire, Relaxed, Release},
-    Atomic,
-};
 use core::{cell::UnsafeCell, mem::MaybeUninit};
+use kernel::{
+    error::Result,
+    sync::atomic::{
+        ordering::{Acquire, Relaxed, Release},
+        Atomic,
+    },
+};
 
 /// A container that can be populated at most once. Thread safe.
 ///
@@ -76,10 +79,46 @@ pub fn as_ref(&self) -> Option<&T> {
         }
     }
 
+    /// Get a reference to the contained object, or populate the [`SetOnce`]
+    /// with the value returned by `callable` and return a reference to that
+    /// object.
+    pub fn as_ref_or_populate_with(&self, callable: impl FnOnce() -> Result<T>) -> Result<&T> {
+        if !self.populate_with(callable)? {
+            while self.init.load(Acquire) != 2 {
+                core::hint::spin_loop();
+            }
+        }
+
+        // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
+        // is initialized and valid for shared access.
+        Ok(unsafe { &*self.value.get().cast() })
+    }
+
+    /// Get a reference to the contained object, or populate the [`SetOnce`]
+    /// with `value` and return a reference to that object.
+    pub fn as_ref_or_populate(&self, value: T) -> &T {
+        if !self.populate(value) {
+            while self.init.load(Acquire) != 2 {
+                core::hint::spin_loop();
+            }
+        }
+
+        // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
+        // is initialized and valid for shared access.
+        unsafe { &*self.value.get().cast() }
+    }
+
     /// Populate the [`SetOnce`].
     ///
     /// Returns `true` if the [`SetOnce`] was successfully populated.
     pub fn populate(&self, value: T) -> bool {
+        self.populate_with(|| Ok(value)).expect("Cannot error")
+    }
+
+    /// Populate the [`SetOnce`] with the value returned by `callable`.
+    ///
+    /// Returns `true` if the [`SetOnce`] was successfully populated.
+    pub fn populate_with(&self, callable: impl FnOnce() -> Result<T>) -> Result<bool> {
         // INVARIANT: If the swap succeeds:
         //  - We increase `init`.
         //  - We write the valid value `1` to `init`.
@@ -88,16 +127,16 @@ pub fn populate(&self, value: T) -> bool {
         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 `self.value`.
-            unsafe { core::ptr::write(self.value.get().cast(), value) };
+            unsafe { core::ptr::write(self.value.get().cast(), callable()?) };
             // INVARIANT:
             //  - We increase `init`.
             //  - We write the valid value `2` to `init`.
             //  - We release our exclusive access to `self.value` and it is now valid for shared
             //    access.
             self.init.store(2, Release);
-            true
+            Ok(true)
         } else {
-            false
+            Ok(false)
         }
     }
 

---
base-commit: 05f7e89ab9731565d8a62e3b5d1ec206485eeb0b
change-id: 20260215-set-once-lazy-c73fc34a55d9

Best regards,
-- 
Andreas Hindborg <a.hindborg@kernel.org>

Re: [PATCH] rust: sync: add lazy initialization methods to SetOnce

[Benno Lossin]

On Sun Feb 15, 2026 at 9:27 PM CET, Andreas Hindborg wrote:
> @@ -76,10 +79,46 @@ pub fn as_ref(&self) -> Option<&T> {
>          }
>      }
>  
> +    /// Get a reference to the contained object, or populate the [`SetOnce`]
> +    /// with the value returned by `callable` and return a reference to that
> +    /// object.
> +    pub fn as_ref_or_populate_with(&self, callable: impl FnOnce() -> Result<T>) -> Result<&T> {

I would name the argument `create`, but not `callable`. Same below.

Cheers,
Benno

> +        if !self.populate_with(callable)? {
> +            while self.init.load(Acquire) != 2 {
> +                core::hint::spin_loop();
> +            }
> +        }

Re: [PATCH] rust: sync: add lazy initialization methods to SetOnce

[Alice Ryhl]

On Sun, Feb 15, 2026 at 09:27:17PM +0100, Andreas Hindborg wrote:
> Add methods to get a reference to the contained value or populate the
> SetOnce if empty. The new `as_ref_or_populate` method accepts a value
> directly, while `as_ref_or_populate_with` accepts a fallible closure,
> allowing for lazy initialization that may fail. Both methods spin-wait
> if another thread is concurrently initializing the container.
> 
> Also add `populate_with` which takes a fallible closure and serves as
> the implementation basis for the other populate methods.
> 
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

> +    /// Get a reference to the contained object, or populate the [`SetOnce`]
> +    /// with the value returned by `callable` and return a reference to that
> +    /// object.
> +    pub fn as_ref_or_populate_with(&self, callable: impl FnOnce() -> Result<T>) -> Result<&T> {
> +        if !self.populate_with(callable)? {
> +            while self.init.load(Acquire) != 2 {
> +                core::hint::spin_loop();
> +            }

We should not be implementing our own spinlocks.

Alice

Re: [PATCH] rust: sync: add lazy initialization methods to SetOnce

[Andreas Hindborg]

"Alice Ryhl" <aliceryhl@google.com> writes:

> On Sun, Feb 15, 2026 at 09:27:17PM +0100, Andreas Hindborg wrote:
>> Add methods to get a reference to the contained value or populate the
>> SetOnce if empty. The new `as_ref_or_populate` method accepts a value
>> directly, while `as_ref_or_populate_with` accepts a fallible closure,
>> allowing for lazy initialization that may fail. Both methods spin-wait
>> if another thread is concurrently initializing the container.
>>
>> Also add `populate_with` which takes a fallible closure and serves as
>> the implementation basis for the other populate methods.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
>> +    /// Get a reference to the contained object, or populate the [`SetOnce`]
>> +    /// with the value returned by `callable` and return a reference to that
>> +    /// object.
>> +    pub fn as_ref_or_populate_with(&self, callable: impl FnOnce() -> Result<T>) -> Result<&T> {
>> +        if !self.populate_with(callable)? {
>> +            while self.init.load(Acquire) != 2 {
>> +                core::hint::spin_loop();
>> +            }
>
> We should not be implementing our own spinlocks.

That is a great proverb. I'd be happy to receive a suggestion on an
alternate approach for this particular context.


Best regards,
Andreas Hindborg

Re: [PATCH] rust: sync: add lazy initialization methods to SetOnce

[Alice Ryhl]

On Mon, Feb 16, 2026 at 12:10:16PM +0100, Andreas Hindborg wrote:
> "Alice Ryhl" <aliceryhl@google.com> writes:
> 
> > On Sun, Feb 15, 2026 at 09:27:17PM +0100, Andreas Hindborg wrote:
> >> Add methods to get a reference to the contained value or populate the
> >> SetOnce if empty. The new `as_ref_or_populate` method accepts a value
> >> directly, while `as_ref_or_populate_with` accepts a fallible closure,
> >> allowing for lazy initialization that may fail. Both methods spin-wait
> >> if another thread is concurrently initializing the container.
> >>
> >> Also add `populate_with` which takes a fallible closure and serves as
> >> the implementation basis for the other populate methods.
> >>
> >> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> >
> >> +    /// Get a reference to the contained object, or populate the [`SetOnce`]
> >> +    /// with the value returned by `callable` and return a reference to that
> >> +    /// object.
> >> +    pub fn as_ref_or_populate_with(&self, callable: impl FnOnce() -> Result<T>) -> Result<&T> {
> >> +        if !self.populate_with(callable)? {
> >> +            while self.init.load(Acquire) != 2 {
> >> +                core::hint::spin_loop();
> >> +            }
> >
> > We should not be implementing our own spinlocks.
> 
> That is a great proverb. I'd be happy to receive a suggestion on an
> alternate approach for this particular context.

You can add a spinlock to SetOnce. Like I mentioned previously [1],
support for waiting will require the addition of extra fields.

I suppose we might be able to use include/linux/bit_spinlock.h here to
avoid taking up any additional space.

Alice

[1]: https://lore.kernel.org/all/CAH5fLggY2Ei14nVJzLBEoR1Rut1GKU4SZX=+14tuRH1aSuQVTA@mail.gmail.com/

Re: [PATCH] rust: sync: add lazy initialization methods to SetOnce

[Alice Ryhl]

On Mon, Feb 16, 2026 at 11:26:11AM +0000, Alice Ryhl wrote:
> On Mon, Feb 16, 2026 at 12:10:16PM +0100, Andreas Hindborg wrote:
> > "Alice Ryhl" <aliceryhl@google.com> writes:
> > 
> > > On Sun, Feb 15, 2026 at 09:27:17PM +0100, Andreas Hindborg wrote:
> > >> Add methods to get a reference to the contained value or populate the
> > >> SetOnce if empty. The new `as_ref_or_populate` method accepts a value
> > >> directly, while `as_ref_or_populate_with` accepts a fallible closure,
> > >> allowing for lazy initialization that may fail. Both methods spin-wait
> > >> if another thread is concurrently initializing the container.
> > >>
> > >> Also add `populate_with` which takes a fallible closure and serves as
> > >> the implementation basis for the other populate methods.
> > >>
> > >> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> > >
> > >> +    /// Get a reference to the contained object, or populate the [`SetOnce`]
> > >> +    /// with the value returned by `callable` and return a reference to that
> > >> +    /// object.
> > >> +    pub fn as_ref_or_populate_with(&self, callable: impl FnOnce() -> Result<T>) -> Result<&T> {
> > >> +        if !self.populate_with(callable)? {
> > >> +            while self.init.load(Acquire) != 2 {
> > >> +                core::hint::spin_loop();
> > >> +            }
> > >
> > > We should not be implementing our own spinlocks.
> > 
> > That is a great proverb. I'd be happy to receive a suggestion on an
> > alternate approach for this particular context.
> 
> You can add a spinlock to SetOnce. Like I mentioned previously [1],
> support for waiting will require the addition of extra fields.

By the way, back then I suggested renaming it from OnceLock to SetOnce
because you did not support waiting for the value to be populated, and
you said you didn't need that. If you add that feature, then we should
rename it back to OnceLock, or create a new type OnceLock for users who
need that additional feature.

Alice

Re: [PATCH] rust: sync: add lazy initialization methods to SetOnce

[Andreas Hindborg]

"Alice Ryhl" <aliceryhl@google.com> writes:

> On Mon, Feb 16, 2026 at 11:26:11AM +0000, Alice Ryhl wrote:
>> On Mon, Feb 16, 2026 at 12:10:16PM +0100, Andreas Hindborg wrote:
>> > "Alice Ryhl" <aliceryhl@google.com> writes:
>> >
>> > > On Sun, Feb 15, 2026 at 09:27:17PM +0100, Andreas Hindborg wrote:
>> > >> Add methods to get a reference to the contained value or populate the
>> > >> SetOnce if empty. The new `as_ref_or_populate` method accepts a value
>> > >> directly, while `as_ref_or_populate_with` accepts a fallible closure,
>> > >> allowing for lazy initialization that may fail. Both methods spin-wait
>> > >> if another thread is concurrently initializing the container.
>> > >>
>> > >> Also add `populate_with` which takes a fallible closure and serves as
>> > >> the implementation basis for the other populate methods.
>> > >>
>> > >> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> > >
>> > >> +    /// Get a reference to the contained object, or populate the [`SetOnce`]
>> > >> +    /// with the value returned by `callable` and return a reference to that
>> > >> +    /// object.
>> > >> +    pub fn as_ref_or_populate_with(&self, callable: impl FnOnce() -> Result<T>) -> Result<&T> {
>> > >> +        if !self.populate_with(callable)? {
>> > >> +            while self.init.load(Acquire) != 2 {
>> > >> +                core::hint::spin_loop();
>> > >> +            }
>> > >
>> > > We should not be implementing our own spinlocks.
>> >
>> > That is a great proverb. I'd be happy to receive a suggestion on an
>> > alternate approach for this particular context.
>>
>> You can add a spinlock to SetOnce. Like I mentioned previously [1],
>> support for waiting will require the addition of extra fields.

Thanks, I'll be sure to take a look again.

>
> By the way, back then I suggested renaming it from OnceLock to SetOnce
> because you did not support waiting for the value to be populated, and
> you said you didn't need that. If you add that feature, then we should
> rename it back to OnceLock, or create a new type OnceLock for users who
> need that additional feature.

That is fair. This is a different use case than the original one though.
I think we should keep this as one type for code reuse, but I am fine
with renaming to something that describe the usage better.


Best regards,
Andreas Hindborg

Re: [PATCH] rust: sync: add lazy initialization methods to SetOnce

[Gary Guo]

On Sun Feb 15, 2026 at 8:27 PM GMT, Andreas Hindborg wrote:
> Add methods to get a reference to the contained value or populate the
> SetOnce if empty. The new `as_ref_or_populate` method accepts a value
> directly, while `as_ref_or_populate_with` accepts a fallible closure,
> allowing for lazy initialization that may fail. Both methods spin-wait
> if another thread is concurrently initializing the container.
>
> Also add `populate_with` which takes a fallible closure and serves as
> the implementation basis for the other populate methods.

Hi Andreas, in an earlier call I mentioned that I'm working on getting SetOnce
to work with pin-init, the capability of which I think is a superset of you have
here.

The API I have is

impl<T> SetOnce<T> {
    pub fn init<E>(&self, init: impl Init<T, E>) -> Result<&T, InitError<E>>;
    pub fn pin_init<E>(self, Pin<&Self>, init: impl PinInit<T, E>) -> Result<&T, InitError<E>>;
}

To achieve what you need with a function, you can simply write:

    set_once.init(pin_init::init_scope(your_fn))

The patch that implement the API is here:
https://github.com/nbdd0121/linux/commit/4aabdbcf20b11626c253f203745b1d55c37ab2ee
in tree
https://github.com/nbdd0121/linux/tree/lazy_revocable_nova_wip/

which I haven't submitted to the list as the user side of this API isn't ready.

Best,
Gary

>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
>  rust/kernel/sync/set_once.rs | 53 ++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 46 insertions(+), 7 deletions(-)
>
> diff --git a/rust/kernel/sync/set_once.rs b/rust/kernel/sync/set_once.rs
> index bdba601807d8b..9e3c4be4047f8 100644
> --- a/rust/kernel/sync/set_once.rs
> +++ b/rust/kernel/sync/set_once.rs
> @@ -2,11 +2,14 @@
>  
>  //! A container that can be initialized at most once.
>  
> -use super::atomic::{
> -    ordering::{Acquire, Relaxed, Release},
> -    Atomic,
> -};
>  use core::{cell::UnsafeCell, mem::MaybeUninit};
> +use kernel::{
> +    error::Result,
> +    sync::atomic::{
> +        ordering::{Acquire, Relaxed, Release},
> +        Atomic,
> +    },
> +};
>  
>  /// A container that can be populated at most once. Thread safe.
>  ///
> @@ -76,10 +79,46 @@ pub fn as_ref(&self) -> Option<&T> {
>          }
>      }
>  
> +    /// Get a reference to the contained object, or populate the [`SetOnce`]
> +    /// with the value returned by `callable` and return a reference to that
> +    /// object.
> +    pub fn as_ref_or_populate_with(&self, callable: impl FnOnce() -> Result<T>) -> Result<&T> {
> +        if !self.populate_with(callable)? {
> +            while self.init.load(Acquire) != 2 {
> +                core::hint::spin_loop();
> +            }
> +        }
> +
> +        // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
> +        // is initialized and valid for shared access.
> +        Ok(unsafe { &*self.value.get().cast() })
> +    }
> +
> +    /// Get a reference to the contained object, or populate the [`SetOnce`]
> +    /// with `value` and return a reference to that object.
> +    pub fn as_ref_or_populate(&self, value: T) -> &T {
> +        if !self.populate(value) {
> +            while self.init.load(Acquire) != 2 {
> +                core::hint::spin_loop();
> +            }
> +        }
> +
> +        // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
> +        // is initialized and valid for shared access.
> +        unsafe { &*self.value.get().cast() }
> +    }
> +
>      /// Populate the [`SetOnce`].
>      ///
>      /// Returns `true` if the [`SetOnce`] was successfully populated.
>      pub fn populate(&self, value: T) -> bool {
> +        self.populate_with(|| Ok(value)).expect("Cannot error")
> +    }
> +
> +    /// Populate the [`SetOnce`] with the value returned by `callable`.
> +    ///
> +    /// Returns `true` if the [`SetOnce`] was successfully populated.
> +    pub fn populate_with(&self, callable: impl FnOnce() -> Result<T>) -> Result<bool> {
>          // INVARIANT: If the swap succeeds:
>          //  - We increase `init`.
>          //  - We write the valid value `1` to `init`.
> @@ -88,16 +127,16 @@ pub fn populate(&self, value: T) -> bool {
>          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 `self.value`.
> -            unsafe { core::ptr::write(self.value.get().cast(), value) };
> +            unsafe { core::ptr::write(self.value.get().cast(), callable()?) };
>              // INVARIANT:
>              //  - We increase `init`.
>              //  - We write the valid value `2` to `init`.
>              //  - We release our exclusive access to `self.value` and it is now valid for shared
>              //    access.
>              self.init.store(2, Release);
> -            true
> +            Ok(true)
>          } else {
> -            false
> +            Ok(false)
>          }
>      }
>  
>
> ---
> base-commit: 05f7e89ab9731565d8a62e3b5d1ec206485eeb0b
> change-id: 20260215-set-once-lazy-c73fc34a55d9
>
> Best regards,

Re: [PATCH] rust: sync: add lazy initialization methods to SetOnce

[Andreas Hindborg]

"Gary Guo" <gary@garyguo.net> writes:

> On Sun Feb 15, 2026 at 8:27 PM GMT, Andreas Hindborg wrote:
>> Add methods to get a reference to the contained value or populate the
>> SetOnce if empty. The new `as_ref_or_populate` method accepts a value
>> directly, while `as_ref_or_populate_with` accepts a fallible closure,
>> allowing for lazy initialization that may fail. Both methods spin-wait
>> if another thread is concurrently initializing the container.
>>
>> Also add `populate_with` which takes a fallible closure and serves as
>> the implementation basis for the other populate methods.
>
> Hi Andreas, in an earlier call I mentioned that I'm working on getting SetOnce
> to work with pin-init, the capability of which I think is a superset of you have
> here.
>
> The API I have is
>
> impl<T> SetOnce<T> {
>     pub fn init<E>(&self, init: impl Init<T, E>) -> Result<&T, InitError<E>>;
>     pub fn pin_init<E>(self, Pin<&Self>, init: impl PinInit<T, E>) -> Result<&T, InitError<E>>;
> }
>
> To achieve what you need with a function, you can simply write:
>
>     set_once.init(pin_init::init_scope(your_fn))
>
> The patch that implement the API is here:
> https://github.com/nbdd0121/linux/commit/4aabdbcf20b11626c253f203745b1d55c37ab2ee
> in tree
> https://github.com/nbdd0121/linux/tree/lazy_revocable_nova_wip/
>
> which I haven't submitted to the list as the user side of this API isn't ready.

I probably evicted that from cache.

It looks like that could replace the `populate` in my patch? We would
still need the synchronization in `as_ref_or_populate`, right? (or
`as_ref_or_init` if you will)


Best regards,
Andreas Hindborg

Re: [PATCH] rust: sync: add lazy initialization methods to SetOnce

[Andreas Hindborg]

Andreas Hindborg <a.hindborg@kernel.org> writes:

> "Gary Guo" <gary@garyguo.net> writes:
>
>> On Sun Feb 15, 2026 at 8:27 PM GMT, Andreas Hindborg wrote:
>>> Add methods to get a reference to the contained value or populate the
>>> SetOnce if empty. The new `as_ref_or_populate` method accepts a value
>>> directly, while `as_ref_or_populate_with` accepts a fallible closure,
>>> allowing for lazy initialization that may fail. Both methods spin-wait
>>> if another thread is concurrently initializing the container.
>>>
>>> Also add `populate_with` which takes a fallible closure and serves as
>>> the implementation basis for the other populate methods.
>>
>> Hi Andreas, in an earlier call I mentioned that I'm working on getting SetOnce
>> to work with pin-init, the capability of which I think is a superset of you have
>> here.
>>
>> The API I have is
>>
>> impl<T> SetOnce<T> {
>>     pub fn init<E>(&self, init: impl Init<T, E>) -> Result<&T, InitError<E>>;
>>     pub fn pin_init<E>(self, Pin<&Self>, init: impl PinInit<T, E>) -> Result<&T, InitError<E>>;
>> }
>>
>> To achieve what you need with a function, you can simply write:
>>
>>     set_once.init(pin_init::init_scope(your_fn))
>>
>> The patch that implement the API is here:
>> https://github.com/nbdd0121/linux/commit/4aabdbcf20b11626c253f203745b1d55c37ab2ee
>> in tree
>> https://github.com/nbdd0121/linux/tree/lazy_revocable_nova_wip/
>>
>> which I haven't submitted to the list as the user side of this API isn't ready.
>
> I probably evicted that from cache.
>
> It looks like that could replace the `populate` in my patch? We would
> still need the synchronization in `as_ref_or_populate`, right? (or
> `as_ref_or_init` if you will)

Looks like this is not ready yet. I think we can move forward with the
current suggestion and then wire up pin-init when it lands.


Best regards,
Andreas Hindborg

Re: [PATCH] rust: sync: add lazy initialization methods to SetOnce

[Andreas Hindborg]

Andreas Hindborg <a.hindborg@kernel.org> writes:

> "Alice Ryhl" <aliceryhl@google.com> writes:
>
>> On Mon, Feb 16, 2026 at 11:26:11AM +0000, Alice Ryhl wrote:
>>> On Mon, Feb 16, 2026 at 12:10:16PM +0100, Andreas Hindborg wrote:
>>> > "Alice Ryhl" <aliceryhl@google.com> writes:
>>> >
>>> > > On Sun, Feb 15, 2026 at 09:27:17PM +0100, Andreas Hindborg wrote:
>>> > >> Add methods to get a reference to the contained value or populate the
>>> > >> SetOnce if empty. The new `as_ref_or_populate` method accepts a value
>>> > >> directly, while `as_ref_or_populate_with` accepts a fallible closure,
>>> > >> allowing for lazy initialization that may fail. Both methods spin-wait
>>> > >> if another thread is concurrently initializing the container.
>>> > >>
>>> > >> Also add `populate_with` which takes a fallible closure and serves as
>>> > >> the implementation basis for the other populate methods.
>>> > >>
>>> > >> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>>> > >
>>> > >> +    /// Get a reference to the contained object, or populate the [`SetOnce`]
>>> > >> +    /// with the value returned by `callable` and return a reference to that
>>> > >> +    /// object.
>>> > >> +    pub fn as_ref_or_populate_with(&self, callable: impl FnOnce() -> Result<T>) -> Result<&T> {
>>> > >> +        if !self.populate_with(callable)? {
>>> > >> +            while self.init.load(Acquire) != 2 {
>>> > >> +                core::hint::spin_loop();
>>> > >> +            }
>>> > >
>>> > > We should not be implementing our own spinlocks.
>>> >
>>> > That is a great proverb. I'd be happy to receive a suggestion on an
>>> > alternate approach for this particular context.
>>>
>>> You can add a spinlock to SetOnce. Like I mentioned previously [1],
>>> support for waiting will require the addition of extra fields.
>
> Thanks, I'll be sure to take a look again.

I took a look at this again. I think the structure will be less
efficient if we use a spin lock.

Initialization is now
 - cmpxchg lock relaxed
 - store pointer
 - store lock release

With a spin lock it will be
 - lock acquire
 - test pointer
 - store pointer
 - lock release

All the other tests for initialization is now:
 - load lock acquire
 - load pointer

They will become
 - lock acquire
 - load pointer
 - test pointer
 - lock release


bit_spinlock does not make this any better. It just gives us 64 locks in
a u64. It does not help us store state of the data structure
(empty/populated).

Do we want a less efficient data structure in order to gain benefits of
lockdep and friends?

>> By the way, back then I suggested renaming it from OnceLock to SetOnce
>> because you did not support waiting for the value to be populated, and
>> you said you didn't need that. If you add that feature, then we should
>> rename it back to OnceLock, or create a new type OnceLock for users who
>> need that additional feature.
>
> That is fair. This is a different use case than the original one though.
> I think we should keep this as one type for code reuse, but I am fine
> with renaming to something that describe the usage better.

Please note that even though it could be added, we do not have a `wait`
method now. What we have are blocking initializers.

I personally have no opinion on the name. Is everyone in favor of
renaming to OnceLock?


Best regards,
Andreas Hindborg

Re: [PATCH] rust: sync: add lazy initialization methods to SetOnce

[Alice Ryhl]

On Tue, May 12, 2026 at 10:39:57AM +0200, Andreas Hindborg wrote:
> Andreas Hindborg <a.hindborg@kernel.org> writes:
> 
> > "Alice Ryhl" <aliceryhl@google.com> writes:
> >
> >> On Mon, Feb 16, 2026 at 11:26:11AM +0000, Alice Ryhl wrote:
> >>> On Mon, Feb 16, 2026 at 12:10:16PM +0100, Andreas Hindborg wrote:
> >>> > "Alice Ryhl" <aliceryhl@google.com> writes:
> >>> >
> >>> > > On Sun, Feb 15, 2026 at 09:27:17PM +0100, Andreas Hindborg wrote:
> >>> > >> Add methods to get a reference to the contained value or populate the
> >>> > >> SetOnce if empty. The new `as_ref_or_populate` method accepts a value
> >>> > >> directly, while `as_ref_or_populate_with` accepts a fallible closure,
> >>> > >> allowing for lazy initialization that may fail. Both methods spin-wait
> >>> > >> if another thread is concurrently initializing the container.
> >>> > >>
> >>> > >> Also add `populate_with` which takes a fallible closure and serves as
> >>> > >> the implementation basis for the other populate methods.
> >>> > >>
> >>> > >> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> >>> > >
> >>> > >> +    /// Get a reference to the contained object, or populate the [`SetOnce`]
> >>> > >> +    /// with the value returned by `callable` and return a reference to that
> >>> > >> +    /// object.
> >>> > >> +    pub fn as_ref_or_populate_with(&self, callable: impl FnOnce() -> Result<T>) -> Result<&T> {
> >>> > >> +        if !self.populate_with(callable)? {
> >>> > >> +            while self.init.load(Acquire) != 2 {
> >>> > >> +                core::hint::spin_loop();
> >>> > >> +            }
> >>> > >
> >>> > > We should not be implementing our own spinlocks.
> >>> >
> >>> > That is a great proverb. I'd be happy to receive a suggestion on an
> >>> > alternate approach for this particular context.
> >>>
> >>> You can add a spinlock to SetOnce. Like I mentioned previously [1],
> >>> support for waiting will require the addition of extra fields.
> >
> > Thanks, I'll be sure to take a look again.
> 
> I took a look at this again. I think the structure will be less
> efficient if we use a spin lock.
> 
> Initialization is now
>  - cmpxchg lock relaxed
>  - store pointer
>  - store lock release
> 
> With a spin lock it will be
>  - lock acquire
>  - test pointer
>  - store pointer
>  - lock release
> 
> All the other tests for initialization is now:
>  - load lock acquire
>  - load pointer
> 
> They will become
>  - lock acquire
>  - load pointer
>  - test pointer
>  - lock release
> 
> 
> bit_spinlock does not make this any better. It just gives us 64 locks in
> a u64. It does not help us store state of the data structure
> (empty/populated).
> 
> Do we want a less efficient data structure in order to gain benefits of
> lockdep and friends?

I'm not just talking about lockdep. Your current implementation is wrong
in several other ways, for example:

1. Spinlocks must disable preemption.
2. It doesn't fall back to a mutex under PREEMPT_RT.

And probably lots of other things. By using the kernel spinlock, you do
not have to worry about the long list of things spinlocks have to get
right.

> >> By the way, back then I suggested renaming it from OnceLock to SetOnce
> >> because you did not support waiting for the value to be populated, and
> >> you said you didn't need that. If you add that feature, then we should
> >> rename it back to OnceLock, or create a new type OnceLock for users who
> >> need that additional feature.
> >
> > That is fair. This is a different use case than the original one though.
> > I think we should keep this as one type for code reuse, but I am fine
> > with renaming to something that describe the usage better.
> 
> Please note that even though it could be added, we do not have a `wait`
> method now. What we have are blocking initializers.

You may have open-coded `wait` inside of `as_ref_or_populate_with`, but
you still have the functionality.

Alice

> I personally have no opinion on the name. Is everyone in favor of
> renaming to OnceLock?
> 
> 
> Best regards,
> Andreas Hindborg
> 
> 

Re: [PATCH] rust: sync: add lazy initialization methods to SetOnce

[Andreas Hindborg]

"Alice Ryhl" <aliceryhl@google.com> writes:

> On Tue, May 12, 2026 at 10:39:57AM +0200, Andreas Hindborg wrote:
>> Andreas Hindborg <a.hindborg@kernel.org> writes:
>>
>> > "Alice Ryhl" <aliceryhl@google.com> writes:
>> >
>> >> On Mon, Feb 16, 2026 at 11:26:11AM +0000, Alice Ryhl wrote:
>> >>> On Mon, Feb 16, 2026 at 12:10:16PM +0100, Andreas Hindborg wrote:
>> >>> > "Alice Ryhl" <aliceryhl@google.com> writes:
>> >>> >
>> >>> > > On Sun, Feb 15, 2026 at 09:27:17PM +0100, Andreas Hindborg wrote:
>> >>> > >> Add methods to get a reference to the contained value or populate the
>> >>> > >> SetOnce if empty. The new `as_ref_or_populate` method accepts a value
>> >>> > >> directly, while `as_ref_or_populate_with` accepts a fallible closure,
>> >>> > >> allowing for lazy initialization that may fail. Both methods spin-wait
>> >>> > >> if another thread is concurrently initializing the container.
>> >>> > >>
>> >>> > >> Also add `populate_with` which takes a fallible closure and serves as
>> >>> > >> the implementation basis for the other populate methods.
>> >>> > >>
>> >>> > >> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> >>> > >
>> >>> > >> +    /// Get a reference to the contained object, or populate the [`SetOnce`]
>> >>> > >> +    /// with the value returned by `callable` and return a reference to that
>> >>> > >> +    /// object.
>> >>> > >> +    pub fn as_ref_or_populate_with(&self, callable: impl FnOnce() -> Result<T>) -> Result<&T> {
>> >>> > >> +        if !self.populate_with(callable)? {
>> >>> > >> +            while self.init.load(Acquire) != 2 {
>> >>> > >> +                core::hint::spin_loop();
>> >>> > >> +            }
>> >>> > >
>> >>> > > We should not be implementing our own spinlocks.
>> >>> >
>> >>> > That is a great proverb. I'd be happy to receive a suggestion on an
>> >>> > alternate approach for this particular context.
>> >>>
>> >>> You can add a spinlock to SetOnce. Like I mentioned previously [1],
>> >>> support for waiting will require the addition of extra fields.
>> >
>> > Thanks, I'll be sure to take a look again.
>>
>> I took a look at this again. I think the structure will be less
>> efficient if we use a spin lock.
>>
>> Initialization is now
>>  - cmpxchg lock relaxed
>>  - store pointer
>>  - store lock release
>>
>> With a spin lock it will be
>>  - lock acquire
>>  - test pointer
>>  - store pointer
>>  - lock release
>>
>> All the other tests for initialization is now:
>>  - load lock acquire
>>  - load pointer
>>
>> They will become
>>  - lock acquire
>>  - load pointer
>>  - test pointer
>>  - lock release
>>
>>
>> bit_spinlock does not make this any better. It just gives us 64 locks in
>> a u64. It does not help us store state of the data structure
>> (empty/populated).
>>
>> Do we want a less efficient data structure in order to gain benefits of
>> lockdep and friends?
>
> I'm not just talking about lockdep. Your current implementation is wrong
> in several other ways, for example:
>
> 1. Spinlocks must disable preemption.

That is an easy fix.

> 2. It doesn't fall back to a mutex under PREEMPT_RT.

I don't know how to solve that, but I'm sure there is a way.

>
> And probably lots of other things. By using the kernel spinlock, you do
> not have to worry about the long list of things spinlocks have to get
> right.

Nah, can't be that many things. But I get your point.

>> >> By the way, back then I suggested renaming it from OnceLock to SetOnce
>> >> because you did not support waiting for the value to be populated, and
>> >> you said you didn't need that. If you add that feature, then we should
>> >> rename it back to OnceLock, or create a new type OnceLock for users who
>> >> need that additional feature.
>> >
>> > That is fair. This is a different use case than the original one though.
>> > I think we should keep this as one type for code reuse, but I am fine
>> > with renaming to something that describe the usage better.
>>
>> Please note that even though it could be added, we do not have a `wait`
>> method now. What we have are blocking initializers.
>
> You may have open-coded `wait` inside of `as_ref_or_populate_with`, but
> you still have the functionality.

As I said, I'm fine with whatever name, but I'd appreciate if someone
else chime in, so we don't have to change the name too many times.


Best regards,
Andreas Hindborg

Re: [PATCH] rust: sync: add lazy initialization methods to SetOnce

[Andreas Hindborg]

Andreas Hindborg <a.hindborg@kernel.org> writes:

> "Alice Ryhl" <aliceryhl@google.com> writes:
>
>> On Tue, May 12, 2026 at 10:39:57AM +0200, Andreas Hindborg wrote:
>>> Andreas Hindborg <a.hindborg@kernel.org> writes:
>>>
>>> > "Alice Ryhl" <aliceryhl@google.com> writes:
>>> >
>>> >> On Mon, Feb 16, 2026 at 11:26:11AM +0000, Alice Ryhl wrote:
>>> >>> On Mon, Feb 16, 2026 at 12:10:16PM +0100, Andreas Hindborg wrote:
>>> >>> > "Alice Ryhl" <aliceryhl@google.com> writes:
>>> >>> >
>>> >>> > > On Sun, Feb 15, 2026 at 09:27:17PM +0100, Andreas Hindborg wrote:
>>> >>> > >> Add methods to get a reference to the contained value or populate the
>>> >>> > >> SetOnce if empty. The new `as_ref_or_populate` method accepts a value
>>> >>> > >> directly, while `as_ref_or_populate_with` accepts a fallible closure,
>>> >>> > >> allowing for lazy initialization that may fail. Both methods spin-wait
>>> >>> > >> if another thread is concurrently initializing the container.
>>> >>> > >>
>>> >>> > >> Also add `populate_with` which takes a fallible closure and serves as
>>> >>> > >> the implementation basis for the other populate methods.
>>> >>> > >>
>>> >>> > >> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>>> >>> > >
>>> >>> > >> +    /// Get a reference to the contained object, or populate the [`SetOnce`]
>>> >>> > >> +    /// with the value returned by `callable` and return a reference to that
>>> >>> > >> +    /// object.
>>> >>> > >> +    pub fn as_ref_or_populate_with(&self, callable: impl FnOnce() -> Result<T>) -> Result<&T> {
>>> >>> > >> +        if !self.populate_with(callable)? {
>>> >>> > >> +            while self.init.load(Acquire) != 2 {
>>> >>> > >> +                core::hint::spin_loop();
>>> >>> > >> +            }
>>> >>> > >
>>> >>> > > We should not be implementing our own spinlocks.
>>> >>> >
>>> >>> > That is a great proverb. I'd be happy to receive a suggestion on an
>>> >>> > alternate approach for this particular context.
>>> >>>
>>> >>> You can add a spinlock to SetOnce. Like I mentioned previously [1],
>>> >>> support for waiting will require the addition of extra fields.
>>> >
>>> > Thanks, I'll be sure to take a look again.
>>>
>>> I took a look at this again. I think the structure will be less
>>> efficient if we use a spin lock.
>>>
>>> Initialization is now
>>>  - cmpxchg lock relaxed
>>>  - store pointer
>>>  - store lock release
>>>
>>> With a spin lock it will be
>>>  - lock acquire
>>>  - test pointer
>>>  - store pointer
>>>  - lock release
>>>
>>> All the other tests for initialization is now:
>>>  - load lock acquire
>>>  - load pointer
>>>
>>> They will become
>>>  - lock acquire
>>>  - load pointer
>>>  - test pointer
>>>  - lock release
>>>
>>>
>>> bit_spinlock does not make this any better. It just gives us 64 locks in
>>> a u64. It does not help us store state of the data structure
>>> (empty/populated).
>>>
>>> Do we want a less efficient data structure in order to gain benefits of
>>> lockdep and friends?
>>
>> I'm not just talking about lockdep. Your current implementation is wrong
>> in several other ways, for example:
>>
>> 1. Spinlocks must disable preemption.
>
> That is an easy fix.
>
>> 2. It doesn't fall back to a mutex under PREEMPT_RT.
>
> I don't know how to solve that, but I'm sure there is a way.
>
>>
>> And probably lots of other things. By using the kernel spinlock, you do
>> not have to worry about the long list of things spinlocks have to get
>> right.
>
> Nah, can't be that many things. But I get your point.

So, when messing around with this `SpinLock` business, I run into a
problem. `SetOnce::new` is `const` and has to be for the use in
`module_param` as well as for the use I have in `rnull` where I use
`SetOnce` in global scope:

  static SHARED_TAG_SET: SetOnce<Arc<TagSet<NullBlkDevice>>> = SetOnce::new();

I could use `global_lock` instead, but I'd rather not have the unsafe
initializer in my driver.

We could split `SetOnce` and `OnceLock` as you have previously
suggested, but that would still require some kind of unsafe to
initialize a global `OnceLock`.

Based on these observations, I think I should either drop this patch
entirely and use `global_lock!`, which I'd rather not, or we should find
a way to open code the spin lock in a way that is compatible with the
kernel in general.

We could yank the spinning functionality out of `SetOnce` into `Atomic`.
An `Atomic` with the ability to spin until a certain value is observed
could be a nice primitive. Or it could spin until a `Fn(T) -> bool` on
the observed value returns true.

Any alternatives?


Best regards,
Andreas Hindborg

Re: [PATCH] rust: sync: add lazy initialization methods to SetOnce

[Gary Guo]

On Tue May 12, 2026 at 9:07 AM BST, Andreas Hindborg wrote:
> Andreas Hindborg <a.hindborg@kernel.org> writes:
>
>> "Gary Guo" <gary@garyguo.net> writes:
>>
>>> On Sun Feb 15, 2026 at 8:27 PM GMT, Andreas Hindborg wrote:
>>>> Add methods to get a reference to the contained value or populate the
>>>> SetOnce if empty. The new `as_ref_or_populate` method accepts a value
>>>> directly, while `as_ref_or_populate_with` accepts a fallible closure,
>>>> allowing for lazy initialization that may fail. Both methods spin-wait
>>>> if another thread is concurrently initializing the container.
>>>>
>>>> Also add `populate_with` which takes a fallible closure and serves as
>>>> the implementation basis for the other populate methods.
>>>
>>> Hi Andreas, in an earlier call I mentioned that I'm working on getting SetOnce
>>> to work with pin-init, the capability of which I think is a superset of you have
>>> here.
>>>
>>> The API I have is
>>>
>>> impl<T> SetOnce<T> {
>>>     pub fn init<E>(&self, init: impl Init<T, E>) -> Result<&T, InitError<E>>;
>>>     pub fn pin_init<E>(self, Pin<&Self>, init: impl PinInit<T, E>) -> Result<&T, InitError<E>>;
>>> }
>>>
>>> To achieve what you need with a function, you can simply write:
>>>
>>>     set_once.init(pin_init::init_scope(your_fn))
>>>
>>> The patch that implement the API is here:
>>> https://github.com/nbdd0121/linux/commit/4aabdbcf20b11626c253f203745b1d55c37ab2ee
>>> in tree
>>> https://github.com/nbdd0121/linux/tree/lazy_revocable_nova_wip/
>>>
>>> which I haven't submitted to the list as the user side of this API isn't ready.
>>
>> I probably evicted that from cache.
>>
>> It looks like that could replace the `populate` in my patch? We would
>> still need the synchronization in `as_ref_or_populate`, right? (or
>> `as_ref_or_init` if you will)
>
> Looks like this is not ready yet. I think we can move forward with the
> current suggestion and then wire up pin-init when it lands.
>

Alvin's been carrying the patch in his series:
https://lore.kernel.org/rust-for-linux/20260326-b4-tyr-debugfs-v1-1-074badd18716@linux.dev/

Best,
Gary

Re: [PATCH] rust: sync: add lazy initialization methods to SetOnce

[Alice Ryhl]

On Tue, May 12, 2026 at 12:42:47PM +0200, Andreas Hindborg wrote:
> Andreas Hindborg <a.hindborg@kernel.org> writes:
> 
> > "Alice Ryhl" <aliceryhl@google.com> writes:
> >
> >> On Tue, May 12, 2026 at 10:39:57AM +0200, Andreas Hindborg wrote:
> >>> Andreas Hindborg <a.hindborg@kernel.org> writes:
> >>>
> >>> > "Alice Ryhl" <aliceryhl@google.com> writes:
> >>> >
> >>> >> On Mon, Feb 16, 2026 at 11:26:11AM +0000, Alice Ryhl wrote:
> >>> >>> On Mon, Feb 16, 2026 at 12:10:16PM +0100, Andreas Hindborg wrote:
> >>> >>> > "Alice Ryhl" <aliceryhl@google.com> writes:
> >>> >>> >
> >>> >>> > > On Sun, Feb 15, 2026 at 09:27:17PM +0100, Andreas Hindborg wrote:
> >>> >>> > >> Add methods to get a reference to the contained value or populate the
> >>> >>> > >> SetOnce if empty. The new `as_ref_or_populate` method accepts a value
> >>> >>> > >> directly, while `as_ref_or_populate_with` accepts a fallible closure,
> >>> >>> > >> allowing for lazy initialization that may fail. Both methods spin-wait
> >>> >>> > >> if another thread is concurrently initializing the container.
> >>> >>> > >>
> >>> >>> > >> Also add `populate_with` which takes a fallible closure and serves as
> >>> >>> > >> the implementation basis for the other populate methods.
> >>> >>> > >>
> >>> >>> > >> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> >>> >>> > >
> >>> >>> > >> +    /// Get a reference to the contained object, or populate the [`SetOnce`]
> >>> >>> > >> +    /// with the value returned by `callable` and return a reference to that
> >>> >>> > >> +    /// object.
> >>> >>> > >> +    pub fn as_ref_or_populate_with(&self, callable: impl FnOnce() -> Result<T>) -> Result<&T> {
> >>> >>> > >> +        if !self.populate_with(callable)? {
> >>> >>> > >> +            while self.init.load(Acquire) != 2 {
> >>> >>> > >> +                core::hint::spin_loop();
> >>> >>> > >> +            }
> >>> >>> > >
> >>> >>> > > We should not be implementing our own spinlocks.
> >>> >>> >
> >>> >>> > That is a great proverb. I'd be happy to receive a suggestion on an
> >>> >>> > alternate approach for this particular context.
> >>> >>>
> >>> >>> You can add a spinlock to SetOnce. Like I mentioned previously [1],
> >>> >>> support for waiting will require the addition of extra fields.
> >>> >
> >>> > Thanks, I'll be sure to take a look again.
> >>>
> >>> I took a look at this again. I think the structure will be less
> >>> efficient if we use a spin lock.
> >>>
> >>> Initialization is now
> >>>  - cmpxchg lock relaxed
> >>>  - store pointer
> >>>  - store lock release
> >>>
> >>> With a spin lock it will be
> >>>  - lock acquire
> >>>  - test pointer
> >>>  - store pointer
> >>>  - lock release
> >>>
> >>> All the other tests for initialization is now:
> >>>  - load lock acquire
> >>>  - load pointer
> >>>
> >>> They will become
> >>>  - lock acquire
> >>>  - load pointer
> >>>  - test pointer
> >>>  - lock release
> >>>
> >>>
> >>> bit_spinlock does not make this any better. It just gives us 64 locks in
> >>> a u64. It does not help us store state of the data structure
> >>> (empty/populated).
> >>>
> >>> Do we want a less efficient data structure in order to gain benefits of
> >>> lockdep and friends?
> >>
> >> I'm not just talking about lockdep. Your current implementation is wrong
> >> in several other ways, for example:
> >>
> >> 1. Spinlocks must disable preemption.
> >
> > That is an easy fix.
> >
> >> 2. It doesn't fall back to a mutex under PREEMPT_RT.
> >
> > I don't know how to solve that, but I'm sure there is a way.
> >
> >>
> >> And probably lots of other things. By using the kernel spinlock, you do
> >> not have to worry about the long list of things spinlocks have to get
> >> right.
> >
> > Nah, can't be that many things. But I get your point.

And what about the quirks added to spinlocks in the future?

I'm not willing to put my name on a manual spinlock implementation.

> So, when messing around with this `SpinLock` business, I run into a
> problem. `SetOnce::new` is `const` and has to be for the use in
> `module_param` as well as for the use I have in `rnull` where I use
> `SetOnce` in global scope:
> 
>   static SHARED_TAG_SET: SetOnce<Arc<TagSet<NullBlkDevice>>> = SetOnce::new();
> 
> I could use `global_lock` instead, but I'd rather not have the unsafe
> initializer in my driver.
> 
> We could split `SetOnce` and `OnceLock` as you have previously
> suggested, but that would still require some kind of unsafe to
> initialize a global `OnceLock`.
> 
> Based on these observations, I think I should either drop this patch
> entirely and use `global_lock!`, which I'd rather not, or we should find
> a way to open code the spin lock in a way that is compatible with the
> kernel in general.

So it's true that putting synchronization primitives in global variables
is a real problem we don't have amazing solutions to right now. There
are various ways we can work around it. I came up with one workaround,
which is a single unsafe block to ensure that the global is initialized
in `init()`. You've come up with another workaround, which is a manual
spinlock.

My opinion is that the manual spinlock is far more problematic than the
unsafe block.

And I also think that as a principle we should avoid coming up with
several different work-arounds for the same problem.

> We could yank the spinning functionality out of `SetOnce` into `Atomic`.
> An `Atomic` with the ability to spin until a certain value is observed
> could be a nice primitive. Or it could spin until a `Fn(T) -> bool` on
> the observed value returns true.
> 
> Any alternatives?

Why do you need waiting for SHARED_TAG_SET to begin with? If you make
sure to initialize it as the very first thing in your module init, then
all other uses can just `unwrap()` the `SetOnce` because you know the
value is already there.

Alice

Re: [PATCH] rust: sync: add lazy initialization methods to SetOnce

[Andreas Hindborg]

"Alice Ryhl" <aliceryhl@google.com> writes:

> On Tue, May 12, 2026 at 12:42:47PM +0200, Andreas Hindborg wrote:
>> Andreas Hindborg <a.hindborg@kernel.org> writes:
>>
>> > "Alice Ryhl" <aliceryhl@google.com> writes:
>> >
>> >> On Tue, May 12, 2026 at 10:39:57AM +0200, Andreas Hindborg wrote:
>> >>> Andreas Hindborg <a.hindborg@kernel.org> writes:
>> >>>
>> >>> > "Alice Ryhl" <aliceryhl@google.com> writes:
>> >>> >
>> >>> >> On Mon, Feb 16, 2026 at 11:26:11AM +0000, Alice Ryhl wrote:
>> >>> >>> On Mon, Feb 16, 2026 at 12:10:16PM +0100, Andreas Hindborg wrote:
>> >>> >>> > "Alice Ryhl" <aliceryhl@google.com> writes:
>> >>> >>> >
>> >>> >>> > > On Sun, Feb 15, 2026 at 09:27:17PM +0100, Andreas Hindborg wrote:
>> >>> >>> > >> Add methods to get a reference to the contained value or populate the
>> >>> >>> > >> SetOnce if empty. The new `as_ref_or_populate` method accepts a value
>> >>> >>> > >> directly, while `as_ref_or_populate_with` accepts a fallible closure,
>> >>> >>> > >> allowing for lazy initialization that may fail. Both methods spin-wait
>> >>> >>> > >> if another thread is concurrently initializing the container.
>> >>> >>> > >>
>> >>> >>> > >> Also add `populate_with` which takes a fallible closure and serves as
>> >>> >>> > >> the implementation basis for the other populate methods.
>> >>> >>> > >>
>> >>> >>> > >> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> >>> >>> > >
>> >>> >>> > >> +    /// Get a reference to the contained object, or populate the [`SetOnce`]
>> >>> >>> > >> +    /// with the value returned by `callable` and return a reference to that
>> >>> >>> > >> +    /// object.
>> >>> >>> > >> +    pub fn as_ref_or_populate_with(&self, callable: impl FnOnce() -> Result<T>) -> Result<&T> {
>> >>> >>> > >> +        if !self.populate_with(callable)? {
>> >>> >>> > >> +            while self.init.load(Acquire) != 2 {
>> >>> >>> > >> +                core::hint::spin_loop();
>> >>> >>> > >> +            }
>> >>> >>> > >
>> >>> >>> > > We should not be implementing our own spinlocks.
>> >>> >>> >
>> >>> >>> > That is a great proverb. I'd be happy to receive a suggestion on an
>> >>> >>> > alternate approach for this particular context.
>> >>> >>>
>> >>> >>> You can add a spinlock to SetOnce. Like I mentioned previously [1],
>> >>> >>> support for waiting will require the addition of extra fields.
>> >>> >
>> >>> > Thanks, I'll be sure to take a look again.
>> >>>
>> >>> I took a look at this again. I think the structure will be less
>> >>> efficient if we use a spin lock.
>> >>>
>> >>> Initialization is now
>> >>>  - cmpxchg lock relaxed
>> >>>  - store pointer
>> >>>  - store lock release
>> >>>
>> >>> With a spin lock it will be
>> >>>  - lock acquire
>> >>>  - test pointer
>> >>>  - store pointer
>> >>>  - lock release
>> >>>
>> >>> All the other tests for initialization is now:
>> >>>  - load lock acquire
>> >>>  - load pointer
>> >>>
>> >>> They will become
>> >>>  - lock acquire
>> >>>  - load pointer
>> >>>  - test pointer
>> >>>  - lock release
>> >>>
>> >>>
>> >>> bit_spinlock does not make this any better. It just gives us 64 locks in
>> >>> a u64. It does not help us store state of the data structure
>> >>> (empty/populated).
>> >>>
>> >>> Do we want a less efficient data structure in order to gain benefits of
>> >>> lockdep and friends?
>> >>
>> >> I'm not just talking about lockdep. Your current implementation is wrong
>> >> in several other ways, for example:
>> >>
>> >> 1. Spinlocks must disable preemption.
>> >
>> > That is an easy fix.
>> >
>> >> 2. It doesn't fall back to a mutex under PREEMPT_RT.
>> >
>> > I don't know how to solve that, but I'm sure there is a way.
>> >
>> >>
>> >> And probably lots of other things. By using the kernel spinlock, you do
>> >> not have to worry about the long list of things spinlocks have to get
>> >> right.
>> >
>> > Nah, can't be that many things. But I get your point.
>
> And what about the quirks added to spinlocks in the future?
>
> I'm not willing to put my name on a manual spinlock implementation.
>
>> So, when messing around with this `SpinLock` business, I run into a
>> problem. `SetOnce::new` is `const` and has to be for the use in
>> `module_param` as well as for the use I have in `rnull` where I use
>> `SetOnce` in global scope:
>>
>>   static SHARED_TAG_SET: SetOnce<Arc<TagSet<NullBlkDevice>>> = SetOnce::new();
>>
>> I could use `global_lock` instead, but I'd rather not have the unsafe
>> initializer in my driver.
>>
>> We could split `SetOnce` and `OnceLock` as you have previously
>> suggested, but that would still require some kind of unsafe to
>> initialize a global `OnceLock`.
>>
>> Based on these observations, I think I should either drop this patch
>> entirely and use `global_lock!`, which I'd rather not, or we should find
>> a way to open code the spin lock in a way that is compatible with the
>> kernel in general.
>
> So it's true that putting synchronization primitives in global variables
> is a real problem we don't have amazing solutions to right now. There
> are various ways we can work around it. I came up with one workaround,
> which is a single unsafe block to ensure that the global is initialized
> in `init()`. You've come up with another workaround, which is a manual
> spinlock.
>
> My opinion is that the manual spinlock is far more problematic than the
> unsafe block.
>
> And I also think that as a principle we should avoid coming up with
> several different work-arounds for the same problem.
>
>> We could yank the spinning functionality out of `SetOnce` into `Atomic`.
>> An `Atomic` with the ability to spin until a certain value is observed
>> could be a nice primitive. Or it could spin until a `Fn(T) -> bool` on
>> the observed value returns true.
>>
>> Any alternatives?
>
> Why do you need waiting for SHARED_TAG_SET to begin with? If you make
> sure to initialize it as the very first thing in your module init, then
> all other uses can just `unwrap()` the `SetOnce` because you know the
> value is already there.

I guess I could just always initialize it. I'll do that for now and then
drop this patch.

My thought was that I did not want to initialize it at all, if it is not
going to be used. I think that is how the corresponding C code is
designed in the C null block driver.

I don't _need_ to build the Rust driver the same as the C one in this
particular case, but now it is annoying me slightly that we don't have a
safe way to implement similar logic.

I guess one option I could go for if I really want to lazy init with
safe code is to move the global out of global scope and into the module
struct. Then it can be initialized with a spin lock in it and I can have
something like `OnceLock`.

Best regards,
Andreas Hindborg