On 23 March 2015 at 14:43, Sumith Dev Vojini <[email protected]> wrote:
> > 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. >> > Cortex-A7 is ARMv7a like Cortex-A15. The current atomics and timer implementation works with ARMv7a, with native 64-bit atomics support (using load/store exclusive paired). Why doesn't this for work for you? I was expecting you to use something based on PPC32 which doesn't support 64-bit atomics. -- Ola >> >> >>>> >>> 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
