On Mon, Mar 23, 2015 at 5:31 AM, Ola Liljedahl <[email protected]> wrote:
> 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? > > I am working on Freescale's LS1021A with cortex A7. > > >>> >> 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 >> >> > -- Regards SD
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
