From: ext Santosh Shukla [mailto:santosh.shu...@linaro.org] Sent: Wednesday, November 05, 2014 2:57 PM To: Petri Savolainen Cc: lng-odp-forward Subject: Re: [lng-odp] [PATCH] Removed odp_atomic_int_t
On 31 October 2014 19:09, Petri Savolainen <petri.savolai...@linaro.org<mailto:petri.savolai...@linaro.org>> wrote: Integer version is not needed. Unsigned 32 and 64 bit atomics are used instead. If signed 32/64 bits can be added later instead - if signed 32/64 bits required then can be added later on need basis. Yes. The word “needed” missing from commit message. Is it obvious from the context or could e.g. Maxim add that. on need basis. Signed-off-by: Petri Savolainen <petri.savolai...@linaro.org<mailto:petri.savolai...@linaro.org>> --- platform/linux-generic/include/api/odp_atomic.h | 115 --------------------- platform/linux-generic/include/api/odp_barrier.h | 4 +- .../linux-generic/include/odp_buffer_internal.h | 2 +- platform/linux-generic/odp_barrier.c | 6 +- platform/linux-generic/odp_thread.c | 6 +- test/api_test/odp_atomic_test.c | 80 ++------------ test/api_test/odp_atomic_test.h | 9 -- 7 files changed, 16 insertions(+), 206 deletions(-) diff --git a/platform/linux-generic/include/api/odp_atomic.h b/platform/linux-generic/include/api/odp_atomic.h index 213c81f..5c83b39 100644 --- a/platform/linux-generic/include/api/odp_atomic.h +++ b/platform/linux-generic/include/api/odp_atomic.h @@ -26,10 +26,6 @@ extern "C" { * @{ */ -/** - * Atomic integer - */ -typedef volatile int32_t odp_atomic_int_t; /** * Atomic unsigned integer 64 bits @@ -43,117 +39,6 @@ typedef volatile uint32_t odp_atomic_u32_t; /** - * Initialize atomic integer - * - * @param ptr An integer atomic variable - * - * @note The operation is not synchronized with other threads - */ -static inline void odp_atomic_init_int(odp_atomic_int_t *ptr) -{ - *ptr = 0; -} - -/** - * Load value of atomic integer - * - * @param ptr An atomic variable - * - * @return atomic integer value - * - * @note The operation is not synchronized with other threads - */ -static inline int odp_atomic_load_int(odp_atomic_int_t *ptr) -{ - return *ptr; -} - -/** - * Store value to atomic integer - * - * @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_int(odp_atomic_int_t *ptr, int new_value) -{ - *ptr = new_value; -} - -/** - * Fetch and add atomic integer - * - * @param ptr An atomic variable - * @param value A value to be added to the variable - * - * @return Value of the variable before the operation - */ -static inline int odp_atomic_fetch_add_int(odp_atomic_int_t *ptr, int value) -{ - return __sync_fetch_and_add(ptr, value); -} - -/** - * Fetch and subtract atomic integer - * - * @param ptr An atomic integer variable - * @param value A value to be subtracted from the variable - * - * @return Value of the variable before the operation - */ -static inline int odp_atomic_fetch_sub_int(odp_atomic_int_t *ptr, int value) -{ - return __sync_fetch_and_sub(ptr, value); -} - -/** - * Fetch and increment atomic integer by 1 - * - * @param ptr An atomic variable - * - * @return Value of the variable before the operation - */ -static inline int odp_atomic_fetch_inc_int(odp_atomic_int_t *ptr) -{ - return odp_atomic_fetch_add_int(ptr, 1); -} - -/** - * Increment atomic integer by 1 - * - * @param ptr An atomic variable - * - */ -static inline void odp_atomic_inc_int(odp_atomic_int_t *ptr) -{ - odp_atomic_fetch_add_int(ptr, 1); -} - -/** - * Fetch and decrement atomic integer by 1 - * - * @param ptr An atomic int variable - * - * @return Value of the variable before the operation - */ -static inline int odp_atomic_fetch_dec_int(odp_atomic_int_t *ptr) -{ - return odp_atomic_fetch_sub_int(ptr, 1); -} - -/** - * Decrement atomic integer by 1 - * - * @param ptr An atomic variable - * - */ -static inline void odp_atomic_dec_int(odp_atomic_int_t *ptr) -{ - odp_atomic_fetch_sub_int(ptr, 1); -} - -/** * Initialize atomic uint32 * * @param ptr An atomic variable diff --git a/platform/linux-generic/include/api/odp_barrier.h b/platform/linux-generic/include/api/odp_barrier.h index 866648f..fb02a9d 100644 --- a/platform/linux-generic/include/api/odp_barrier.h +++ b/platform/linux-generic/include/api/odp_barrier.h @@ -31,8 +31,8 @@ extern "C" { * ODP execution barrier */ typedef struct odp_barrier_t { - int count; /**< @private Thread count */ - odp_atomic_int_t bar; /**< @private Barrier counter */ + uint32_t count; /**< @private Thread count */ + odp_atomic_u32_t bar; /**< @private Barrier counter */ } odp_barrier_t; diff --git a/platform/linux-generic/include/odp_buffer_internal.h b/platform/linux-generic/include/odp_buffer_internal.h index 2002b51..0027bfc 100644 --- a/platform/linux-generic/include/odp_buffer_internal.h +++ b/platform/linux-generic/include/odp_buffer_internal.h @@ -88,7 +88,7 @@ typedef struct odp_buffer_hdr_t { uint32_t index; /* buf index in the pool */ size_t size; /* max data size */ size_t cur_offset; /* current offset */ - odp_atomic_int_t ref_count; /* reference count */ + odp_atomic_u32_t ref_count; /* reference count */ odp_buffer_scatter_t scatter; /* Scatter/gather list */ int type; /* type of next header */ odp_buffer_pool_t pool_hdl; /* buffer pool handle */ diff --git a/platform/linux-generic/odp_barrier.c b/platform/linux-generic/odp_barrier.c index a82b294..f4a87c8 100644 --- a/platform/linux-generic/odp_barrier.c +++ b/platform/linux-generic/odp_barrier.c @@ -11,7 +11,7 @@ void odp_barrier_init_count(odp_barrier_t *barrier, int count) { barrier->count = count; - barrier->bar = 0; + barrier->bar = 0; odp_sync_stores(); } @@ -30,12 +30,12 @@ void odp_barrier_init_count(odp_barrier_t *barrier, int count) void odp_barrier_sync(odp_barrier_t *barrier) { - int count; + uint32_t count; int wasless; odp_sync_stores(); wasless = barrier->bar < barrier->count; - count = odp_atomic_fetch_inc_int(&barrier->bar); + count = odp_atomic_fetch_inc_u32(&barrier->bar); if (count == 2*barrier->count-1) { barrier->bar = 0; diff --git a/platform/linux-generic/odp_thread.c b/platform/linux-generic/odp_thread.c index b869b27..dcb893d 100644 --- a/platform/linux-generic/odp_thread.c +++ b/platform/linux-generic/odp_thread.c @@ -31,7 +31,7 @@ typedef struct { typedef struct { thread_state_t thr[ODP_CONFIG_MAX_THREADS]; - odp_atomic_int_t num; + odp_atomic_u32_t num; } thread_globals_t; @@ -64,10 +64,10 @@ int odp_thread_init_global(void) static int thread_id(void) { - int id; + uint32_t id; int cpu; - id = odp_atomic_fetch_add_int(&thread_globals->num, 1); + id = odp_atomic_fetch_inc_u32(&thread_globals->num); if (id >= ODP_CONFIG_MAX_THREADS) { ODP_ERR("Too many threads\n"); diff --git a/test/api_test/odp_atomic_test.c b/test/api_test/odp_atomic_test.c index d92a8c1..3ca7674 100644 --- a/test/api_test/odp_atomic_test.c +++ b/test/api_test/odp_atomic_test.c @@ -10,17 +10,14 @@ #include <odp_common.h> #include <odp_atomic_test.h> -static odp_atomic_int_t a32; static odp_atomic_u32_t a32u; static odp_atomic_u64_t a64u; -static odp_atomic_int_t numthrds; +static odp_atomic_u32_t numthrds; static const char * const test_name[] = { "dummy", "test atomic basic ops add/sub/inc/dec", - "test atomic inc/dec of signed word", - "test atomic add/sub of signed word", "test atomic inc/dec of unsigned word", "test atomic add/sub of unsigned word", "test atomic inc/dec of unsigned double word", @@ -34,12 +31,10 @@ static void usage(void) printf("\n./odp_atomic -t <testcase> -n <num of pthread>,\n\n" "\t<testcase> is\n" "\t\t1 - Test mix(does inc,dec,add,sub on 32/64 bit)\n" - "\t\t2 - Test inc dec of signed word\n" - "\t\t3 - Test add sub of signed word\n" - "\t\t4 - Test inc dec of unsigned word\n" - "\t\t5 - Test add sub of unsigned word\n" - "\t\t6 - Test inc dec of double word\n" - "\t\t7 - Test add sub of double word\n" + "\t\t2 - Test inc dec of unsigned word\n" + "\t\t3 - Test add sub of unsigned word\n" + "\t\t4 - Test inc dec of double word\n" + "\t\t5 - Test add sub of double word\n" "\t<num of pthread> is optional\n" "\t\t<1 - 31> - no of pthreads to start\n" "\t\tif user doesn't specify this option, then\n" @@ -50,13 +45,6 @@ static void usage(void) "\t\t./odp_atomic -t 3 -n 12\n"); } -void test_atomic_inc_32(void) -{ - int i; - - for (i = 0; i < CNT; i++) - odp_atomic_inc_int(&a32); -} void test_atomic_inc_u32(void) { @@ -74,14 +62,6 @@ void test_atomic_inc_64(void) odp_atomic_inc_u64(&a64u); } -void test_atomic_dec_32(void) -{ - int i; - - for (i = 0; i < CNT; i++) - odp_atomic_dec_int(&a32); -} - void test_atomic_dec_u32(void) { int i; @@ -98,14 +78,6 @@ void test_atomic_dec_64(void) odp_atomic_dec_u64(&a64u); } -void test_atomic_add_32(void) -{ - int i; - - for (i = 0; i < (CNT / ADD_SUB_CNT); i++) - odp_atomic_fetch_add_int(&a32, ADD_SUB_CNT); -} - void test_atomic_add_u32(void) { int i; @@ -122,14 +94,6 @@ void test_atomic_add_64(void) odp_atomic_fetch_add_u64(&a64u, ADD_SUB_CNT); } -void test_atomic_sub_32(void) -{ - int i; - - for (i = 0; i < (CNT / ADD_SUB_CNT); i++) - odp_atomic_fetch_sub_int(&a32, ADD_SUB_CNT); -} - void test_atomic_sub_u32(void) { int i; @@ -146,18 +110,6 @@ void test_atomic_sub_64(void) odp_atomic_fetch_sub_u64(&a64u, ADD_SUB_CNT); } -void test_atomic_inc_dec_32(void) -{ - test_atomic_inc_32(); - test_atomic_dec_32(); -} - -void test_atomic_add_sub_32(void) -{ - test_atomic_add_32(); - test_atomic_sub_32(); -} - void test_atomic_inc_dec_u32(void) { test_atomic_inc_u32(); @@ -188,11 +140,6 @@ void test_atomic_add_sub_64(void) */ void test_atomic_basic(void) { - test_atomic_inc_32(); - test_atomic_dec_32(); - test_atomic_add_32(); - test_atomic_sub_32(); - test_atomic_inc_u32(); test_atomic_dec_u32(); test_atomic_add_u32(); @@ -206,25 +153,18 @@ void test_atomic_basic(void) void test_atomic_init(void) { - odp_atomic_init_int(&a32); odp_atomic_init_u32(&a32u); odp_atomic_init_u64(&a64u); } void test_atomic_store(void) { - odp_atomic_store_int(&a32, S32_INIT_VAL); odp_atomic_store_u32(&a32u, U32_INIT_VAL); odp_atomic_store_u64(&a64u, U64_INIT_VAL); } int test_atomic_validate(void) { - if (odp_atomic_load_int(&a32) != S32_INIT_VAL) { - ODP_ERR("Atomic signed 32 usual functions failed\n"); - return -1; - } - if (odp_atomic_load_u32(&a32u) != U32_INIT_VAL) { ODP_ERR("Atomic u32 usual functions failed\n"); return -1; @@ -247,7 +187,7 @@ static void *run_thread(void *arg) ODP_DBG("Thread %i starts\n", thr); - odp_atomic_inc_int(&numthrds); + odp_atomic_inc_u32(&numthrds); /* Wait here until all pthreads are created */ while (*(volatile int *)&numthrds < parg->numthrds) @@ -259,12 +199,6 @@ static void *run_thread(void *arg) case TEST_MIX: test_atomic_basic(); break; - case TEST_INC_DEC_S32: - test_atomic_inc_dec_32(); - break; - case TEST_ADD_SUB_S32: - test_atomic_add_sub_32(); - break; case TEST_INC_DEC_U32: test_atomic_inc_dec_u32(); break; @@ -328,7 +262,7 @@ int main(int argc, char *argv[]) if (pthrdnum == 0) pthrdnum = odp_sys_core_count(); - odp_atomic_init_int(&numthrds); + odp_atomic_init_u32(&numthrds); test_atomic_init(); test_atomic_store(); diff --git a/test/api_test/odp_atomic_test.h b/test/api_test/odp_atomic_test.h index 7814da5..aaa9d34 100644 --- a/test/api_test/odp_atomic_test.h +++ b/test/api_test/odp_atomic_test.h @@ -18,14 +18,11 @@ #define ADD_SUB_CNT 5 #define CNT 500000 -#define S32_INIT_VAL (1UL << 10) #define U32_INIT_VAL (1UL << 10) #define U64_INIT_VAL (1ULL << 33) typedef enum { TEST_MIX = 1, /* Must be first test case num */ - TEST_INC_DEC_S32, - TEST_ADD_SUB_S32, TEST_INC_DEC_U32, TEST_ADD_SUB_U32, TEST_INC_DEC_64, @@ -34,16 +31,10 @@ typedef enum { } odp_test_atomic_t; -void test_atomic_inc_dec_32(void); -void test_atomic_add_sub_32(void); void test_atomic_inc_dec_u32(void); void test_atomic_add_sub_u32(void); void test_atomic_inc_dec_64(void); void test_atomic_add_sub_64(void); so as we should replace above 2 api from _64 to _u64, right? I believe this is your code. I didn’t rename functions, just removed those using int atomics (signed 32). The _64 functions calls _u64 atomic APIs. So, would you like to align function naming after this patch has been merged? -Petri -void test_atomic_inc_32(void); -void test_atomic_dec_32(void); -void test_atomic_add_32(void); -void test_atomic_sub_32(void); void test_atomic_inc_u32(void); void test_atomic_dec_u32(void); void test_atomic_add_u32(void); -- 2.1.1 _______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> http://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp
