[PATCH] locking/atomics: Combine the atomic_andnot() and atomic64_andnot() API definitions
From: Ingo Molnar
Date: Sat May 05 2018 - 04:55:01 EST
* Ingo Molnar <mingo@xxxxxxxxxx> wrote:
> Note that the simplest definition block is now:
>
> #ifndef atomic_cmpxchg_relaxed
> # define atomic_cmpxchg_relaxed atomic_cmpxchg
> # define atomic_cmpxchg_acquire atomic_cmpxchg
> # define atomic_cmpxchg_release atomic_cmpxchg
> #else
> # ifndef atomic_cmpxchg
> # define atomic_cmpxchg(...) __atomic_op_fence(atomic_cmpxchg, __VA_ARGS__)
> # define atomic_cmpxchg_acquire(...) __atomic_op_acquire(atomic_cmpxchg, __VA_ARGS__)
> # define atomic_cmpxchg_release(...) __atomic_op_release(atomic_cmpxchg, __VA_ARGS__)
> # endif
> #endif
>
> ... which is very readable!
>
> The total linecount reduction of the two patches is pretty significant as well:
>
> include/linux/atomic.h | 1063 ++++++++++++++++--------------------------------
> 1 file changed, 343 insertions(+), 720 deletions(-)
BTW., I noticed two asymmetries while cleaning up this code:
==============>
#ifdef atomic_andnot
#ifndef atomic_fetch_andnot_relaxed
# define atomic_fetch_andnot_relaxed atomic_fetch_andnot
# define atomic_fetch_andnot_acquire atomic_fetch_andnot
# define atomic_fetch_andnot_release atomic_fetch_andnot
#else
# ifndef atomic_fetch_andnot
# define atomic_fetch_andnot(...) __atomic_op_fence(atomic_fetch_andnot, __VA_ARGS__)
# define atomic_fetch_andnot_acquire(...) __atomic_op_acquire(atomic_fetch_andnot, __VA_ARGS__)
# define atomic_fetch_andnot_release(...) __atomic_op_release(atomic_fetch_andnot, __VA_ARGS__)
# endif
#endif
#endif /* atomic_andnot */
...
#ifdef atomic64_andnot
#ifndef atomic64_fetch_andnot_relaxed
# define atomic64_fetch_andnot_relaxed atomic64_fetch_andnot
# define atomic64_fetch_andnot_acquire atomic64_fetch_andnot
# define atomic64_fetch_andnot_release atomic64_fetch_andnot
#else
# ifndef atomic64_fetch_andnot
# define atomic64_fetch_andnot(...) __atomic_op_fence(atomic64_fetch_andnot, __VA_ARGS__)
# define atomic64_fetch_andnot_acquire(...) __atomic_op_acquire(atomic64_fetch_andnot, __VA_ARGS__)
# define atomic64_fetch_andnot_release(...) __atomic_op_release(atomic64_fetch_andnot, __VA_ARGS__)
# endif
#endif
#endif /* atomic64_andnot */
<==============
Why do these two API groups have an outer condition, i.e.:
#ifdef atomic_andnot
...
#endif /* atomic_andnot */
...
#ifdef atomic64_andnot
...
#endif /* atomic64_andnot */
because the base APIs themselves are optional and have a default implementation:
#ifndef atomic_andnot
...
#endif
...
#ifndef atomic64_andnot
...
#endif
I think it's overall cleaner if we combine them into continous blocks, defining
all variants of an API group in a single place:
#ifdef atomic_andnot
#else
#endif
etc.
The patch below implements this.
Thanks,
Ingo
===================>