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

Boqun Feng
2025-11-13T15:20:35Z

On Thu, Nov 13, 2025 at 08:18:44PM +0900, FUJITA Tomonori wrote:
> On Thu, 13 Nov 2025 10:07:36 +0000
> Alice Ryhl <aliceryhl@google.com> wrote:
>
> > On Wed, Nov 05, 2025 at 02:47:29PM +0900, FUJITA Tomonori wrote:
> >> 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.
> >>
> >> [1] https://lore.kernel.org/rust-for-linux/20241126-pr_once_macros-v4-0-410b8ca9643e@tuta.io/
> >
> > There has been a fair bit of discussion below. Here is my take on the
> > way forward:
> >

Thank Alice for putting together the summary of the discussion, add
Peter Cced as well. For context, Peter, this is about having the
pr_*_once() on Rust side. Also Alice has a reply to Tomo, but I'm
copying this to you because it contains more context.

> > 1. OnceLite should be it's own special type, and it should be in a
> > printing-related module, not kernel::sync. The reason for this is
> > that do_once_lite! is hooked up to a special section that allows you
> > to reset the once status, and if someone used it for anything not
> > related to printing, they would end up with being incorrectly reset
> > when someone resets pr_warn_once calls.
>
> Do you mean that only the do_once_lite! macro, which places the data
> in .data..once section, should live in a printing-related module? Or
> should everything including OnceLite struct itself, also be moved
> there?
>

I know Alice mentioned in a reply that she prefers to moving both, but I
think with a reset() function [1] added, it's totally fine to keep
OnceLite in kernel::sync and make do_once_lite!() an internal macro
(i.e. not exposed) for pr_*_once!() only. But let's first focus on more
important differences below.

> > 2. I would suggest just using a relaxed load followed by store instead
> > of xchg. This is what the C-side does, and I think there is no strong
> > reason to deviate. It allows for a single byte of data instead of i32.
>
> You meant best-effort try-once logic like C?
>
> pub fn call_once<F>(&self, f: F) -> bool
> where
> F: FnOnce(),
> {
> if self.0.load(Relaxed) == State::Incomplete {
> self.0.store(State::Complete, Relaxed);
> f();
> return true;
> }
> false
> }
>

I understand Alice's point, but personally, I think the C's
DO_ONCE_LITE() was implemented wrongly, because it DOES NOT guarantee
"do something once", hence I suggested we don't repeat that logic in the
Rust version.

Speak of personal experience, sometimes I do need to provide an
educational guess of "what's going on" with only one copy of a kernel
log, and I think I'm not alone. For that purpose, the uncertainty of "it
may print more than once" does matter. My point is more about since it's
a logging mechanism, it better be accurate than efficient because the
logging itself would take more time and space.

> Using u8 as atomic type instead of i32 would require a fair amount of
> work, since at the moment only i32 and i64 are supported as atomic
> types, right?
>

I agree with Alice in the other reply that u8 store and load (even
xchg() and cmpxchg()) are not hard to add. However, depending on which
logic we decide to use:

1. If we use the current C logic (i.e. no xchg() and it might do things
more than once), then using u8 is fine and saves spaces.

2. If we use the correct "doing once" logic, that means we have to use
xchg() and we need to notice that xchg() on certain architectures
(RISC for example), will take more than 1 instruction on u8.
Therefore although we save the 3 bytes by using u8 instead of u32, we
will spend 4 more instructions. That's something to take into
consideration. Maybe u8/u32 can be an architecture-specific thing.
But we can push it as a future patch.

>
> > 3. If we *do* decide to use xchg, then the xchg needs to be guarded by a
> > load so that we avoid a write operation when it's already cleared.
> > Many read ops are much cheaper than many read/write ops.
>
> Make sense.

Agreed.

Regards,
Boqun

>

[1]: https://lore.kernel.org/rust-for-linux/aRPvjQJLdzvxLUpr@tardis.local/