Re: [PATCH v1 0/2] Add support for print exactly once

[Boqun Feng <boqun.feng@gmail.com>]

On Wed, Nov 12, 2025 at 09:45:08AM +0900, FUJITA Tomonori wrote:
> On Tue, 11 Nov 2025 10:02:46 +0100
> Andreas Hindborg <a.hindborg@kernel.org> wrote:
> 
> > Boqun Feng <boqun.feng@gmail.com> writes:
> > 
> >> On Mon, Nov 10, 2025 at 01:16:44PM +0100, Andreas Hindborg wrote:
> >>> Boqun Feng <boqun.feng@gmail.com> writes:
> >>> 
> >>> > On Thu, Nov 06, 2025 at 08:12:31AM +0900, FUJITA Tomonori wrote:
> >>> >> On Wed, 05 Nov 2025 21:59:06 +0100
> >>> >> Andreas Hindborg <a.hindborg@kernel.org> wrote:
> >>> >> 
> >>> >> > "FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:
> >>> >> > 
> >>> >> >> This adds the Rust equivalent of the kernel's DO_ONCE_LITE and
> >>> >> >> pr_*_once macros.
> >>> >> >>
> >>> >> >> A proposal for this feature was made in the past [1], but it didn't
> >>> >> >> reach consensus on the implementation and wasn't merged. After reading
> >>> >> >> the previous discussions, I implemented it using a different approach.
> >>> >> >>
> >>> >> >> In the previous proposal, a structure equivalent to std::sync::Once
> >>> >> >> was implemented to realize the DO_ONCE_LITE macro. The approach tried
> >>> >> >> to provide Once-like semantics by using two atomic values. As pointed
> >>> >> >> out in the previous review comments, I think this approach tries to
> >>> >> >> provide more functionality than needed, making it unnecessarily
> >>> >> >> complex. Also, because data structures in the .data..once section can
> >>> >> >> be cleared at any time (via debugfs clear_warn_once), an
> >>> >> >> implementation using two atomics wouldn't work correctly.
> >>> >> >>
> >>> >> >> Therefore, I decided to drop the idea of emulating Once and took a
> >>> >> >> minimal approach to implement DO_ONCE_LITE with only one atomic
> >>> >> >> variable. While it would be possible to implement the feature entirely
> >>> >> >> as a Rust macro, the functionality that can be implemented as regular
> >>> >> >> functions has been extracted and implemented as the OnceLite struct
> >>> >> >> for better code readability.
> >>> >> >>
> >>> >> >> Of course, unlike the previous proposal, this uses LKMM atomics.
> >>> >> > 
> >>> >> > Please consider if it makes sense to base this on `SetOnce`. It is in
> >>> >> > linux-next now, but was on list here [1].
> >>> >> 
> >>> >> Data placed in the .data..once section can be zero-cleared when a user
> >>> >> writes to debugfs clear_warn_once. In that case, would such data still
> >>> >> be considered a valid SetOnce value?
> >>> >> 
> >>> >
> >>> > It's still a valid value I believe. In term of data races, Rust and C
> >>> > have no difference, so if writing to debugfs could cause issues in Rust,
> >>> > it would cause issues in C as well.
> >>> 
> >>> @Tomo you are right, `SetOnce` would not work with someone (debugfs)
> >>> asynchronously modifying the state atomic variable. It requires
> >>> exclusive access while writing the contained value.
> >>> 
> >>
> >> I mean if we were to use `SetOnce` in pr_*_once(), we should just use
> >> `SetOnce<()>`, and the problem you mentioned doesn't exist in this case.
> > 
> > At the very least it would break the type invariants and assumptions of
> > the `SetOnce` type. There a race condition between `SetOnce::as_ref` and
> > `SetOnce::populate`. I don't think we are allowed to race like this,
> > even if the type is `()`?
> > 
> > At any rate if we do this, we should update the safety invariant of
> > `SetOnce` to state that it is valid to revert the type back to the
> > initial state for zero sized types that do not have a `Drop`
> > implementation.
> 
> I agree. I think that even if the type is (), a race condition would
> still occur if the init member were reset to zero using non-atomic
> operations. For instance, the as_ref() method effectively checks

I already explained this, the debugfs writes have to be treated as
per-byte atomic, otherwise it's data race, using "non-atomic" to
reference that won't be accurate.

> whether a value has been set, so for the () type, it would return
> either Ok(()) or None.
> 
> Although SetOnce is designed to atomically set a value, allowing the
> data to be modified by non-atomic operations at arbitrary times would,
> as I mentioned in another thread, unnecessarily complicate its
> implementation for a feature that would not be useful in other use
> cases.
> 
> Could the root cause of the issue be that OnceLite, which isn't
> updated atomically, is implemented using atomic operations?
> 
> It might be better to implement it with non-atomic operations, as in
> the C version, as that would make the update behavior clearer.
> 

No, that's introducing more problems and inheriting bad things from C.

I would say we can probably implement an OnceFlag type (similiar to
OnceLite but also support cmpxchg so that SetOnce::populate() can use)
and use it to implement SetOnce. Then you can avoid violating SetOnce's
type invariants that Andreas mentioned.

Regards,
Boqun

> What do you think?
>