Re: [Xen-devel] [PATCH v2 4/5] asm/atomic.h: common prototyping (add xen/atomic.h)

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 13 Jul 2016, Corneliu ZUZU wrote:
> Create a common-side <xen/atomic.h> to establish, among others, prototypes of
> atomic functions called from common-code. Done to avoid introducing
> inconsistencies between arch-side <asm/atomic.h> headers when we make subtle
> changes to one of them.
> 
> Some arm-side macros had to be turned into inline functions in the process.
> Removed outdated comment ("NB. I've [...]").
> 
> Signed-off-by: Corneliu ZUZU <czuzu@xxxxxxxxxxxxxxx>
> Suggested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> Changed since v1:
>   * removed comments that were duplicate between asm-x86/atomic.h and
>     xen/atomic.h
>   * remove outdated comment ("NB. [...]")
>   * add atomic_cmpxchg doc-comment
>   * don't use yoda condition
> ---
>  xen/include/asm-arm/atomic.h |  45 ++++++++----
>  xen/include/asm-x86/atomic.h | 103 +-------------------------
>  xen/include/xen/atomic.h     | 171 
> +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 202 insertions(+), 117 deletions(-)
>  create mode 100644 xen/include/xen/atomic.h
> 
> diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
> index e8f7340..01af43b 100644
> --- a/xen/include/asm-arm/atomic.h
> +++ b/xen/include/asm-arm/atomic.h
> @@ -2,6 +2,7 @@
>  #define __ARCH_ARM_ATOMIC__
>  
>  #include <xen/config.h>
> +#include <xen/atomic.h>
>  #include <xen/prefetch.h>
>  #include <asm/system.h>
>  
> @@ -95,15 +96,6 @@ void __bad_atomic_size(void);
>      default: __bad_atomic_size(); break;                                \
>      }                                                                   \
>  })
> -    
> -/*
> - * NB. I've pushed the volatile qualifier into the operations. This allows
> - * fast accessors such as _atomic_read() and _atomic_set() which don't give
> - * the compiler a fit.
> - */
> -typedef struct { int counter; } atomic_t;
> -
> -#define ATOMIC_INIT(i) { (i) }
>  
>  /*
>   * On ARM, ordinary assignment (str instruction) doesn't clear the local
> @@ -141,12 +133,35 @@ static inline void _atomic_set(atomic_t *v, int i)
>  #define atomic_inc_return(v)        (atomic_add_return(1, v))
>  #define atomic_dec_return(v)        (atomic_sub_return(1, v))

What about atomic_inc_return and atomic_dec_return? Doesn't it make
sense to do this for all of them, since we are at it? I believe there
are also a couple more which are #define'd.


> -#define atomic_sub_and_test(i, v)   (atomic_sub_return(i, v) == 0)
> -#define atomic_inc(v)               atomic_add(1, v)
> -#define atomic_inc_and_test(v)      (atomic_add_return(1, v) == 0)
> -#define atomic_dec(v)               atomic_sub(1, v)
> -#define atomic_dec_and_test(v)      (atomic_sub_return(1, v) == 0)
> -#define atomic_add_negative(i,v)    (atomic_add_return(i, v) < 0)
> +static inline int atomic_sub_and_test(int i, atomic_t *v)
> +{
> +    return atomic_sub_return(i, v) == 0;
> +}
> +
> +static inline void atomic_inc(atomic_t *v)
> +{
> +    atomic_add(1, v);
> +}
> +
> +static inline int atomic_inc_and_test(atomic_t *v)
> +{
> +    return atomic_add_return(1, v) == 0;
> +}
> +
> +static inline void atomic_dec(atomic_t *v)
> +{
> +    atomic_sub(1, v);
> +}
> +
> +static inline int atomic_dec_and_test(atomic_t *v)
> +{
> +    return atomic_sub_return(1, v) == 0;
> +}
> +
> +static inline int atomic_add_negative(int i, atomic_t *v)
> +{
> +    return atomic_add_return(i, v) < 0;
> +}
>  
>  #endif /* __ARCH_ARM_ATOMIC__ */

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel