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

[Andreas Hindborg <a.hindborg@kernel.org>]

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