linux-kernel - Re: [PATCH v14 1/7] rust: sync: add `OnceLock`

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87sejdteei.fsf@kernel.org>
Date: Thu, 03 Jul 2025 18:41:57 +0200
From: Andreas Hindborg <a.hindborg@...nel.org>
To: "Wren Turkal" <wt@...guintechs.org>
Cc: "Miguel Ojeda" <ojeda@...nel.org>,  "Alex Gaynor"
 <alex.gaynor@...il.com>,  "Boqun Feng" <boqun.feng@...il.com>,  "Gary Guo"
 <gary@...yguo.net>,  Björn Roy Baron
 <bjorn3_gh@...tonmail.com>,  "Alice
 Ryhl" <aliceryhl@...gle.com>,  "Masahiro Yamada" <masahiroy@...nel.org>,
  "Nathan Chancellor" <nathan@...nel.org>,  "Luis Chamberlain"
 <mcgrof@...nel.org>,  "Danilo Krummrich" <dakr@...nel.org>,  "Benno
 Lossin" <lossin@...nel.org>,  "Nicolas Schier" <nicolas.schier@...ux.dev>,
  "Trevor Gross" <tmgross@...ch.edu>,  "Adam Bratschi-Kaye"
 <ark.email@...il.com>,  <rust-for-linux@...r.kernel.org>,
  <linux-kernel@...r.kernel.org>,  <linux-kbuild@...r.kernel.org>,  "Petr
 Pavlu" <petr.pavlu@...e.com>,  "Sami Tolvanen" <samitolvanen@...gle.com>,
  "Daniel Gomez" <da.gomez@...sung.com>,  "Simona Vetter"
 <simona.vetter@...ll.ch>,  "Greg KH" <gregkh@...uxfoundation.org>,  "Fiona
 Behrens" <me@...enk.dev>,  "Daniel Almeida"
 <daniel.almeida@...labora.com>,  <linux-modules@...r.kernel.org>
Subject: Re: [PATCH v14 1/7] rust: sync: add `OnceLock`

"Wren Turkal" <wt@...guintechs.org> writes:

> On 7/2/25 6:18 AM, Andreas Hindborg wrote:

[...]

>> +pub struct OnceLock<T> {
>> +    init: Atomic<u32>,
>> +    value: Opaque<T>,
>> +}
>
> This type looks very much like the Once type in rust's stdlib. I am
> wondering if the api could be changed to match that api. I know that
> this type is trying to provide a version subset of std::sync::OnceLock
> that doesn't allow resetting the type like these apis:
>
> * https://doc.rust-lang.org/std/sync/struct.OnceLock.html#method.get_mut
> * https://doc.rust-lang.org/std/sync/struct.OnceLock.html#method.take
>
> However, these methods can only be used on mut. See here for failing
> example:
> https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=a78e51203c5b9555e3c151e162f0acab
>
> I think it might make more sense to match the api of the stdlib API and
> maybe only implement the methods you need.

I agree, it would be nice to match the names to std. But I do not like
that they have `OnceLock::set`, `OnceLock::try_init` and
`OnceLock::get_or{_try}_init`. Why is it not `OnceLock::init` or
`OnceLock::try_set`?

>
>> +
>> +impl<T> Default for OnceLock<T> {
>> +    fn default() -> Self {
>> +        Self::new()
>> +    }
>> +}
>
> Any reason not to use #[derive(Default)]?

We don't have `Default` for neither `Atomic` or `Opaque`.

[...]

> Might also be worth implementing get_or_{try,}init, which get the value
> while initializing.

I did not have a user for those, so not adding for now. It would be dead
code. They should be fairly straight forward to add though.

>
>> +        // INVARIANT: We obtain exclusive access to the contained allocation and write 1 to
>> +        // `init`.
>> +        if let Ok(0) = self.init.cmpxchg(0, 1, Acquire) {
>> +            // SAFETY: We obtained exclusive access to the contained object.
>> +            unsafe { core::ptr::write(self.value.get(), value) };
>> +            // INVARIANT: We release our exclusive access and transition the object to shared
>> +            // access.
>> +            self.init.store(2, Release);
>> +            true
>> +        } else {
>> +            false
>> +        }
>> +    }
>> +}
>> +
>> +impl<T: Copy> OnceLock<T> {
>> +    /// Get a copy of the contained object.
>> +    ///
>> +    /// Returns [`None`] if the [`OnceLock`] is empty.
>> +    pub fn copy(&self) -> Option<T> {
>
> No equivalent in OnceLock. Similar to something like this:
>
> x.get().copied().unwrap(); // x is a OnceLock
>
> Example:
> https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=f21068e55f73722544fb5ad341bce1c5
>
> Maybe not specifically needed?

I don't actually have a user for this, so I think I will drop it.


Best regards,
Andreas Hindborg