Re: [PATCH v14 07/15] atomic: Add atomic_cond_read_*_timeout()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] Missing `atomic_long_cond_read_*_timeout` wrappers, breaking API symmetry.

Pre-existing issues:
- [High] The atomic64_cond_read_*_timeout macros bypass generic_atomic64 spinlocks on 32-bit architectures, leading to torn reads.
--

--- Patch [7]: [PATCH v14 07/15] atomic: Add atomic_cond_read_*_timeout() ---
commit 51b6c1c793c6e910ea5b78db4aba0270bfcc54ff
Author: Ankur Arora <ankur.a.arora@oracle.com>

atomic: Add atomic_cond_read_*_timeout()

This patch adds atomic load wrappers, atomic_cond_read_*_timeout() and
atomic64_cond_read_*_timeout() for the cond-load timeout interfaces. It
also updates documentation to briefly describe these non-RMW interfaces.

> diff --git a/include/linux/atomic.h b/include/linux/atomic.h
> index 8dd57c3a99e9b..5bcb86e077845 100644
> --- a/include/linux/atomic.h
> +++ b/include/linux/atomic.h
> @@ -31,6 +31,16 @@
>  #define atomic64_cond_read_acquire(v, c) smp_cond_load_acquire(&(v)->counter, (c))
>  #define atomic64_cond_read_relaxed(v, c) smp_cond_load_relaxed(&(v)->counter, (c))
>  
> +#define atomic_cond_read_acquire_timeout(v, c, e, t) \
> +	smp_cond_load_acquire_timeout(&(v)->counter, (c), (e), (t))
> +#define atomic_cond_read_relaxed_timeout(v, c, e, t) \
> +	smp_cond_load_relaxed_timeout(&(v)->counter, (c), (e), (t))

[Severity: Low]
Does this also need to update scripts/atomic/gen-atomic-long.sh to generate
the atomic_long_cond_read_*_timeout variants?

The generator script explicitly hardcodes the mappings for the non-timeout
variants:

#define atomic_long_cond_read_acquire    atomic64_cond_read_acquire
#define atomic_long_cond_read_relaxed    atomic64_cond_read_relaxed

Without updating the script, developers attempting to use the timeout
variants with atomic_long_t will encounter undefined symbol errors.

> +
> +#define atomic64_cond_read_acquire_timeout(v, c, e, t) \
> +	smp_cond_load_acquire_timeout(&(v)->counter, (c), (e), (t))

[Severity: High]
This is a pre-existing issue, but does mapping atomic64_t to
smp_cond_load_acquire_timeout directly on &(v)->counter cause torn reads
on 32-bit architectures?

On 32-bit systems using CONFIG_GENERIC_ATOMIC64, native 64-bit atomics are
unavailable, so atomic64_t operations rely on a hashed spinlock array in
lib/atomic64.c.

Because smp_cond_load_acquire_timeout uses READ_ONCE() directly on the
counter address, it bypasses these spinlocks. A concurrent atomic64_set()
holding the spinlock could update the value while the lockless READ_ONCE()
splits the 64-bit read into two 32-bit loads.

If the resulting torn read falsely matches the wait condition, the thread
could prematurely unblock and enter a critical section before the data is
ready.

> +#define atomic64_cond_read_relaxed_timeout(v, c, e, t) \
> +	smp_cond_load_relaxed_timeout(&(v)->counter, (c), (e), (t))
> +
>  /*
>   * The idea here is to build acquire/release variants by adding explicit
>   * barriers on top of the relaxed variant. In the case where the relaxed

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714073041.40250-1-ankur.a.arora@oracle.com?part=7