Re: [dpdk-dev] [PATCH v2 2/3] test/atomic: add 128b compare and swap test

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

> -----Original Message-----
> From: Eads, Gage <gage.eads@intel.com>
> Sent: Monday, June 24, 2019 11:10 PM
> To: Phil Yang (Arm Technology China) <Phil.Yang@arm.com>; dev@dpdk.org
> Cc: thomas@monjalon.net; jerinj@marvell.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: [PATCH v2 2/3] test/atomic: add 128b compare and swap test
> 
> Hi Phil,
> 
> Looks good overall, just a few documentation issues.
> 
> <snip>
> 
> > + *
> > + * - Test "128b compare and swap" (aarch64 and x86_64 only)
> > + *
> > + *   - Initialize 128-bit atomic variables to zero.
> > + *
> > + *   - Invoke ``test_atomici128_cmp_exchange()`` on each lcore. Before
> > doing
> > + *     anything else, the cores are waiting a synchro. Each lcore does
> > + *     these compare and swap (CAS) operations several times::
> > + *
> > + *       Relaxed CAS update counter.val[0] + 2; counter.val[0] + 1;
> > + *       Acquired CAS update counter.val[0] + 2; counter.val[0] + 1;
> > + *       Released CAS update counter.val[0] + 2; counter.val[0] + 1;
> > + *       Acquired_Released CAS update counter.val[0] + 2; counter.val[0] + 1;
> 
Hi Gage,

> The array index in "counter.val[0] + 1", is incorrect, I believe.
Yes, you are correct. I will fix it. It should be 'counter.val[2] + 1'.

> 
> Just a nitpick, but "Relaxed CAS update" can go last to match the order in the
> code.
Sure. Thank you for your correction.

> 
> <snip>
> 
> > +#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_ARM64)
> > +/*
> > + * rte_atomic128_cmp_exchange() should update a 128 bits counter's
> > +first 64
> > + * bits by 2 and the second 64 bits by 1 in this test. It should
> > +return true
> > + * if the compare exchange operation successful.
> 
> "operation successful" -> "operation is successful"
Yes.

> 
> > + * This test repeat 128 bits compare and swap operations 10K rounds.
> > +In each
> 
> "repeat" -> "repeats"
Yes.

> 
> Thanks,
> Gage

Thanks,
Phil