[dpdk-stable] [PATCH v5 2/2] ring: move the atomic load of head above the loop

[Gavin Hu (Arm Technology China)]

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson at intel.com>
> Sent: Friday, November 2, 2018 7:44 PM
> To: Gavin Hu (Arm Technology China) <Gavin.Hu at arm.com>
> Cc: dev at dpdk.org; thomas at monjalon.net; stephen at networkplumber.org;
> olivier.matz at 6wind.com; chaozhu at linux.vnet.ibm.com;
> konstantin.ananyev at intel.com; jerin.jacob at caviumnetworks.com;
> Honnappa Nagarahalli <Honnappa.Nagarahalli at arm.com>; stable at dpdk.org
> Subject: Re: [PATCH v5 2/2] ring: move the atomic load of head above the
> loop
>
> On Fri, Nov 02, 2018 at 07:21:28PM +0800, Gavin Hu wrote:
> > In __rte_ring_move_prod_head, move the __atomic_load_n up and out
> of
> > the do {} while loop as upon failure the old_head will be updated,
> > another load is costly and not necessary.
> >
> > This helps a little on the latency,about 1~5%.
> >
> >  Test result with the patch(two cores):
> >  SP/SC bulk enq/dequeue (size: 8): 5.64  MP/MC bulk enq/dequeue (size:
> > 8): 9.58  SP/SC bulk enq/dequeue (size: 32): 1.98  MP/MC bulk
> > enq/dequeue (size: 32): 2.30
> >
> > Fixes: 39368ebfc606 ("ring: introduce C11 memory model barrier
> > option")
> > Cc: stable at dpdk.org
> >
> > Signed-off-by: Gavin Hu <gavin.hu at arm.com>
> > Reviewed-by: Honnappa Nagarahalli <Honnappa.Nagarahalli at arm.com>
> > Reviewed-by: Steve Capper <steve.capper at arm.com>
> > Reviewed-by: Ola Liljedahl <Ola.Liljedahl at arm.com>
> > Reviewed-by: Jia He <justin.he at arm.com>
> > Acked-by: Jerin Jacob <jerin.jacob at caviumnetworks.com>
> > Tested-by: Jerin Jacob <jerin.jacob at caviumnetworks.com>
> > ---
> >  doc/guides/rel_notes/release_18_11.rst |  7 +++++++
> >  lib/librte_ring/rte_ring_c11_mem.h     | 10 ++++------
> >  2 files changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/doc/guides/rel_notes/release_18_11.rst
> > b/doc/guides/rel_notes/release_18_11.rst
> > index 376128f..b68afab 100644
> > --- a/doc/guides/rel_notes/release_18_11.rst
> > +++ b/doc/guides/rel_notes/release_18_11.rst
> > @@ -69,6 +69,13 @@ New Features
> >    checked out against that dma mask and rejected if out of range. If more
> than
> >    one device has addressing limitations, the dma mask is the more
> restricted one.
> >
> > +* **Updated the ring library with C11 memory model.**
> > +
> > +  Updated the ring library with C11 memory model, in our tests the
> > + changes  decreased latency by 27~29% and 3~15% for MPMC and SPSC
> cases respectively.
> > +  The real improvements may vary with the number of contending lcores
> > + and the  size of ring.
> > +
> Is this a little misleading, and will users expect massive performance
> improvements generally? The C11 model seems to be used only on some,
> but not all, arm platforms, and then only with "make" builds.
>
> config/arm/meson.build: ['RTE_USE_C11_MEM_MODEL', false]]
> config/common_armv8a_linuxapp:CONFIG_RTE_USE_C11_MEM_MODEL=y
> config/common_base:CONFIG_RTE_USE_C11_MEM_MODEL=n
> config/defconfig_arm64-thunderx-linuxapp-
> gcc:CONFIG_RTE_USE_C11_MEM_MODEL=n
>
> /Bruce

Thank you Bruce for the review, to limit the scope of improvement, I rewrite the note as follows, could you help review? Feel free to change anything if you like.
" Updated the ring library with C11 memory model, running ring_perf_autotest on Cavium ThunderX2 platform, the changes  decreased latency by 27~29% and 3~15% for MPMC and SPSC cases (2 lcores) respectively. Note the changes help the relaxed memory ordering architectures (arm, ppc) only when CONFIG_RTE_USE_C11_MEM_MODEL=y was configured, no impact on strong memory ordering architectures like x86. To what extent they help the real use cases depends on other factors, like the number of contending readers/writers, size of the ring, whether or not it is on the critical path."

/Gavin
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.