The 'bar' variable in the barrier that counts the number of threads currently waiting at the barrier is of type odp_atomic_u32_t, i.e. an unsigned types. And we will compare 'bar' against 'count'. Should 'count' be unsigned but the function parameter used to initialize 'count' be signed (int)?
Maybe this is just a difference of philosophy or style, some like to use int as the default type for anything scalar, I favor unsigned or uint32_t for values that have reason for being negative. -- Ola On 21 November 2014 10:57, Ola Liljedahl <[email protected]> wrote: > And why does odp_sys_core_count() return int? Is there any case where > we need to return a negative value? > > -- Ola > > > On 21 November 2014 10:11, Savolainen, Petri (NSN - FI/Espoo) > <[email protected]> wrote: >> >> >>> -----Original Message----- >>> From: [email protected] [mailto:lng-odp- >>> [email protected]] On Behalf Of ext Ola Liljedahl >>> Sent: Thursday, November 20, 2014 1:54 PM >>> To: [email protected] >>> Subject: [lng-odp] [PATCH 2/7] api: odp_barrier.h: signature change, use >>> odp_atomic.h, atomic wrap-around fix >>> >>> Signed-off-by: Ola Liljedahl <[email protected]> >>> --- >>> platform/linux-generic/include/api/odp_barrier.h | 2 +- >>> platform/linux-generic/odp_barrier.c | 17 +++++++++-------- >>> 2 files changed, 10 insertions(+), 9 deletions(-) >>> >>> diff --git a/platform/linux-generic/include/api/odp_barrier.h >>> b/platform/linux-generic/include/api/odp_barrier.h >>> index 1790ea3..03e889d 100644 >>> --- a/platform/linux-generic/include/api/odp_barrier.h >>> +++ b/platform/linux-generic/include/api/odp_barrier.h >>> @@ -42,7 +42,7 @@ typedef struct odp_barrier_t { >>> * @param barrier Barrier >>> * @param count Thread count >>> */ >>> -void odp_barrier_init(odp_barrier_t *barrier, int count); >>> +void odp_barrier_init(odp_barrier_t *barrier, uint32_t count); >>> >>> >>> /** >>> diff --git a/platform/linux-generic/odp_barrier.c b/platform/linux- >>> generic/odp_barrier.c >>> index 87be2a1..2f14e2f 100644 >>> --- a/platform/linux-generic/odp_barrier.c >>> +++ b/platform/linux-generic/odp_barrier.c >>> @@ -8,11 +8,10 @@ >>> #include <odp_sync.h> >>> #include <odp_spin_internal.h> >>> >>> -void odp_barrier_init(odp_barrier_t *barrier, int count) >>> +void odp_barrier_init(odp_barrier_t *barrier, uint32_t count) >> >> Signature should stay as int, since it matches e.g. odp_sys_core_count(). >> >> -Petri >> >>> { >>> barrier->count = count; >>> - barrier->bar = 0; >>> - odp_sync_stores(); >>> + odp_atomic_init_u32(&barrier->bar, 0); >>> } >>> >>> /* >>> @@ -33,16 +32,18 @@ void odp_barrier_wait(odp_barrier_t *barrier) >>> uint32_t count; >>> int wasless; >>> >>> - odp_sync_stores(); >>> - wasless = barrier->bar < barrier->count; >>> + __atomic_thread_fence(__ATOMIC_SEQ_CST); >>> count = odp_atomic_fetch_inc_u32(&barrier->bar); >>> + wasless = count < barrier->count; >>> >>> if (count == 2*barrier->count-1) { >>> - barrier->bar = 0; >>> + /* Wrap around *atomically* */ >>> + odp_atomic_sub_u32(&barrier->bar, 2 * barrier->count); >>> } else { >>> - while ((barrier->bar < barrier->count) == wasless) >>> + while ((odp_atomic_load_u32(&barrier->bar) < barrier->count) >>> + == wasless) >>> odp_spin(); >>> } >>> >>> - odp_mem_barrier(); >>> + __atomic_thread_fence(__ATOMIC_SEQ_CST); >>> } >>> -- >>> 1.9.1 >>> >>> >>> _______________________________________________ >>> lng-odp mailing list >>> [email protected] >>> http://lists.linaro.org/mailman/listinfo/lng-odp _______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
