Re: [PATCH v2] rust: sync: set_once: Implement Send and Sync

[Alice Ryhl <aliceryhl@google.com>]

On Thu, Dec 18, 2025 at 06:10:37AM +0900, Boqun Feng wrote:
> On Wed, Dec 17, 2025 at 08:45:35PM +0000, Alice Ryhl wrote:
> > On Tue, Dec 16, 2025 at 09:09:01AM +0900, FUJITA Tomonori wrote:
> > > Implement Send and Sync for SetOnce<T> to allow it to be used across
> > > thread boundaries.
> > > 
> > > Send: SetOnce<T> can be transferred across threads when T: Send, as
> > > the contained value is also transferred and will be dropped on the
> > > destination thread.
> > > 
> > > Sync: SetOnce<T> can be shared across threads when T: Sync, as
> > > as_ref() provides shared references &T and atomic operations ensure
> > > proper synchronization. Since the inner T may be dropped on any
> > > thread, we also require T: Send.
> > > 
> > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> > > ---
> > > v2:
> > >   - update the `Sync` impl to require `T: Send + Sync` with the comment adjusted
> > > v1: https://lore.kernel.org/rust-for-linux/20251211230919.1303926-2-fujita.tomonori@gmail.com/
> > > 
> > >  rust/kernel/sync/set_once.rs | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/rust/kernel/sync/set_once.rs b/rust/kernel/sync/set_once.rs
> > > index bdba601807d8..139cef05e935 100644
> > > --- a/rust/kernel/sync/set_once.rs
> > > +++ b/rust/kernel/sync/set_once.rs
> > > @@ -123,3 +123,11 @@ fn drop(&mut self) {
> > >          }
> > >      }
> > >  }
> > > +
> > > +// SAFETY: `SetOnce` can be transferred across thread boundaries iff the data it contains can.
> > > +unsafe impl<T: Send> Send for SetOnce<T> {}
> > > +
> > > +// SAFETY: `SetOnce` synchronises access to the inner value via atomic operations,
> > > +// so shared references are safe when `T: Sync`. Since the inner `T` may be dropped
> > > +// on any thread, we also require `T: Send`.
> > > +unsafe impl<T: Send + Sync> Sync for SetOnce<T> {}
> > 
> > The reason you need Sync is because &SetOnce<T> allows you to access &T.
> > It's not because atomic operations are used.
> > 
> 
> I think the point here is that atomic operations contribute to the
> safety of sharing &SetOnce<T> between threads, if the synchronization
> part of SetOnce is not done via atomics (or any other synchronization),
> then SetOnce is not Sync. In other words, making `SetOnce<T>: Sync`
> requires all the following:
> 
> * The internal operation is proper synchronized
> * T is Sync because a `&T` can be derived from `&SetOnce<T>`
> * T is Send because copy() may create a `T` on another thread.
> 
> (hmm... the third condition does looks weird to me, because if `T` is
> `Sync` and `Copy`, the copy on `T` should guarante the drop of the
> copied `T` is safe, otherwise `T` itself is not `Sync`. Gary, do you
> have an example in mind that requires `T` being `Send` for `SetOnce<T>`
> being `Sync`?)

I guess in general safety comments for Send/Sync are a bit tricky ...
you really need to argue that the entire API is designed correctly to
show that having &SetOnce<T> on multiple threads in parallel is sound
(in the case of Sync).

Alice