Re: [PATCH v1 1/2] rust: Add support for calling a function exactly once

FUJITA Tomonori
2025-11-06T00:10:30Z

On Wed, 5 Nov 2025 08:19:37 -0800
Boqun Feng <boqun.feng@gmail.com> wrote:

>> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
>> index 3dd7bebe7888..19553eb8c188 100644
>> --- a/rust/kernel/lib.rs
>> +++ b/rust/kernel/lib.rs
>> @@ -110,6 +110,7 @@
>> #[cfg(CONFIG_NET)]
>> pub mod net;
>> pub mod of;
>> +pub mod once_lite;
>
> Would it make more sense to put it the kernel::sync module?

I actually considered that as well. The OnceLite structure could
probably live under the sync module.

However, the do_once_lite macro places its data in the .data..once
section, which can be zero-cleared when a user writes to debugfs
clear_warn_once. In that case, there is no guarantee of any atomicity,
so it doesn't really fit the semantics expected for the sync module.

For now, OnceLite the only used by do_once_lite macro, so I didn't see
much benefit in splitting it into separate files.
Also, data placed in the .data..once section should ideally be of a
type whose zero-cleared state is clearly valid, which makes it
doubtful that OnceLite would be generally useful in the way that other
synchronization primitives in sync are.

From a Rust perspective, data that is shared with the C side and can
be zero-cleared at any time might ideally require some special
structures? However, what we actually want to achieve here is simply
something like "probably print only once", which is a very simple use
case. So I'm not sure it's worth introducing something complicated.

>> +impl OnceLite {
>> + /// Creates a new [`OnceLite`] in the incomplete state.
>> + #[inline(always)]
>> + #[allow(clippy::new_without_default)]
>> + pub const fn new() -> Self {
>> + OnceLite(Atomic::new(State::Incomplete))
>> + }
>> +
>> + /// Calls the provided function exactly once.
>
> I think a few more comments here won't hurt:
>
> /// There is no other synchronization between two `call_once()`s
> /// except that only one will execute `f`, in other words, callers
> /// should not use a failed `call_once()` as a proof that another
> /// `call_once()` has already finished and the effect is observable
> /// to this thread.
>
> Thoughts?

I don't expect OnceLite to be used in cases where it matters whether
another thread has completed call_once(), but adding the above comment
wouldn't hurt, I think.

>> + pub fn call_once<F>(&self, f: F) -> bool
>> + where
>> + F: FnOnce(),
>> + {
>> + let old = self.0.xchg(State::Complete, Relaxed);
>
> And we probably want a // ORDERING: comment here to explain why
> `Relaxed` is used here (because of the semantics mentioned above in the
> comment).

What kind of comment do you think would be appropriate here?