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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 14 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>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> Changed since v2:
>   * sample code for atomic_cmpxchg() (doc-comment in <xen/atomic.h>) updated 
> to
>     use atomic_read() instead of directly accessing v.counter
> ---
>  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 620c636..a79420a 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))
>  
> -#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;
> +}
>  
>  #define atomic_xchg(v, new) (xchg(&((v)->counter), new))
>  
> diff --git a/xen/include/asm-x86/atomic.h b/xen/include/asm-x86/atomic.h
> index 5f9f2dd..3e99b03 100644
> --- a/xen/include/asm-x86/atomic.h
> +++ b/xen/include/asm-x86/atomic.h
> @@ -2,6 +2,7 @@
>  #define __ARCH_X86_ATOMIC__
>  
>  #include <xen/config.h>
> +#include <xen/atomic.h>
>  #include <asm/system.h>
>  
>  #define build_read_atomic(name, size, type, reg, barrier) \
> @@ -79,56 +80,21 @@ void __bad_atomic_size(void);
>      }                                                     \
>  })
>  
> -/*
> - * 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) }
> -
> -/**
> - * atomic_read - read atomic variable
> - * @v: pointer of type atomic_t
> - *
> - * Atomically reads the value of @v.
> - */
>  static inline int atomic_read(atomic_t *v)
>  {
>      return read_atomic(&v->counter);
>  }
>  
> -/**
> - * _atomic_read - read atomic variable non-atomically
> - * @v atomic_t
> - *
> - * Non-atomically reads the value of @v
> - */
>  static inline int _atomic_read(atomic_t v)
>  {
>      return v.counter;
>  }
>  
> -/**
> - * atomic_set - set atomic variable
> - * @v: pointer of type atomic_t
> - * @i: required value
> - *
> - * Atomically sets the value of @v to @i.
> - */
>  static inline void atomic_set(atomic_t *v, int i)
>  {
>      write_atomic(&v->counter, i);
>  }
>  
> -/**
> - * _atomic_set - set atomic variable non-atomically
> - * @v: pointer of type atomic_t
> - * @i: required value
> - *
> - * Non-atomically sets the value of @v to @i.
> - */
>  static inline void _atomic_set(atomic_t *v, int i)
>  {
>      v->counter = i;
> @@ -139,13 +105,6 @@ static inline int atomic_cmpxchg(atomic_t *v, int old, 
> int new)
>      return cmpxchg(&v->counter, old, new);
>  }
>  
> -/**
> - * atomic_add - add integer to atomic variable
> - * @i: integer value to add
> - * @v: pointer of type atomic_t
> - * 
> - * Atomically adds @i to @v.
> - */
>  static inline void atomic_add(int i, atomic_t *v)
>  {
>      asm volatile (
> @@ -154,25 +113,11 @@ static inline void atomic_add(int i, atomic_t *v)
>          : "ir" (i), "m" (*(volatile int *)&v->counter) );
>  }
>  
> -/**
> - * atomic_add_return - add integer and return
> - * @i: integer value to add
> - * @v: pointer of type atomic_t
> - *
> - * Atomically adds @i to @v and returns @i + @v
> - */
>  static inline int atomic_add_return(int i, atomic_t *v)
>  {
>      return i + arch_fetch_and_add(&v->counter, i);
>  }
>  
> -/**
> - * atomic_sub - subtract the atomic variable
> - * @i: integer value to subtract
> - * @v: pointer of type atomic_t
> - * 
> - * Atomically subtracts @i from @v.
> - */
>  static inline void atomic_sub(int i, atomic_t *v)
>  {
>      asm volatile (
> @@ -181,15 +126,6 @@ static inline void atomic_sub(int i, atomic_t *v)
>          : "ir" (i), "m" (*(volatile int *)&v->counter) );
>  }
>  
> -/**
> - * atomic_sub_and_test - subtract value from variable and test result
> - * @i: integer value to subtract
> - * @v: pointer of type atomic_t
> - * 
> - * Atomically subtracts @i from @v and returns
> - * true if the result is zero, or false for all
> - * other cases.
> - */
>  static inline int atomic_sub_and_test(int i, atomic_t *v)
>  {
>      unsigned char c;
> @@ -201,12 +137,6 @@ static inline int atomic_sub_and_test(int i, atomic_t *v)
>      return c;
>  }
>  
> -/**
> - * atomic_inc - increment atomic variable
> - * @v: pointer of type atomic_t
> - * 
> - * Atomically increments @v by 1.
> - */ 
>  static inline void atomic_inc(atomic_t *v)
>  {
>      asm volatile (
> @@ -215,14 +145,6 @@ static inline void atomic_inc(atomic_t *v)
>          : "m" (*(volatile int *)&v->counter) );
>  }
>  
> -/**
> - * atomic_inc_and_test - increment and test
> - * @v: pointer of type atomic_t
> - *
> - * Atomically increments @v by 1
> - * and returns true if the result is zero, or false for all
> - * other cases.
> - */
>  static inline int atomic_inc_and_test(atomic_t *v)
>  {
>      unsigned char c;
> @@ -234,12 +156,6 @@ static inline int atomic_inc_and_test(atomic_t *v)
>      return c != 0;
>  }
>  
> -/**
> - * atomic_dec - decrement atomic variable
> - * @v: pointer of type atomic_t
> - * 
> - * Atomically decrements @v by 1.
> - */ 
>  static inline void atomic_dec(atomic_t *v)
>  {
>      asm volatile (
> @@ -248,14 +164,6 @@ static inline void atomic_dec(atomic_t *v)
>          : "m" (*(volatile int *)&v->counter) );
>  }
>  
> -/**
> - * atomic_dec_and_test - decrement and test
> - * @v: pointer of type atomic_t
> - * 
> - * Atomically decrements @v by 1 and
> - * returns true if the result is 0, or false for all other
> - * cases.
> - */ 
>  static inline int atomic_dec_and_test(atomic_t *v)
>  {
>      unsigned char c;
> @@ -267,15 +175,6 @@ static inline int atomic_dec_and_test(atomic_t *v)
>      return c != 0;
>  }
>  
> -/**
> - * atomic_add_negative - add and test if negative
> - * @v: pointer of type atomic_t
> - * @i: integer value to add
> - * 
> - * Atomically adds @i to @v and returns true
> - * if the result is negative, or false when
> - * result is greater than or equal to zero.
> - */ 
>  static inline int atomic_add_negative(int i, atomic_t *v)
>  {
>      unsigned char c;
> diff --git a/xen/include/xen/atomic.h b/xen/include/xen/atomic.h
> new file mode 100644
> index 0000000..d072912
> --- /dev/null
> +++ b/xen/include/xen/atomic.h
> @@ -0,0 +1,171 @@
> +/*
> + * include/xen/atomic.h
> + *
> + * Common atomic operations entities (atomic_t, function prototypes).
> + * Include _from_ arch-side <asm/atomic.h>.
> + *
> + * Copyright (c) 2016 Bitdefender S.R.L.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * this program; If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __XEN_ATOMIC_H__
> +#define __XEN_ATOMIC_H__
> +
> +typedef struct { int counter; } atomic_t;
> +
> +#define ATOMIC_INIT(i) { (i) }
> +
> +/**
> + * atomic_read - read atomic variable
> + * @v: pointer of type atomic_t
> + *
> + * Atomically reads the value of @v.
> + */
> +static inline int atomic_read(atomic_t *v);
> +
> +/**
> + * _atomic_read - read atomic variable non-atomically
> + * @v atomic_t
> + *
> + * Non-atomically reads the value of @v
> + */
> +static inline int _atomic_read(atomic_t v);
> +
> +/**
> + * atomic_set - set atomic variable
> + * @v: pointer of type atomic_t
> + * @i: required value
> + *
> + * Atomically sets the value of @v to @i.
> + */
> +static inline void atomic_set(atomic_t *v, int i);
> +
> +/**
> + * _atomic_set - set atomic variable non-atomically
> + * @v: pointer of type atomic_t
> + * @i: required value
> + *
> + * Non-atomically sets the value of @v to @i.
> + */
> +static inline void _atomic_set(atomic_t *v, int i);
> +
> +/**
> + * atomic_cmpxchg - compare and exchange an atomic variable
> + * @v: pointer of type atomic_t
> + * @old: old value
> + * @new: new value
> + *
> + * Before calling, @old should be set to @v.
> + * Succeeds if @old == @v (likely), in which case stores @new in @v.
> + * Returns the initial value in @v, hence succeeds when the return value
> + * matches that of @old.
> + *
> + * Sample (tries atomic increment of v until the operation succeeds):
> + *
> + *  while(1)
> + *  {
> + *      int old = atomic_read(&v);
> + *      int new = old + 1;
> + *      if ( likely(old == atomic_cmpxchg(&v, old, new)) )
> + *          break; // success!
> + *  }
> + */
> +static inline int atomic_cmpxchg(atomic_t *v, int old, int new);
> +
> +/**
> + * atomic_add - add integer to atomic variable
> + * @i: integer value to add
> + * @v: pointer of type atomic_t
> + *
> + * Atomically adds @i to @v.
> + */
> +static inline void atomic_add(int i, atomic_t *v);
> +
> +/**
> + * atomic_add_return - add integer and return
> + * @i: integer value to add
> + * @v: pointer of type atomic_t
> + *
> + * Atomically adds @i to @v and returns @i + @v
> + */
> +static inline int atomic_add_return(int i, atomic_t *v);
> +
> +/**
> + * atomic_sub - subtract the atomic variable
> + * @i: integer value to subtract
> + * @v: pointer of type atomic_t
> + *
> + * Atomically subtracts @i from @v.
> + */
> +static inline void atomic_sub(int i, atomic_t *v);
> +
> +/**
> + * atomic_sub_and_test - subtract value from variable and test result
> + * @i: integer value to subtract
> + * @v: pointer of type atomic_t
> + *
> + * Atomically subtracts @i from @v and returns
> + * true if the result is zero, or false for all
> + * other cases.
> + */
> +static inline int atomic_sub_and_test(int i, atomic_t *v);
> +
> +/**
> + * atomic_inc - increment atomic variable
> + * @v: pointer of type atomic_t
> + *
> + * Atomically increments @v by 1.
> + */
> +static inline void atomic_inc(atomic_t *v);
> +
> +/**
> + * atomic_inc_and_test - increment and test
> + * @v: pointer of type atomic_t
> + *
> + * Atomically increments @v by 1
> + * and returns true if the result is zero, or false for all
> + * other cases.
> + */
> +static inline int atomic_inc_and_test(atomic_t *v);
> +
> +/**
> + * atomic_dec - decrement atomic variable
> + * @v: pointer of type atomic_t
> + *
> + * Atomically decrements @v by 1.
> + */
> +static inline void atomic_dec(atomic_t *v);
> +
> +/**
> + * atomic_dec_and_test - decrement and test
> + * @v: pointer of type atomic_t
> + *
> + * Atomically decrements @v by 1 and
> + * returns true if the result is 0, or false for all other
> + * cases.
> + */
> +static inline int atomic_dec_and_test(atomic_t *v);
> +
> +/**
> + * atomic_add_negative - add and test if negative
> + * @v: pointer of type atomic_t
> + * @i: integer value to add
> + *
> + * Atomically adds @i to @v and returns true
> + * if the result is negative, or false when
> + * result is greater than or equal to zero.
> + */
> +static inline int atomic_add_negative(int i, atomic_t *v);
> +
> +#endif /* __XEN_ATOMIC_H__ */
> -- 
> 2.5.0
> 

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