[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

===================>