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

Alice Ryhl
2025-11-07T09:03:03Z

On Thu, Nov 06, 2025 at 06:46:39AM -0800, Boqun Feng wrote:
> On Thu, Nov 06, 2025 at 09:10:26AM +0900, FUJITA Tomonori wrote:
> > 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,
>
> This is not true actually, clear_warn_once_set() uses memset() to zero
> the memory, which is usually implemented by kernel itself and at least
> indicates per-byte atomicity, so it totally works with other atomic
> accesses in the kernel memory model. Otherwise pr_*_once() will be
> considered as data races.
>
> > 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.
>
> I lean towards Andreas' suggestion that we should use SetOnce() here if
> possible. I was missing that before this reply, thank Andrea for bring
> it up.
>
> > 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.
> >
>
> Why? A lot of synchronization primitives have 0 as a valid value, no?
>
> > 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
>
> I don't think special structures are required or it's already
> implemented in our Atomic type.
>
> > something like "probably print only once", which is a very simple use
> > case. So I'm not sure it's worth introducing something complicated.
> >
>
> That's my point (and probably also Andreas' point), we already has the
> type `SetOnce` to do this, no need for a `OnceLite` type if not
> necessary, and the fact that it can be zero'd by debugfs doesn't change
> it as I explained above.

The SetOnce type doesn't do the same thing as OnceLite. SetOnce has
three different states, but OnceLite only needs two. I don't think we
should be reusing SetOnce here.

Alice