Re: [dpdk-dev] [EXT] [PATCH v4 1/3] eal/arm64: add 128-bit atomic compare exchange

["Phil Yang (Arm Technology China)" <Phil.Yang@arm.com>]

> -----Original Message-----
> From: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Sent: Monday, July 22, 2019 6:20 PM
> To: Phil Yang (Arm Technology China) <Phil.Yang@arm.com>; dev@dpdk.org
> Cc: thomas@monjalon.net; gage.eads@intel.com;
> hemant.agrawal@nxp.com; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; Gavin Hu (Arm Technology China)
> <Gavin.Hu@arm.com>; nd <nd@arm.com>
> Subject: RE: [EXT] [PATCH v4 1/3] eal/arm64: add 128-bit atomic compare
> exchange
> 
> > -----Original Message-----
> > From: Phil Yang <phil.yang@arm.com>
> > Sent: Monday, July 22, 2019 2:14 PM
> > To: dev@dpdk.org
> > Cc: thomas@monjalon.net; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>;
> > gage.eads@intel.com; hemant.agrawal@nxp.com;
> > Honnappa.Nagarahalli@arm.com; gavin.hu@arm.com; nd@arm.com
> > Subject: [EXT] [PATCH v4 1/3] eal/arm64: add 128-bit atomic compare
> exchange
> > Add 128-bit atomic compare exchange on aarch64.
> >
> > Suggested-by: Jerin Jacob <jerinj@marvell.com>
> > Signed-off-by: Phil Yang <phil.yang@arm.com>
> > Tested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > +
> > +#
> > +# Compile ARM LSE ATOMIC instructions statically #
> 
> There is NO value for the keyword "statically" here. Right?
Agree.  Will remove it.

> 
> > +CONFIG_RTE_ARM_FEATURE_ATOMICS=n
> 
> 
> > diff --git a/config/defconfig_arm64-thunderx2-linuxapp-gcc
> > b/config/defconfig_arm64-thunderx2-linuxapp-gcc
> > index cc5c64b..17b6dec 100644
> > --- a/config/defconfig_arm64-thunderx2-linuxapp-gcc
> > +++ b/config/defconfig_arm64-thunderx2-linuxapp-gcc
> > @@ -6,6 +6,7 @@
> >
> >  CONFIG_RTE_MACHINE="thunderx2"
> >
> > +CONFIG_RTE_ARM_FEATURE_ATOMICS=y
> 
> 
> Add for octeontx2 as well.

OK. Will add it in v5.

> 
> >  CONFIG_RTE_CACHE_LINE_SIZE=64
> >  CONFIG_RTE_MAX_NUMA_NODES=2
> >  CONFIG_RTE_MAX_LCORE=256
> 
> 
> > +rte_atomic128_cmp_exchange(rte_int128_t *dst,
> > +				rte_int128_t *exp,
> > +				const rte_int128_t *src,
> > +				unsigned int weak,
> > +				int success,
> > +				int failure)
> > +{
> > +	/* Always do strong CAS */
> > +	RTE_SET_USED(weak);
> > +	/* Ignore memory ordering for failure, memory order for
> > +	 * success must be stronger or equal
> > +	 */
> > +	RTE_SET_USED(failure);
> > +	/* Find invalid memory order */
> > +	RTE_ASSERT(success == __ATOMIC_RELAXED
> > +			|| success == __ATOMIC_ACQUIRE
> > +			|| success == __ATOMIC_RELEASE
> > +			|| success == __ATOMIC_ACQ_REL
> > +			|| success == __ATOMIC_SEQ_CST);
> > +
> > +#ifdef __ARM_FEATURE_ATOMICS
> 
> Shouldn't it be #if defined(__ARM_FEATURE_ATOMICS) ||
> defined(RTE_ARM_FEATURE_ATOMICS) ?

Yes. That was a mistake. Will update in the version 5. Thanks.

Thanks,
Phil Yang