On Mon, 2 Nov 2015 11:23:05 +0530
Jerin Jacob <jerin.jacob at caviumnetworks.com> wrote:
--snip--
> > +/*------------------------- 16 bit atomic operations
> > -------------------------*/
> > +
> > +#ifndef RTE_FORCE_INTRINSICS
> > +static inline int
> > +rte_atomic16_cmpset(volatile uint16_t *dst, uint16_t exp, uint16_t src)
> > +{
> > + return __atomic_compare_exchange(dst, &exp, &src, 0, __ATOMIC_ACQUIRE,
> > + __ATOMIC_ACQUIRE) ? 1 : 0;
> > +}
>
> IMO, it should be __ATOMIC_SEQ_CST be instead of __ATOMIC_ACQUIRE.
> __ATOMIC_ACQUIRE works in conjunction with __ATOMIC_RELEASE.
> AFAIK, DPDK atomic api expects full barrier. C11 memory model not yet
> used.
Seems to be reasonable, thanks.
> So why can't we use RTE_FORCE_INTRINSICS based generic
> implementation. Same holds true for spinlock implementation too(i.e using
> RTE_FORCE_INTRINSICS). Am I missing something here ?
True. This was done with the intention to rewrite as a platform-specific
assembly. But it's never been done yet... If you mean to set
RTE_FORCE_INTRINSICS=y in the defconfig and remove this code entirely
(at least for ARMv7), I would agree.
>
>
>
> > +
> > +static inline int rte_atomic16_test_and_set(rte_atomic16_t *v)
> > +{
> > + return rte_atomic16_cmpset((volatile uint16_t *)&v->cnt, 0, 1);
> > +}
--snip--
--
Jan Viktorin E-mail: Viktorin at RehiveTech.com
System Architect Web: www.RehiveTech.com
RehiveTech
Brno, Czech Republic
