| Message ID | 1416484428-23849-2-git-send-email-ola.liljedahl@linaro.org |
|---|---|
| State | New |
| Headers | show |
After minor doxygen corrections (see comments inline), API and implementation OK. I agree with others that each patch should build correctly (when applied in the right order). You could e.g. break dependency to cmpset in first patch, do API modifications in second (incl removal of cmpset), atomic implementation changes in third (uint -> struct), correct application atomic usage in fourth and correct lock/barrier implementations in their own patches. -Petri > -----Original Message----- > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- > bounces@lists.linaro.org] On Behalf Of ext Ola Liljedahl > Sent: Thursday, November 20, 2014 1:54 PM > To: lng-odp@lists.linaro.org > Subject: [lng-odp] [PATCH 1/7] api: odp_atomic.h: use struct type, relaxed > implem, missing funcs > > Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> > --- > platform/linux-generic/include/api/odp_atomic.h | 269 +++++++++++++++---- > ----- > test/api_test/odp_atomic_test.c | 31 +-- > 2 files changed, 182 insertions(+), 118 deletions(-) > > diff --git a/platform/linux-generic/include/api/odp_atomic.h > b/platform/linux-generic/include/api/odp_atomic.h > index 5c83b39..78ba403 100644 > --- a/platform/linux-generic/include/api/odp_atomic.h > +++ b/platform/linux-generic/include/api/odp_atomic.h > @@ -18,11 +18,12 @@ > extern "C" { > #endif > > - > -#include <odp_std_types.h> > +#include <stdint.h> > +#include <odp_align.h> > > /** @addtogroup odp_synchronizers > - * Atomic operations. > + * Atomic types and relaxed operations. These operations cannot be used > for > + * synchronization. > * @{ > */ > > @@ -30,56 +31,67 @@ extern "C" { > /** > * Atomic unsigned integer 64 bits > */ > -typedef volatile uint64_t odp_atomic_u64_t; > +typedef struct { > + uint64_t v; /**< Actual storage for the atomic variable */ > +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 > + /* Some architectures do not support lock-free operations on 64-bit > + * data types. We use a spin lock to ensure atomicity. */ > + char lock; > +#endif > +} odp_atomic_u64_t > +ODP_ALIGNED(sizeof(uint64_t)); /* Enforce alignement! */ > + > > /** > * Atomic unsigned integer 32 bits > */ > -typedef volatile uint32_t odp_atomic_u32_t; > +typedef struct { > + uint32_t v; /**< Actual storage for the atomic variable */ > +} odp_atomic_u32_t > +ODP_ALIGNED(sizeof(uint32_t)); /* Enforce alignement! */ > > > /** > * Initialize atomic uint32 > + * @note Relaxed memory order, cannot be used for synchronization > * > * @param ptr An atomic variable > - * > - * @note The operation is not synchronized with other threads > + * @param val Value to initialize the variable with > */ > -static inline void odp_atomic_init_u32(odp_atomic_u32_t *ptr) > +static inline void odp_atomic_init_u32(odp_atomic_u32_t *ptr, uint32_t > val) > { > - *ptr = 0; > + __atomic_store_n(&ptr->v, val, __ATOMIC_RELAXED); > } > > /** > * Load value of atomic uint32 > + * @note Relaxed memory order, cannot be used for synchronization > * > * @param ptr An atomic variable > * > - * @return atomic uint32 value > - * > - * @note The operation is not synchronized with other threads > + * @return Value of the variable > */ > static inline uint32_t odp_atomic_load_u32(odp_atomic_u32_t *ptr) > { > - return *ptr; > + return __atomic_load_n(&ptr->v, __ATOMIC_RELAXED); > } > > /** > * Store value to atomic uint32 > + * @note Relaxed memory order, cannot be used for synchronization > * > * @param ptr An atomic variable > * @param new_value Store new_value to a variable > - * > - * @note The operation is not synchronized with other threads > */ > static inline void odp_atomic_store_u32(odp_atomic_u32_t *ptr, > uint32_t new_value) > { > - *ptr = new_value; > + __atomic_store_n(&ptr->v, new_value, __ATOMIC_RELAXED); > } > > /** > * Fetch and add atomic uint32 > + * @note Relaxed memory order, cannot be used for synchronization > * > * @param ptr An atomic variable > * @param value A value to be added to the variable > @@ -89,11 +101,25 @@ static inline void > odp_atomic_store_u32(odp_atomic_u32_t *ptr, > static inline uint32_t odp_atomic_fetch_add_u32(odp_atomic_u32_t *ptr, > uint32_t value) > { > - return __sync_fetch_and_add(ptr, value); > + return __atomic_fetch_add(&ptr->v, value, __ATOMIC_RELAXED); > +} > + > +/** > + * Add atomic uint32 > + * @note Relaxed memory order, cannot be used for synchronization > + * > + * @param ptr An atomic variable > + * @param value A value to be added to the variable > + */ > +static inline void odp_atomic_add_u32(odp_atomic_u32_t *ptr, > + uint32_t value) > +{ > + (void)__atomic_fetch_add(&ptr->v, value, __ATOMIC_RELAXED); > } > > /** > * Fetch and subtract uint32 > + * @note Relaxed memory order, cannot be used for synchronization > * > * @param ptr An atomic variable > * @param value A value to be sub to the variable > @@ -103,51 +129,59 @@ static inline uint32_t > odp_atomic_fetch_add_u32(odp_atomic_u32_t *ptr, > static inline uint32_t odp_atomic_fetch_sub_u32(odp_atomic_u32_t *ptr, > uint32_t value) > { > - return __sync_fetch_and_sub(ptr, value); > + return __atomic_fetch_sub(&ptr->v, value, __ATOMIC_RELAXED); > +} > + > +/** > + * Subtract uint32 > + * @note Relaxed memory order, cannot be used for synchronization > + * > + * @param ptr An atomic variable > + * @param value A value to be subtract from the variable > + */ > +static inline void odp_atomic_sub_u32(odp_atomic_u32_t *ptr, > + uint32_t value) > +{ > + (void)__atomic_fetch_sub(&ptr->v, value, __ATOMIC_RELAXED); > } > > /** > * Fetch and increment atomic uint32 by 1 > + * @note Relaxed memory order, cannot be used for synchronization > * > * @param ptr An atomic variable > * > * @return Value of the variable before the operation > */ > -#if defined __OCTEON__ > > static inline uint32_t odp_atomic_fetch_inc_u32(odp_atomic_u32_t *ptr) > { > +#if defined __OCTEON__ > uint32_t ret; > - > __asm__ __volatile__ ("syncws"); > __asm__ __volatile__ ("lai %0,(%2)" : "=r" (ret), "+m" (ptr) : > "r" (ptr)); > - > return ret; > -} > - > #else > - > -static inline uint32_t odp_atomic_fetch_inc_u32(odp_atomic_u32_t *ptr) > -{ > - return odp_atomic_fetch_add_u32(ptr, 1); > -} > - > + return __atomic_fetch_add(&ptr->v, 1, __ATOMIC_RELAXED); > #endif > +} > > /** > * Increment atomic uint32 by 1 > + * @note Relaxed memory order, cannot be used for synchronization > * > * @param ptr An atomic variable > * > */ > static inline void odp_atomic_inc_u32(odp_atomic_u32_t *ptr) > { > - odp_atomic_fetch_add_u32(ptr, 1); > + (void)__atomic_fetch_add(&ptr->v, 1, __ATOMIC_RELAXED); > } > > /** > * Fetch and decrement uint32 by 1 > + * @note Relaxed memory order, cannot be used for synchronization > * > * @param ptr An atomic variable > * > @@ -155,62 +189,75 @@ static inline void > odp_atomic_inc_u32(odp_atomic_u32_t *ptr) > */ > static inline uint32_t odp_atomic_fetch_dec_u32(odp_atomic_u32_t *ptr) > { > - return odp_atomic_fetch_sub_u32(ptr, 1); > + return __atomic_fetch_sub(&ptr->v, 1, __ATOMIC_RELAXED); > } > > /** > * Decrement atomic uint32 by 1 > + * @note Relaxed memory order, cannot be used for synchronization > * > * @param ptr An atomic variable > * > */ > static inline void odp_atomic_dec_u32(odp_atomic_u32_t *ptr) > { > - odp_atomic_fetch_sub_u32(ptr, 1); > + (void)__atomic_fetch_sub(&ptr->v, 1, __ATOMIC_RELAXED); > } > > /** > - * Atomic compare and set for 32bit > + * Initialize atomic uint64 > + * @note Relaxed memory order, cannot be used for synchronization > * > - * @param dst destination location into which the value will be written. > - * @param exp expected value. > - * @param src new value. > - * @return Non-zero on success; 0 on failure. > + * @param ptr An atomic variable @param init_val ... > */ > -static inline int > -odp_atomic_cmpset_u32(odp_atomic_u32_t *dst, uint32_t exp, uint32_t src) > +static inline void odp_atomic_init_u64(odp_atomic_u64_t *ptr, uint64_t > init_val) > { > - return __sync_bool_compare_and_swap(dst, exp, src); > + ptr->v = init_val; > +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 > + __atomic_clear(&ptr->lock, __ATOMIC_RELAXED); > +#endif > } > > +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 > /** > - * Initialize atomic uint64 > - * > - * @param ptr An atomic variable > - * > - * @note The operation is not synchronized with other threads > + * Helper macro for lock-based atomic operations on 64-bit integers Add doxygen tag @internal or some other notification that this macro is not part of the API > + * @param ptr Pointer to the 64-bit atomic variable > + * @param expr Expression used update the variable. > + * @return The old value of the variable. > */ > -static inline void odp_atomic_init_u64(odp_atomic_u64_t *ptr) > -{ > - *ptr = 0; > -} > +#define ATOMIC_OP(ptr, expr) \ > +({ \ > + uint64_t old_val; \ > + /* Loop while lock is already taken, stop when lock becomes clear */ > \ > + while (__atomic_test_and_set(&(ptr)->lock, __ATOMIC_ACQUIRE)) \ > + (void)0; \ > + old_val = (ptr)->v; \ > + (expr); /* Perform whatever update is desired */ \ > + __atomic_clear(&(ptr)->lock, __ATOMIC_RELEASE); \ > + old_val; /* Return old value */ \ > +}) > +#endif > > /** > * Load value of atomic uint64 > + * @note Relaxed memory order, cannot be used for synchronization > * > * @param ptr An atomic variable > * > * @return atomic uint64 value > - * > - * @note The operation is not synchronized with other threads > */ > static inline uint64_t odp_atomic_load_u64(odp_atomic_u64_t *ptr) > { > - return *ptr; > +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 > + return ATOMIC_OP(ptr, (void)0); > +#else > + return __atomic_load_n(&ptr->v, __ATOMIC_RELAXED); > +#endif > } > > /** > * Store value to atomic uint64 > + * @note Relaxed memory order, cannot be used for synchronization > * > * @param ptr An atomic variable > * @param new_value Store new_value to a variable > @@ -220,23 +267,16 @@ static inline uint64_t > odp_atomic_load_u64(odp_atomic_u64_t *ptr) > static inline void odp_atomic_store_u64(odp_atomic_u64_t *ptr, > uint64_t new_value) > { > - *ptr = new_value; > -} > - > -/** > - * Add atomic uint64 > - * > - * @param ptr An atomic variable > - * @param value A value to be added to the variable > - * > - */ > -static inline void odp_atomic_add_u64(odp_atomic_u64_t *ptr, uint64_t > value) > -{ > - __sync_fetch_and_add(ptr, value); > +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 > + (void)ATOMIC_OP(ptr, ptr->v = new_value); > +#else > + __atomic_store_n(&ptr->v, new_value, __ATOMIC_RELAXED); > +#endif > } > > /** > * Fetch and add atomic uint64 > + * @note Relaxed memory order, cannot be used for synchronization > * > * @param ptr An atomic variable > * @param value A value to be added to the variable > @@ -244,56 +284,72 @@ static inline void > odp_atomic_add_u64(odp_atomic_u64_t *ptr, uint64_t value) > * @return Value of the variable before the operation > */ > > -#if defined __powerpc__ && !defined __powerpc64__ > static inline uint64_t odp_atomic_fetch_add_u64(odp_atomic_u64_t *ptr, > uint64_t value) > { > - return __sync_fetch_and_add((odp_atomic_u32_t *)ptr, > - (uint32_t)value); > -} > +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 > + return ATOMIC_OP(ptr, ptr->v += value); > #else > -static inline uint64_t odp_atomic_fetch_add_u64(odp_atomic_u64_t *ptr, > - uint64_t value) > -{ > - return __sync_fetch_and_add(ptr, value); > -} > + return __atomic_fetch_add(&ptr->v, value, __ATOMIC_RELAXED); > #endif > +} > + > /** > - * Subtract atomic uint64 > + * Add atomic uint64 > + * @note Relaxed memory order, cannot be used for synchronization > * > * @param ptr An atomic variable > - * @param value A value to be subtracted from the variable > + * @param value A value to be added to the variable > * > */ > -static inline void odp_atomic_sub_u64(odp_atomic_u64_t *ptr, uint64_t > value) > +static inline void odp_atomic_add_u64(odp_atomic_u64_t *ptr, uint64_t > value) > { > - __sync_fetch_and_sub(ptr, value); > +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 > + (void)ATOMIC_OP(ptr, ptr->v += value); > +#else > + (void)__atomic_fetch_add(&ptr->v, value, __ATOMIC_RELAXED); > +#endif > } > > /** > * Fetch and subtract atomic uint64 > + * @note Relaxed memory order, cannot be used for synchronization > * > * @param ptr An atomic variable > * @param value A value to be subtracted from the variable > * > * @return Value of the variable before the operation > */ > -#if defined __powerpc__ && !defined __powerpc64__ > static inline uint64_t odp_atomic_fetch_sub_u64(odp_atomic_u64_t *ptr, > uint64_t value) > { > - return __sync_fetch_and_sub((odp_atomic_u32_t *)ptr, > - (uint32_t)value); > -} > +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 > + return ATOMIC_OP(ptr, ptr->v -= value); > #else > -static inline uint64_t odp_atomic_fetch_sub_u64(odp_atomic_u64_t *ptr, > - uint64_t value) > -{ > - return __sync_fetch_and_sub(ptr, value); > + return __atomic_fetch_sub(&ptr->v, value, __ATOMIC_RELAXED); > +#endif > } > + > +/** > + * Subtract atomic uint64 > + * @note Relaxed memory order, cannot be used for synchronization > + * > + * @param ptr An atomic variable > + * @param value A value to be subtracted from the variable > + * > + */ > +static inline void odp_atomic_sub_u64(odp_atomic_u64_t *ptr, uint64_t > value) > +{ > +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 > + (void)ATOMIC_OP(ptr, ptr->v -= value); > +#else > + (void)__atomic_fetch_sub(&ptr->v, value, __ATOMIC_RELAXED); > #endif > +} > + > /** > * Fetch and increment atomic uint64 by 1 > + * @note Relaxed memory order, cannot be used for synchronization > * > * @param ptr An atomic variable > * > @@ -301,22 +357,32 @@ static inline uint64_t > odp_atomic_fetch_sub_u64(odp_atomic_u64_t *ptr, > */ > static inline uint64_t odp_atomic_fetch_inc_u64(odp_atomic_u64_t *ptr) > { > - return odp_atomic_fetch_add_u64(ptr, 1); > +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 > + return ATOMIC_OP(ptr, ptr->v++); > +#else > + return __atomic_fetch_add(&ptr->v, 1, __ATOMIC_RELAXED); > +#endif > } > > /** > * Increment atomic uint64 by 1 > + * @note Relaxed memory order, cannot be used for synchronization > * > * @param ptr An atomic variable > * > */ > static inline void odp_atomic_inc_u64(odp_atomic_u64_t *ptr) > { > - odp_atomic_fetch_add_u64(ptr, 1); > +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 > + (void)ATOMIC_OP(ptr, ptr->v++); > +#else > + (void)__atomic_fetch_add(&ptr->v, 1, __ATOMIC_RELAXED); > +#endif > } > > /** > * Fetch and decrement atomic uint64 by 1 > + * @note Relaxed memory order, cannot be used for synchronization > * > * @param ptr An atomic variable > * > @@ -324,32 +390,27 @@ static inline void > odp_atomic_inc_u64(odp_atomic_u64_t *ptr) > */ > static inline uint64_t odp_atomic_fetch_dec_u64(odp_atomic_u64_t *ptr) > { > - return odp_atomic_fetch_sub_u64(ptr, 1); > +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 > + return ATOMIC_OP(ptr, ptr->v--); > +#else > + return __atomic_fetch_sub(&ptr->v, 1, __ATOMIC_RELAXED); > +#endif > } > > /** > * Decrement atomic uint64 by 1 > + * @note Relaxed memory order, cannot be used for synchronization > * > * @param ptr An atomic variable > * > */ > static inline void odp_atomic_dec_u64(odp_atomic_u64_t *ptr) > { > - odp_atomic_fetch_sub_u64(ptr, 1); > -} > - > -/** > - * Atomic compare and set for 64bit > - * > - * @param dst destination location into which the value will be written. > - * @param exp expected value. > - * @param src new value. > - * @return Non-zero on success; 0 on failure. > - */ > -static inline int > -odp_atomic_cmpset_u64(odp_atomic_u64_t *dst, uint64_t exp, uint64_t src) > -{ > - return __sync_bool_compare_and_swap(dst, exp, src); > +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 > + (void)ATOMIC_OP(ptr, ptr->v--); > +#else > + (void)__atomic_fetch_sub(&ptr->v, 1, __ATOMIC_RELAXED); > +#endif > } > > /** > diff --git a/test/api_test/odp_atomic_test.c > b/test/api_test/odp_atomic_test.c > index f07c962..65d0dd3 100644 > --- a/test/api_test/odp_atomic_test.c > +++ b/test/api_test/odp_atomic_test.c > @@ -13,15 +13,15 @@ > static odp_atomic_u32_t a32u; > static odp_atomic_u64_t a64u; > > -static odp_atomic_u32_t numthrds; > +static odp_barrier_t barrier; > > static const char * const test_name[] = { > "dummy", > - "test atomic basic ops add/sub/inc/dec", > - "test atomic inc/dec of unsigned word", > - "test atomic add/sub of unsigned word", > - "test atomic inc/dec of unsigned double word", > - "test atomic add/sub of unsigned double word" > + "test atomic basic ops (add/sub/inc/dec) on 32- and 64-bit atomic > ints", > + "test atomic inc/dec of 32-bit atomic int", > + "test atomic add/sub of 32-bit atomic int", > + "test atomic inc/dec of 64-bit atomic int", > + "test atomic add/sub of 64-bit atomic int" > }; > > static struct timeval tv0[MAX_WORKERS], tv1[MAX_WORKERS]; > @@ -153,8 +153,8 @@ void test_atomic_basic(void) > > void test_atomic_init(void) > { > - odp_atomic_init_u32(&a32u); > - odp_atomic_init_u64(&a64u); > + odp_atomic_init_u32(&a32u, 0); > + odp_atomic_init_u64(&a64u, 0); > } > > void test_atomic_store(void) > @@ -187,11 +187,14 @@ static void *run_thread(void *arg) > > ODP_DBG("Thread %i starts\n", thr); > > - odp_atomic_inc_u32(&numthrds); > - > - /* Wait here until all pthreads are created */ > - while (*(volatile int *)&numthrds < parg->numthrds) > - ; > + /* Wait here until all threads have arrived */ > + /* Use multiple barriers to verify that it handles wrap around and > + * has no race conditions which could be exposed when invoked back- > + * to-back */ > + odp_barrier_sync(&barrier); > + odp_barrier_sync(&barrier); > + odp_barrier_sync(&barrier); > + odp_barrier_sync(&barrier); > > gettimeofday(&tv0[thr], NULL); > > @@ -262,7 +265,6 @@ int main(int argc, char *argv[]) > if (pthrdnum == 0) > pthrdnum = odp_sys_core_count(); > > - odp_atomic_init_u32(&numthrds); > test_atomic_init(); > test_atomic_store(); > > @@ -277,6 +279,7 @@ int main(int argc, char *argv[]) > usage(); > goto err_exit; > } > + odp_barrier_init_count(&barrier, pthrdnum); > odp_test_thread_create(run_thread, &thrdarg); > > odp_test_thread_exit(&thrdarg); > -- > 1.9.1 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
On 21 November 2014 09:57, Savolainen, Petri (NSN - FI/Espoo) <petri.savolainen@nsn.com> wrote: > After minor doxygen corrections (see comments inline), API and implementation OK. OK. I see two of them. > > I agree with others that each patch should build correctly (when applied in the right order). You could e.g. break dependency to cmpset in first patch, do API modifications in second (incl removal of cmpset), atomic implementation changes in third (uint -> struct), correct application atomic usage in fourth and correct lock/barrier implementations in their own patches. I agree the odp_atomic.h changes could be broken down into smaller patches. But you approved of this file in an earlier email (or at least I interpreted it like an approval) so I didn't see a need for that. It was the other changes that I wanted to separate out (I did not know we required all patches to build separately). And many of these changes are dependent of the atomic types now becoming structs so there is a dependency here between many files than can't be broken apart. I will make the suggested corrections and make another attempt (learning git add -i) at producing a better structured patch set. -- Ola > > > -Petri > > >> -----Original Message----- >> From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- >> bounces@lists.linaro.org] On Behalf Of ext Ola Liljedahl >> Sent: Thursday, November 20, 2014 1:54 PM >> To: lng-odp@lists.linaro.org >> Subject: [lng-odp] [PATCH 1/7] api: odp_atomic.h: use struct type, relaxed >> implem, missing funcs >> >> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> >> --- >> platform/linux-generic/include/api/odp_atomic.h | 269 +++++++++++++++---- >> ----- >> test/api_test/odp_atomic_test.c | 31 +-- >> 2 files changed, 182 insertions(+), 118 deletions(-) >> >> diff --git a/platform/linux-generic/include/api/odp_atomic.h >> b/platform/linux-generic/include/api/odp_atomic.h >> index 5c83b39..78ba403 100644 >> --- a/platform/linux-generic/include/api/odp_atomic.h >> +++ b/platform/linux-generic/include/api/odp_atomic.h >> @@ -18,11 +18,12 @@ >> extern "C" { >> #endif >> >> - >> -#include <odp_std_types.h> >> +#include <stdint.h> >> +#include <odp_align.h> >> >> /** @addtogroup odp_synchronizers >> - * Atomic operations. >> + * Atomic types and relaxed operations. These operations cannot be used >> for >> + * synchronization. >> * @{ >> */ >> >> @@ -30,56 +31,67 @@ extern "C" { >> /** >> * Atomic unsigned integer 64 bits >> */ >> -typedef volatile uint64_t odp_atomic_u64_t; >> +typedef struct { >> + uint64_t v; /**< Actual storage for the atomic variable */ >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 >> + /* Some architectures do not support lock-free operations on 64-bit >> + * data types. We use a spin lock to ensure atomicity. */ >> + char lock; >> +#endif >> +} odp_atomic_u64_t >> +ODP_ALIGNED(sizeof(uint64_t)); /* Enforce alignement! */ >> + >> >> /** >> * Atomic unsigned integer 32 bits >> */ >> -typedef volatile uint32_t odp_atomic_u32_t; >> +typedef struct { >> + uint32_t v; /**< Actual storage for the atomic variable */ >> +} odp_atomic_u32_t >> +ODP_ALIGNED(sizeof(uint32_t)); /* Enforce alignement! */ >> >> >> /** >> * Initialize atomic uint32 >> + * @note Relaxed memory order, cannot be used for synchronization >> * >> * @param ptr An atomic variable >> - * >> - * @note The operation is not synchronized with other threads >> + * @param val Value to initialize the variable with >> */ >> -static inline void odp_atomic_init_u32(odp_atomic_u32_t *ptr) >> +static inline void odp_atomic_init_u32(odp_atomic_u32_t *ptr, uint32_t >> val) >> { >> - *ptr = 0; >> + __atomic_store_n(&ptr->v, val, __ATOMIC_RELAXED); >> } >> >> /** >> * Load value of atomic uint32 >> + * @note Relaxed memory order, cannot be used for synchronization >> * >> * @param ptr An atomic variable >> * >> - * @return atomic uint32 value >> - * >> - * @note The operation is not synchronized with other threads >> + * @return Value of the variable >> */ >> static inline uint32_t odp_atomic_load_u32(odp_atomic_u32_t *ptr) >> { >> - return *ptr; >> + return __atomic_load_n(&ptr->v, __ATOMIC_RELAXED); >> } >> >> /** >> * Store value to atomic uint32 >> + * @note Relaxed memory order, cannot be used for synchronization >> * >> * @param ptr An atomic variable >> * @param new_value Store new_value to a variable >> - * >> - * @note The operation is not synchronized with other threads >> */ >> static inline void odp_atomic_store_u32(odp_atomic_u32_t *ptr, >> uint32_t new_value) >> { >> - *ptr = new_value; >> + __atomic_store_n(&ptr->v, new_value, __ATOMIC_RELAXED); >> } >> >> /** >> * Fetch and add atomic uint32 >> + * @note Relaxed memory order, cannot be used for synchronization >> * >> * @param ptr An atomic variable >> * @param value A value to be added to the variable >> @@ -89,11 +101,25 @@ static inline void >> odp_atomic_store_u32(odp_atomic_u32_t *ptr, >> static inline uint32_t odp_atomic_fetch_add_u32(odp_atomic_u32_t *ptr, >> uint32_t value) >> { >> - return __sync_fetch_and_add(ptr, value); >> + return __atomic_fetch_add(&ptr->v, value, __ATOMIC_RELAXED); >> +} >> + >> +/** >> + * Add atomic uint32 >> + * @note Relaxed memory order, cannot be used for synchronization >> + * >> + * @param ptr An atomic variable >> + * @param value A value to be added to the variable >> + */ >> +static inline void odp_atomic_add_u32(odp_atomic_u32_t *ptr, >> + uint32_t value) >> +{ >> + (void)__atomic_fetch_add(&ptr->v, value, __ATOMIC_RELAXED); >> } >> >> /** >> * Fetch and subtract uint32 >> + * @note Relaxed memory order, cannot be used for synchronization >> * >> * @param ptr An atomic variable >> * @param value A value to be sub to the variable >> @@ -103,51 +129,59 @@ static inline uint32_t >> odp_atomic_fetch_add_u32(odp_atomic_u32_t *ptr, >> static inline uint32_t odp_atomic_fetch_sub_u32(odp_atomic_u32_t *ptr, >> uint32_t value) >> { >> - return __sync_fetch_and_sub(ptr, value); >> + return __atomic_fetch_sub(&ptr->v, value, __ATOMIC_RELAXED); >> +} >> + >> +/** >> + * Subtract uint32 >> + * @note Relaxed memory order, cannot be used for synchronization >> + * >> + * @param ptr An atomic variable >> + * @param value A value to be subtract from the variable >> + */ >> +static inline void odp_atomic_sub_u32(odp_atomic_u32_t *ptr, >> + uint32_t value) >> +{ >> + (void)__atomic_fetch_sub(&ptr->v, value, __ATOMIC_RELAXED); >> } >> >> /** >> * Fetch and increment atomic uint32 by 1 >> + * @note Relaxed memory order, cannot be used for synchronization >> * >> * @param ptr An atomic variable >> * >> * @return Value of the variable before the operation >> */ >> -#if defined __OCTEON__ >> >> static inline uint32_t odp_atomic_fetch_inc_u32(odp_atomic_u32_t *ptr) >> { >> +#if defined __OCTEON__ >> uint32_t ret; >> - >> __asm__ __volatile__ ("syncws"); >> __asm__ __volatile__ ("lai %0,(%2)" : "=r" (ret), "+m" (ptr) : >> "r" (ptr)); >> - >> return ret; >> -} >> - >> #else >> - >> -static inline uint32_t odp_atomic_fetch_inc_u32(odp_atomic_u32_t *ptr) >> -{ >> - return odp_atomic_fetch_add_u32(ptr, 1); >> -} >> - >> + return __atomic_fetch_add(&ptr->v, 1, __ATOMIC_RELAXED); >> #endif >> +} >> >> /** >> * Increment atomic uint32 by 1 >> + * @note Relaxed memory order, cannot be used for synchronization >> * >> * @param ptr An atomic variable >> * >> */ >> static inline void odp_atomic_inc_u32(odp_atomic_u32_t *ptr) >> { >> - odp_atomic_fetch_add_u32(ptr, 1); >> + (void)__atomic_fetch_add(&ptr->v, 1, __ATOMIC_RELAXED); >> } >> >> /** >> * Fetch and decrement uint32 by 1 >> + * @note Relaxed memory order, cannot be used for synchronization >> * >> * @param ptr An atomic variable >> * >> @@ -155,62 +189,75 @@ static inline void >> odp_atomic_inc_u32(odp_atomic_u32_t *ptr) >> */ >> static inline uint32_t odp_atomic_fetch_dec_u32(odp_atomic_u32_t *ptr) >> { >> - return odp_atomic_fetch_sub_u32(ptr, 1); >> + return __atomic_fetch_sub(&ptr->v, 1, __ATOMIC_RELAXED); >> } >> >> /** >> * Decrement atomic uint32 by 1 >> + * @note Relaxed memory order, cannot be used for synchronization >> * >> * @param ptr An atomic variable >> * >> */ >> static inline void odp_atomic_dec_u32(odp_atomic_u32_t *ptr) >> { >> - odp_atomic_fetch_sub_u32(ptr, 1); >> + (void)__atomic_fetch_sub(&ptr->v, 1, __ATOMIC_RELAXED); >> } >> >> /** >> - * Atomic compare and set for 32bit >> + * Initialize atomic uint64 >> + * @note Relaxed memory order, cannot be used for synchronization >> * >> - * @param dst destination location into which the value will be written. >> - * @param exp expected value. >> - * @param src new value. >> - * @return Non-zero on success; 0 on failure. >> + * @param ptr An atomic variable > > @param init_val ... > >> */ >> -static inline int >> -odp_atomic_cmpset_u32(odp_atomic_u32_t *dst, uint32_t exp, uint32_t src) >> +static inline void odp_atomic_init_u64(odp_atomic_u64_t *ptr, uint64_t >> init_val) >> { >> - return __sync_bool_compare_and_swap(dst, exp, src); >> + ptr->v = init_val; >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 >> + __atomic_clear(&ptr->lock, __ATOMIC_RELAXED); >> +#endif >> } >> >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 >> /** >> - * Initialize atomic uint64 >> - * >> - * @param ptr An atomic variable >> - * >> - * @note The operation is not synchronized with other threads >> + * Helper macro for lock-based atomic operations on 64-bit integers > > Add doxygen tag @internal or some other notification that this macro is not part of the API > >> + * @param ptr Pointer to the 64-bit atomic variable >> + * @param expr Expression used update the variable. >> + * @return The old value of the variable. >> */ >> -static inline void odp_atomic_init_u64(odp_atomic_u64_t *ptr) >> -{ >> - *ptr = 0; >> -} >> +#define ATOMIC_OP(ptr, expr) \ >> +({ \ >> + uint64_t old_val; \ >> + /* Loop while lock is already taken, stop when lock becomes clear */ >> \ >> + while (__atomic_test_and_set(&(ptr)->lock, __ATOMIC_ACQUIRE)) \ >> + (void)0; \ >> + old_val = (ptr)->v; \ >> + (expr); /* Perform whatever update is desired */ \ >> + __atomic_clear(&(ptr)->lock, __ATOMIC_RELEASE); \ >> + old_val; /* Return old value */ \ >> +}) >> +#endif >> >> /** >> * Load value of atomic uint64 >> + * @note Relaxed memory order, cannot be used for synchronization >> * >> * @param ptr An atomic variable >> * >> * @return atomic uint64 value >> - * >> - * @note The operation is not synchronized with other threads >> */ >> static inline uint64_t odp_atomic_load_u64(odp_atomic_u64_t *ptr) >> { >> - return *ptr; >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 >> + return ATOMIC_OP(ptr, (void)0); >> +#else >> + return __atomic_load_n(&ptr->v, __ATOMIC_RELAXED); >> +#endif >> } >> >> /** >> * Store value to atomic uint64 >> + * @note Relaxed memory order, cannot be used for synchronization >> * >> * @param ptr An atomic variable >> * @param new_value Store new_value to a variable >> @@ -220,23 +267,16 @@ static inline uint64_t >> odp_atomic_load_u64(odp_atomic_u64_t *ptr) >> static inline void odp_atomic_store_u64(odp_atomic_u64_t *ptr, >> uint64_t new_value) >> { >> - *ptr = new_value; >> -} >> - >> -/** >> - * Add atomic uint64 >> - * >> - * @param ptr An atomic variable >> - * @param value A value to be added to the variable >> - * >> - */ >> -static inline void odp_atomic_add_u64(odp_atomic_u64_t *ptr, uint64_t >> value) >> -{ >> - __sync_fetch_and_add(ptr, value); >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 >> + (void)ATOMIC_OP(ptr, ptr->v = new_value); >> +#else >> + __atomic_store_n(&ptr->v, new_value, __ATOMIC_RELAXED); >> +#endif >> } >> >> /** >> * Fetch and add atomic uint64 >> + * @note Relaxed memory order, cannot be used for synchronization >> * >> * @param ptr An atomic variable >> * @param value A value to be added to the variable >> @@ -244,56 +284,72 @@ static inline void >> odp_atomic_add_u64(odp_atomic_u64_t *ptr, uint64_t value) >> * @return Value of the variable before the operation >> */ >> >> -#if defined __powerpc__ && !defined __powerpc64__ >> static inline uint64_t odp_atomic_fetch_add_u64(odp_atomic_u64_t *ptr, >> uint64_t value) >> { >> - return __sync_fetch_and_add((odp_atomic_u32_t *)ptr, >> - (uint32_t)value); >> -} >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 >> + return ATOMIC_OP(ptr, ptr->v += value); >> #else >> -static inline uint64_t odp_atomic_fetch_add_u64(odp_atomic_u64_t *ptr, >> - uint64_t value) >> -{ >> - return __sync_fetch_and_add(ptr, value); >> -} >> + return __atomic_fetch_add(&ptr->v, value, __ATOMIC_RELAXED); >> #endif >> +} >> + >> /** >> - * Subtract atomic uint64 >> + * Add atomic uint64 >> + * @note Relaxed memory order, cannot be used for synchronization >> * >> * @param ptr An atomic variable >> - * @param value A value to be subtracted from the variable >> + * @param value A value to be added to the variable >> * >> */ >> -static inline void odp_atomic_sub_u64(odp_atomic_u64_t *ptr, uint64_t >> value) >> +static inline void odp_atomic_add_u64(odp_atomic_u64_t *ptr, uint64_t >> value) >> { >> - __sync_fetch_and_sub(ptr, value); >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 >> + (void)ATOMIC_OP(ptr, ptr->v += value); >> +#else >> + (void)__atomic_fetch_add(&ptr->v, value, __ATOMIC_RELAXED); >> +#endif >> } >> >> /** >> * Fetch and subtract atomic uint64 >> + * @note Relaxed memory order, cannot be used for synchronization >> * >> * @param ptr An atomic variable >> * @param value A value to be subtracted from the variable >> * >> * @return Value of the variable before the operation >> */ >> -#if defined __powerpc__ && !defined __powerpc64__ >> static inline uint64_t odp_atomic_fetch_sub_u64(odp_atomic_u64_t *ptr, >> uint64_t value) >> { >> - return __sync_fetch_and_sub((odp_atomic_u32_t *)ptr, >> - (uint32_t)value); >> -} >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 >> + return ATOMIC_OP(ptr, ptr->v -= value); >> #else >> -static inline uint64_t odp_atomic_fetch_sub_u64(odp_atomic_u64_t *ptr, >> - uint64_t value) >> -{ >> - return __sync_fetch_and_sub(ptr, value); >> + return __atomic_fetch_sub(&ptr->v, value, __ATOMIC_RELAXED); >> +#endif >> } >> + >> +/** >> + * Subtract atomic uint64 >> + * @note Relaxed memory order, cannot be used for synchronization >> + * >> + * @param ptr An atomic variable >> + * @param value A value to be subtracted from the variable >> + * >> + */ >> +static inline void odp_atomic_sub_u64(odp_atomic_u64_t *ptr, uint64_t >> value) >> +{ >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 >> + (void)ATOMIC_OP(ptr, ptr->v -= value); >> +#else >> + (void)__atomic_fetch_sub(&ptr->v, value, __ATOMIC_RELAXED); >> #endif >> +} >> + >> /** >> * Fetch and increment atomic uint64 by 1 >> + * @note Relaxed memory order, cannot be used for synchronization >> * >> * @param ptr An atomic variable >> * >> @@ -301,22 +357,32 @@ static inline uint64_t >> odp_atomic_fetch_sub_u64(odp_atomic_u64_t *ptr, >> */ >> static inline uint64_t odp_atomic_fetch_inc_u64(odp_atomic_u64_t *ptr) >> { >> - return odp_atomic_fetch_add_u64(ptr, 1); >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 >> + return ATOMIC_OP(ptr, ptr->v++); >> +#else >> + return __atomic_fetch_add(&ptr->v, 1, __ATOMIC_RELAXED); >> +#endif >> } >> >> /** >> * Increment atomic uint64 by 1 >> + * @note Relaxed memory order, cannot be used for synchronization >> * >> * @param ptr An atomic variable >> * >> */ >> static inline void odp_atomic_inc_u64(odp_atomic_u64_t *ptr) >> { >> - odp_atomic_fetch_add_u64(ptr, 1); >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 >> + (void)ATOMIC_OP(ptr, ptr->v++); >> +#else >> + (void)__atomic_fetch_add(&ptr->v, 1, __ATOMIC_RELAXED); >> +#endif >> } >> >> /** >> * Fetch and decrement atomic uint64 by 1 >> + * @note Relaxed memory order, cannot be used for synchronization >> * >> * @param ptr An atomic variable >> * >> @@ -324,32 +390,27 @@ static inline void >> odp_atomic_inc_u64(odp_atomic_u64_t *ptr) >> */ >> static inline uint64_t odp_atomic_fetch_dec_u64(odp_atomic_u64_t *ptr) >> { >> - return odp_atomic_fetch_sub_u64(ptr, 1); >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 >> + return ATOMIC_OP(ptr, ptr->v--); >> +#else >> + return __atomic_fetch_sub(&ptr->v, 1, __ATOMIC_RELAXED); >> +#endif >> } >> >> /** >> * Decrement atomic uint64 by 1 >> + * @note Relaxed memory order, cannot be used for synchronization >> * >> * @param ptr An atomic variable >> * >> */ >> static inline void odp_atomic_dec_u64(odp_atomic_u64_t *ptr) >> { >> - odp_atomic_fetch_sub_u64(ptr, 1); >> -} >> - >> -/** >> - * Atomic compare and set for 64bit >> - * >> - * @param dst destination location into which the value will be written. >> - * @param exp expected value. >> - * @param src new value. >> - * @return Non-zero on success; 0 on failure. >> - */ >> -static inline int >> -odp_atomic_cmpset_u64(odp_atomic_u64_t *dst, uint64_t exp, uint64_t src) >> -{ >> - return __sync_bool_compare_and_swap(dst, exp, src); >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 >> + (void)ATOMIC_OP(ptr, ptr->v--); >> +#else >> + (void)__atomic_fetch_sub(&ptr->v, 1, __ATOMIC_RELAXED); >> +#endif >> } >> >> /** >> diff --git a/test/api_test/odp_atomic_test.c >> b/test/api_test/odp_atomic_test.c >> index f07c962..65d0dd3 100644 >> --- a/test/api_test/odp_atomic_test.c >> +++ b/test/api_test/odp_atomic_test.c >> @@ -13,15 +13,15 @@ >> static odp_atomic_u32_t a32u; >> static odp_atomic_u64_t a64u; >> >> -static odp_atomic_u32_t numthrds; >> +static odp_barrier_t barrier; >> >> static const char * const test_name[] = { >> "dummy", >> - "test atomic basic ops add/sub/inc/dec", >> - "test atomic inc/dec of unsigned word", >> - "test atomic add/sub of unsigned word", >> - "test atomic inc/dec of unsigned double word", >> - "test atomic add/sub of unsigned double word" >> + "test atomic basic ops (add/sub/inc/dec) on 32- and 64-bit atomic >> ints", >> + "test atomic inc/dec of 32-bit atomic int", >> + "test atomic add/sub of 32-bit atomic int", >> + "test atomic inc/dec of 64-bit atomic int", >> + "test atomic add/sub of 64-bit atomic int" >> }; >> >> static struct timeval tv0[MAX_WORKERS], tv1[MAX_WORKERS]; >> @@ -153,8 +153,8 @@ void test_atomic_basic(void) >> >> void test_atomic_init(void) >> { >> - odp_atomic_init_u32(&a32u); >> - odp_atomic_init_u64(&a64u); >> + odp_atomic_init_u32(&a32u, 0); >> + odp_atomic_init_u64(&a64u, 0); >> } >> >> void test_atomic_store(void) >> @@ -187,11 +187,14 @@ static void *run_thread(void *arg) >> >> ODP_DBG("Thread %i starts\n", thr); >> >> - odp_atomic_inc_u32(&numthrds); >> - >> - /* Wait here until all pthreads are created */ >> - while (*(volatile int *)&numthrds < parg->numthrds) >> - ; >> + /* Wait here until all threads have arrived */ >> + /* Use multiple barriers to verify that it handles wrap around and >> + * has no race conditions which could be exposed when invoked back- >> + * to-back */ >> + odp_barrier_sync(&barrier); >> + odp_barrier_sync(&barrier); >> + odp_barrier_sync(&barrier); >> + odp_barrier_sync(&barrier); >> >> gettimeofday(&tv0[thr], NULL); >> >> @@ -262,7 +265,6 @@ int main(int argc, char *argv[]) >> if (pthrdnum == 0) >> pthrdnum = odp_sys_core_count(); >> >> - odp_atomic_init_u32(&numthrds); >> test_atomic_init(); >> test_atomic_store(); >> >> @@ -277,6 +279,7 @@ int main(int argc, char *argv[]) >> usage(); >> goto err_exit; >> } >> + odp_barrier_init_count(&barrier, pthrdnum); >> odp_test_thread_create(run_thread, &thrdarg); >> >> odp_test_thread_exit(&thrdarg); >> -- >> 1.9.1 >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp
diff --git a/platform/linux-generic/include/api/odp_atomic.h b/platform/linux-generic/include/api/odp_atomic.h index 5c83b39..78ba403 100644 --- a/platform/linux-generic/include/api/odp_atomic.h +++ b/platform/linux-generic/include/api/odp_atomic.h @@ -18,11 +18,12 @@ extern "C" { #endif - -#include <odp_std_types.h> +#include <stdint.h> +#include <odp_align.h> /** @addtogroup odp_synchronizers - * Atomic operations. + * Atomic types and relaxed operations. These operations cannot be used for + * synchronization. * @{ */ @@ -30,56 +31,67 @@ extern "C" { /** * Atomic unsigned integer 64 bits */ -typedef volatile uint64_t odp_atomic_u64_t; +typedef struct { + uint64_t v; /**< Actual storage for the atomic variable */ +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 + /* Some architectures do not support lock-free operations on 64-bit + * data types. We use a spin lock to ensure atomicity. */ + char lock; +#endif +} odp_atomic_u64_t +ODP_ALIGNED(sizeof(uint64_t)); /* Enforce alignement! */ + /** * Atomic unsigned integer 32 bits */ -typedef volatile uint32_t odp_atomic_u32_t; +typedef struct { + uint32_t v; /**< Actual storage for the atomic variable */ +} odp_atomic_u32_t +ODP_ALIGNED(sizeof(uint32_t)); /* Enforce alignement! */ /** * Initialize atomic uint32 + * @note Relaxed memory order, cannot be used for synchronization * * @param ptr An atomic variable - * - * @note The operation is not synchronized with other threads + * @param val Value to initialize the variable with */ -static inline void odp_atomic_init_u32(odp_atomic_u32_t *ptr) +static inline void odp_atomic_init_u32(odp_atomic_u32_t *ptr, uint32_t val) { - *ptr = 0; + __atomic_store_n(&ptr->v, val, __ATOMIC_RELAXED); } /** * Load value of atomic uint32 + * @note Relaxed memory order, cannot be used for synchronization * * @param ptr An atomic variable * - * @return atomic uint32 value - * - * @note The operation is not synchronized with other threads + * @return Value of the variable */ static inline uint32_t odp_atomic_load_u32(odp_atomic_u32_t *ptr) { - return *ptr; + return __atomic_load_n(&ptr->v, __ATOMIC_RELAXED); } /** * Store value to atomic uint32 + * @note Relaxed memory order, cannot be used for synchronization * * @param ptr An atomic variable * @param new_value Store new_value to a variable - * - * @note The operation is not synchronized with other threads */ static inline void odp_atomic_store_u32(odp_atomic_u32_t *ptr, uint32_t new_value) { - *ptr = new_value; + __atomic_store_n(&ptr->v, new_value, __ATOMIC_RELAXED); } /** * Fetch and add atomic uint32 + * @note Relaxed memory order, cannot be used for synchronization * * @param ptr An atomic variable * @param value A value to be added to the variable @@ -89,11 +101,25 @@ static inline void odp_atomic_store_u32(odp_atomic_u32_t *ptr, static inline uint32_t odp_atomic_fetch_add_u32(odp_atomic_u32_t *ptr, uint32_t value) { - return __sync_fetch_and_add(ptr, value); + return __atomic_fetch_add(&ptr->v, value, __ATOMIC_RELAXED); +} + +/** + * Add atomic uint32 + * @note Relaxed memory order, cannot be used for synchronization + * + * @param ptr An atomic variable + * @param value A value to be added to the variable + */ +static inline void odp_atomic_add_u32(odp_atomic_u32_t *ptr, + uint32_t value) +{ + (void)__atomic_fetch_add(&ptr->v, value, __ATOMIC_RELAXED); } /** * Fetch and subtract uint32 + * @note Relaxed memory order, cannot be used for synchronization * * @param ptr An atomic variable * @param value A value to be sub to the variable @@ -103,51 +129,59 @@ static inline uint32_t odp_atomic_fetch_add_u32(odp_atomic_u32_t *ptr, static inline uint32_t odp_atomic_fetch_sub_u32(odp_atomic_u32_t *ptr, uint32_t value) { - return __sync_fetch_and_sub(ptr, value); + return __atomic_fetch_sub(&ptr->v, value, __ATOMIC_RELAXED); +} + +/** + * Subtract uint32 + * @note Relaxed memory order, cannot be used for synchronization + * + * @param ptr An atomic variable + * @param value A value to be subtract from the variable + */ +static inline void odp_atomic_sub_u32(odp_atomic_u32_t *ptr, + uint32_t value) +{ + (void)__atomic_fetch_sub(&ptr->v, value, __ATOMIC_RELAXED); } /** * Fetch and increment atomic uint32 by 1 + * @note Relaxed memory order, cannot be used for synchronization * * @param ptr An atomic variable * * @return Value of the variable before the operation */ -#if defined __OCTEON__ static inline uint32_t odp_atomic_fetch_inc_u32(odp_atomic_u32_t *ptr) { +#if defined __OCTEON__ uint32_t ret; - __asm__ __volatile__ ("syncws"); __asm__ __volatile__ ("lai %0,(%2)" : "=r" (ret), "+m" (ptr) : "r" (ptr)); - return ret; -} - #else - -static inline uint32_t odp_atomic_fetch_inc_u32(odp_atomic_u32_t *ptr) -{ - return odp_atomic_fetch_add_u32(ptr, 1); -} - + return __atomic_fetch_add(&ptr->v, 1, __ATOMIC_RELAXED); #endif +} /** * Increment atomic uint32 by 1 + * @note Relaxed memory order, cannot be used for synchronization * * @param ptr An atomic variable * */ static inline void odp_atomic_inc_u32(odp_atomic_u32_t *ptr) { - odp_atomic_fetch_add_u32(ptr, 1); + (void)__atomic_fetch_add(&ptr->v, 1, __ATOMIC_RELAXED); } /** * Fetch and decrement uint32 by 1 + * @note Relaxed memory order, cannot be used for synchronization * * @param ptr An atomic variable * @@ -155,62 +189,75 @@ static inline void odp_atomic_inc_u32(odp_atomic_u32_t *ptr) */ static inline uint32_t odp_atomic_fetch_dec_u32(odp_atomic_u32_t *ptr) { - return odp_atomic_fetch_sub_u32(ptr, 1); + return __atomic_fetch_sub(&ptr->v, 1, __ATOMIC_RELAXED); } /** * Decrement atomic uint32 by 1 + * @note Relaxed memory order, cannot be used for synchronization * * @param ptr An atomic variable * */ static inline void odp_atomic_dec_u32(odp_atomic_u32_t *ptr) { - odp_atomic_fetch_sub_u32(ptr, 1); + (void)__atomic_fetch_sub(&ptr->v, 1, __ATOMIC_RELAXED); } /** - * Atomic compare and set for 32bit + * Initialize atomic uint64 + * @note Relaxed memory order, cannot be used for synchronization * - * @param dst destination location into which the value will be written. - * @param exp expected value. - * @param src new value. - * @return Non-zero on success; 0 on failure. + * @param ptr An atomic variable */ -static inline int -odp_atomic_cmpset_u32(odp_atomic_u32_t *dst, uint32_t exp, uint32_t src) +static inline void odp_atomic_init_u64(odp_atomic_u64_t *ptr, uint64_t init_val) { - return __sync_bool_compare_and_swap(dst, exp, src); + ptr->v = init_val; +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 + __atomic_clear(&ptr->lock, __ATOMIC_RELAXED); +#endif } +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 /** - * Initialize atomic uint64 - * - * @param ptr An atomic variable - * - * @note The operation is not synchronized with other threads + * Helper macro for lock-based atomic operations on 64-bit integers + * @param ptr Pointer to the 64-bit atomic variable + * @param expr Expression used update the variable. + * @return The old value of the variable. */ -static inline void odp_atomic_init_u64(odp_atomic_u64_t *ptr) -{ - *ptr = 0; -} +#define ATOMIC_OP(ptr, expr) \ +({ \ + uint64_t old_val; \ + /* Loop while lock is already taken, stop when lock becomes clear */ \ + while (__atomic_test_and_set(&(ptr)->lock, __ATOMIC_ACQUIRE)) \ + (void)0; \ + old_val = (ptr)->v; \ + (expr); /* Perform whatever update is desired */ \ + __atomic_clear(&(ptr)->lock, __ATOMIC_RELEASE); \ + old_val; /* Return old value */ \ +}) +#endif /** * Load value of atomic uint64 + * @note Relaxed memory order, cannot be used for synchronization * * @param ptr An atomic variable * * @return atomic uint64 value - * - * @note The operation is not synchronized with other threads */ static inline uint64_t odp_atomic_load_u64(odp_atomic_u64_t *ptr) { - return *ptr; +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 + return ATOMIC_OP(ptr, (void)0); +#else + return __atomic_load_n(&ptr->v, __ATOMIC_RELAXED); +#endif } /** * Store value to atomic uint64 + * @note Relaxed memory order, cannot be used for synchronization * * @param ptr An atomic variable * @param new_value Store new_value to a variable @@ -220,23 +267,16 @@ static inline uint64_t odp_atomic_load_u64(odp_atomic_u64_t *ptr) static inline void odp_atomic_store_u64(odp_atomic_u64_t *ptr, uint64_t new_value) { - *ptr = new_value; -} - -/** - * Add atomic uint64 - * - * @param ptr An atomic variable - * @param value A value to be added to the variable - * - */ -static inline void odp_atomic_add_u64(odp_atomic_u64_t *ptr, uint64_t value) -{ - __sync_fetch_and_add(ptr, value); +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 + (void)ATOMIC_OP(ptr, ptr->v = new_value); +#else + __atomic_store_n(&ptr->v, new_value, __ATOMIC_RELAXED); +#endif } /** * Fetch and add atomic uint64 + * @note Relaxed memory order, cannot be used for synchronization * * @param ptr An atomic variable * @param value A value to be added to the variable @@ -244,56 +284,72 @@ static inline void odp_atomic_add_u64(odp_atomic_u64_t *ptr, uint64_t value) * @return Value of the variable before the operation */ -#if defined __powerpc__ && !defined __powerpc64__ static inline uint64_t odp_atomic_fetch_add_u64(odp_atomic_u64_t *ptr, uint64_t value) { - return __sync_fetch_and_add((odp_atomic_u32_t *)ptr, - (uint32_t)value); -} +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 + return ATOMIC_OP(ptr, ptr->v += value); #else -static inline uint64_t odp_atomic_fetch_add_u64(odp_atomic_u64_t *ptr, - uint64_t value) -{ - return __sync_fetch_and_add(ptr, value); -} + return __atomic_fetch_add(&ptr->v, value, __ATOMIC_RELAXED); #endif +} + /** - * Subtract atomic uint64 + * Add atomic uint64 + * @note Relaxed memory order, cannot be used for synchronization * * @param ptr An atomic variable - * @param value A value to be subtracted from the variable + * @param value A value to be added to the variable * */ -static inline void odp_atomic_sub_u64(odp_atomic_u64_t *ptr, uint64_t value) +static inline void odp_atomic_add_u64(odp_atomic_u64_t *ptr, uint64_t value) { - __sync_fetch_and_sub(ptr, value); +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 + (void)ATOMIC_OP(ptr, ptr->v += value); +#else + (void)__atomic_fetch_add(&ptr->v, value, __ATOMIC_RELAXED); +#endif } /** * Fetch and subtract atomic uint64 + * @note Relaxed memory order, cannot be used for synchronization * * @param ptr An atomic variable * @param value A value to be subtracted from the variable * * @return Value of the variable before the operation */ -#if defined __powerpc__ && !defined __powerpc64__ static inline uint64_t odp_atomic_fetch_sub_u64(odp_atomic_u64_t *ptr, uint64_t value) { - return __sync_fetch_and_sub((odp_atomic_u32_t *)ptr, - (uint32_t)value); -} +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 + return ATOMIC_OP(ptr, ptr->v -= value); #else -static inline uint64_t odp_atomic_fetch_sub_u64(odp_atomic_u64_t *ptr, - uint64_t value) -{ - return __sync_fetch_and_sub(ptr, value); + return __atomic_fetch_sub(&ptr->v, value, __ATOMIC_RELAXED); +#endif } + +/** + * Subtract atomic uint64 + * @note Relaxed memory order, cannot be used for synchronization + * + * @param ptr An atomic variable + * @param value A value to be subtracted from the variable + * + */ +static inline void odp_atomic_sub_u64(odp_atomic_u64_t *ptr, uint64_t value) +{ +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 + (void)ATOMIC_OP(ptr, ptr->v -= value); +#else + (void)__atomic_fetch_sub(&ptr->v, value, __ATOMIC_RELAXED); #endif +} + /** * Fetch and increment atomic uint64 by 1 + * @note Relaxed memory order, cannot be used for synchronization * * @param ptr An atomic variable * @@ -301,22 +357,32 @@ static inline uint64_t odp_atomic_fetch_sub_u64(odp_atomic_u64_t *ptr, */ static inline uint64_t odp_atomic_fetch_inc_u64(odp_atomic_u64_t *ptr) { - return odp_atomic_fetch_add_u64(ptr, 1); +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 + return ATOMIC_OP(ptr, ptr->v++); +#else + return __atomic_fetch_add(&ptr->v, 1, __ATOMIC_RELAXED); +#endif } /** * Increment atomic uint64 by 1 + * @note Relaxed memory order, cannot be used for synchronization * * @param ptr An atomic variable * */ static inline void odp_atomic_inc_u64(odp_atomic_u64_t *ptr) { - odp_atomic_fetch_add_u64(ptr, 1); +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 + (void)ATOMIC_OP(ptr, ptr->v++); +#else + (void)__atomic_fetch_add(&ptr->v, 1, __ATOMIC_RELAXED); +#endif } /** * Fetch and decrement atomic uint64 by 1 + * @note Relaxed memory order, cannot be used for synchronization * * @param ptr An atomic variable * @@ -324,32 +390,27 @@ static inline void odp_atomic_inc_u64(odp_atomic_u64_t *ptr) */ static inline uint64_t odp_atomic_fetch_dec_u64(odp_atomic_u64_t *ptr) { - return odp_atomic_fetch_sub_u64(ptr, 1); +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 + return ATOMIC_OP(ptr, ptr->v--); +#else + return __atomic_fetch_sub(&ptr->v, 1, __ATOMIC_RELAXED); +#endif } /** * Decrement atomic uint64 by 1 + * @note Relaxed memory order, cannot be used for synchronization * * @param ptr An atomic variable * */ static inline void odp_atomic_dec_u64(odp_atomic_u64_t *ptr) { - odp_atomic_fetch_sub_u64(ptr, 1); -} - -/** - * Atomic compare and set for 64bit - * - * @param dst destination location into which the value will be written. - * @param exp expected value. - * @param src new value. - * @return Non-zero on success; 0 on failure. - */ -static inline int -odp_atomic_cmpset_u64(odp_atomic_u64_t *dst, uint64_t exp, uint64_t src) -{ - return __sync_bool_compare_and_swap(dst, exp, src); +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 + (void)ATOMIC_OP(ptr, ptr->v--); +#else + (void)__atomic_fetch_sub(&ptr->v, 1, __ATOMIC_RELAXED); +#endif } /** diff --git a/test/api_test/odp_atomic_test.c b/test/api_test/odp_atomic_test.c index f07c962..65d0dd3 100644 --- a/test/api_test/odp_atomic_test.c +++ b/test/api_test/odp_atomic_test.c @@ -13,15 +13,15 @@ static odp_atomic_u32_t a32u; static odp_atomic_u64_t a64u; -static odp_atomic_u32_t numthrds; +static odp_barrier_t barrier; static const char * const test_name[] = { "dummy", - "test atomic basic ops add/sub/inc/dec", - "test atomic inc/dec of unsigned word", - "test atomic add/sub of unsigned word", - "test atomic inc/dec of unsigned double word", - "test atomic add/sub of unsigned double word" + "test atomic basic ops (add/sub/inc/dec) on 32- and 64-bit atomic ints", + "test atomic inc/dec of 32-bit atomic int", + "test atomic add/sub of 32-bit atomic int", + "test atomic inc/dec of 64-bit atomic int", + "test atomic add/sub of 64-bit atomic int" }; static struct timeval tv0[MAX_WORKERS], tv1[MAX_WORKERS]; @@ -153,8 +153,8 @@ void test_atomic_basic(void) void test_atomic_init(void) { - odp_atomic_init_u32(&a32u); - odp_atomic_init_u64(&a64u); + odp_atomic_init_u32(&a32u, 0); + odp_atomic_init_u64(&a64u, 0); } void test_atomic_store(void) @@ -187,11 +187,14 @@ static void *run_thread(void *arg) ODP_DBG("Thread %i starts\n", thr); - odp_atomic_inc_u32(&numthrds); - - /* Wait here until all pthreads are created */ - while (*(volatile int *)&numthrds < parg->numthrds) - ; + /* Wait here until all threads have arrived */ + /* Use multiple barriers to verify that it handles wrap around and + * has no race conditions which could be exposed when invoked back- + * to-back */ + odp_barrier_sync(&barrier); + odp_barrier_sync(&barrier); + odp_barrier_sync(&barrier); + odp_barrier_sync(&barrier); gettimeofday(&tv0[thr], NULL); @@ -262,7 +265,6 @@ int main(int argc, char *argv[]) if (pthrdnum == 0) pthrdnum = odp_sys_core_count(); - odp_atomic_init_u32(&numthrds); test_atomic_init(); test_atomic_store(); @@ -277,6 +279,7 @@ int main(int argc, char *argv[]) usage(); goto err_exit; } + odp_barrier_init_count(&barrier, pthrdnum); odp_test_thread_create(run_thread, &thrdarg); odp_test_thread_exit(&thrdarg);
Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> --- platform/linux-generic/include/api/odp_atomic.h | 269 +++++++++++++++--------- test/api_test/odp_atomic_test.c | 31 +-- 2 files changed, 182 insertions(+), 118 deletions(-)