On 20 March 2015 at 19:15, Mike Holmes <[email protected]> wrote:
> > > On 20 March 2015 at 14:10, Sumith Dev Vojini <[email protected]> > wrote: > >> Hello, >> >> While building ODP(v1.0.0) I encountered an assertion on the size of >> tick_buf_t in timer.c. The structure is >> >> typedef struct tick_buf_s { >> odp_atomic_u64_t exp_tck;/* Expiration tick or TMO_xxx */ >> odp_buffer_t tmo_buf;/* ODP_BUFFER_INVALID if timer not active */ >> #ifdef TB_NEEDS_PAD >> uint32_t pad;/* Need to be able to access padding for successful >> CAS */ >> #endif >> } tick_buf_t >> #ifdef ODP_ATOMIC_U128 >> ODP_ALIGNED(16) /* 16-byte atomic operations need properly aligned >> addresses */ >> ; >> _ODP_STATIC_ASSERT(sizeof(tick_buf_t) == 16, "sizeof(tick_buf_t) == 16"); >> >> >> odp_atomic_u64_t is of type struct odp_atomic_u64_s >> >> struct odp_atomic_u64_s { >> 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; /**< Spin lock (if needed) used to ensure atomic >> access */ >> #endif >> } ODP_ALIGNED(sizeof(uint64_t)); /* Enforce alignement! */; >> >> So the systems which cannot do atomic 64 bit operations will have lock >> variable defined which increases the size of the that structure >> odp_atomic_u64_s to 16 bytes due to the alignment requirements and the >> assertion fails since tick_buf_s has two more variables increasing its >> size to 24 bytes. >> > Since the size and especially alignment is important to the timer implementation, the best fix is to ensure that odp_atomic_64_t is actually only 64 bits. This could be achieved by moving the lock variable out from the atomic variable into a separate array (equivalent to how timer.c is doing it when the target does not support 128-bit atomics). Each 64-bit atomic does not need its own lock variable, that's mostly a waste of space. Create a N-to-1 mapping to a smaller set of spin locks and use them when accessing the 64-bit atomic variable. Alternatively, let timer.c handle all synchronization internally and use uint64_t instead of odp_atomic_u64_t. BTW: which target is this that does not support 64-bit atomics natively? >> > Thanks Sumith > > Could you make a bug report ? > > https://bugs.linaro.org/enter_bug.cgi?product=OpenDataPlane > > > >> >> -- >> Regards >> Sumith Dev >> >> _______________________________________________ >> lng-odp mailing list >> [email protected] >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> > > > -- > Mike Holmes > Technical Manager - Linaro Networking Group > Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs > > > > _______________________________________________ > 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
