Re: [PATCH v2 3/4] rust: sync: atomic: Add i8/i16 load and store support

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

On Wed, Dec 10, 2025 at 08:31:47AM +0900, FUJITA Tomonori wrote:
> On Tue, 09 Dec 2025 09:27:25 +0900
> "Alexandre Courbot" <acourbot@nvidia.com> wrote:
> 
> >>> Can you document this macro a bit, in particular the motivations for not
> >>> leveraging the existing ones (I guess this has to be with the new
> >>> helpers, but following the code through is a bit difficult without
> >>> comments).
> >>
> >> // Since i8 and i16 are not expected to support the full feature set
> >> // of i32 and i64, using the current declare_and_impl_atomic_methods!
> >> // would require refactoring it to handle specific types or splitting
> >> // the definitions. Furthermore, supporting Atomic<bool> will require
> >> // even more significant structural changes.
> >> //
> >> // To avoid premature refactoring, a separate macro for i8 and i16 is
> >> // used for now, leaving the existing macros untouched until the overall
> >> // design requirements are settled.
> >>
> >> makes sense?
> > 
> > Absolutely, thanks! Maybe also give a short explanation for *why* i8 and
> > i16 won't support the same feature set as i32 and i64.
> 
> I could add, though Boqun might have a different perspective.
> 

Yeah, I think it's more of an "undecided" stage. The reasons that why we
want to avoid the same feature as i32 and i64 for i8/i16 are:

1. For some architectures, 1-byte or 2-byte atomic RmW
   (read-modify-write) operations are not supported, they would generate
   worse binary than designing the algorithm with Atomic<i32/i64>.

2. For some architectures, atomics RmW are implemented by lock-based
   operations on C side, in that case READ_ONCE()/WRITE_ONCE() based
   load/store implementations are incorrect, because it would race with
   lock-based RmW for example.

   atomic_add_unless(v, 1, 0);
    lock();
    ret = READ_ONCE(v->counter); // == 1
						atomic_set(v, 0);
    if (ret != u)				  WRITE_ONCE(v->counter, 0);
      WRITE_ONCE(v->counter, ret + 1);
    unlock();

    Hence, we need to make a design decision here, either 1) we make
    Atomic<i8> support RmW and Atomic::<i8>::load() may contain locking
    on certain architecture (that also means you cannot use it as
    READ_ONCE()), or 2) we disallow RmW for Atomic<i8> if they use
    lock-based atomic.

I feel we are not having the enough data to make the call here, 
therefore I would avoid premature conclusion in the documentation.

Thanks!

Regards,
Boqun

> In my opinion, the reason is that the C side doesn't fully support
> atomic operations for i8 and i16. While the C side doesn't support
> load and store for i8/i16, we can easily support them with simple
> wrappers (as this patchset does). I believe that xchg and cmpxchg
> could also be supported easily.
> 
> However, supporting AtomicArithmeticOps (like fetch_add) for i8/i16
> would likely require more than wrappers; architecture-dependent code
> (likely with assembly). I don't think that it's a good idea to
> implement such.
> 
> 
> My plan is to add supoprt for limited atomic operations for i8/i16,
> and then use the i8 support to implement Atomic<bool>.
> 
> Once that is in place, I intend to replace the Atomic<i32> usage in
> SetOnce and OnceLite [1]. I believe the AtomicBool usage in the
> binder driver could be replaced too.
> 
> [1] https://lore.kernel.org/rust-for-linux/20251117002452.4068692-1-fujita.tomonori@gmail.com/