Re: [PATCH RFC 02/11] rust: sync: atomic: Add Atomic<u{8,16}>

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] rust: test coverage regression: u32 dropped from atomic_arithmetic_tests
- [Low] rust: dead code: unusable AtomicAdd implementations for u8 and u16
--

commit bff46a0936305602d6b5e98e2a29f7e77f8ccefc
Author: Thomas Weißschuh <linux@weissschuh.net>

rust: sync: atomic: Add Atomic<u{8,16}>
    
Add generic atomic support for small unsigned integers that have an
AtomicImpl with the same size and alignment. Also extend the unit tests.

> diff --git a/rust/kernel/sync/atomic/predefine.rs b/rust/kernel/sync/atomic/predefine.rs
> index 3d63f40791fa2..4c1fb5e08dc26 100644
> --- a/rust/kernel/sync/atomic/predefine.rs
> +++ b/rust/kernel/sync/atomic/predefine.rs
> @@ -113,6 +113,32 @@ fn rhs_into_delta(rhs: isize) -> isize_atomic_repr {
>      }
>  }
>  
> +// SAFETY: `u8` and `i8` has the same size and alignment, and `u8` is round-trip transmutable to
> +// `i8`.
> +unsafe impl super::AtomicType for u8 {
> +    type Repr = i8;
> +}
> +
> +// SAFETY: The wrapping add result of two `i8`s is a valid `u8`.
> +unsafe impl super::AtomicAdd<u8> for u8 {
> +    fn rhs_into_delta(rhs: u8) -> i8 {
> +        rhs as i8
> +    }
> +}

[Severity: Low]
Does this implementation for u8 result in unusable dead code?

Since the Repr type for u8 is i8, and i8 lacks an AtomicArithmeticOps
implementation, it seems impossible to call arithmetic methods like fetch_add
or add on Atomic<u8>.

> +// SAFETY: `u16` and `i16` has the same size and alignment, and `u16` is round-trip transmutable to
> +// `i16`.
> +unsafe impl super::AtomicType for u16 {
> +    type Repr = i16;
> +}
> +
> +// SAFETY: The wrapping add result of two `i16`s is a valid `u16`.
> +unsafe impl super::AtomicAdd<u16> for u16 {
> +    fn rhs_into_delta(rhs: u16) -> i16 {
> +        rhs as i16
> +    }
> +}

[Severity: Low]
Does this implementation for u16 also result in unusable dead code?

Similar to u8, the Repr type i16 lacks an AtomicArithmeticOps implementation,
preventing the use of atomic arithmetic methods on Atomic<u16>.

[ ... ]

> @@ -263,7 +289,7 @@ fn atomic_cmpxchg_tests() {
>  
>      #[test]
>      fn atomic_arithmetic_tests() {
> -        for_each_type!(42 in [i32, i64, u32, u64, isize, usize] |v| {
> +        for_each_type!(42 in [i32, i64, u64, isize, usize] |v| {

[Severity: Low]
Is u32 intentionally dropped from the list of types tested here?

This appears to cause a regression in test coverage for 32-bit unsigned atomic
arithmetic operations.

>              let x = Atomic::new(v);
>  
>              assert_eq!(v, x.fetch_add(12, Full));

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011-rust-dyndbg-v1-0-832e4039b71f@weissschuh.net?part=2