patches for DPDK stable branches
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas@monjalon.net>
To: "Gavin Hu (Arm Technology China)" <Gavin.Hu@arm.com>
Cc: stable@dpdk.org, Olivier Matz <olivier.matz@6wind.com>,
	Bruce Richardson <bruce.richardson@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"stephen@networkplumber.org" <stephen@networkplumber.org>,
	"chaozhu@linux.vnet.ibm.com" <chaozhu@linux.vnet.ibm.com>,
	"konstantin.ananyev@intel.com" <konstantin.ananyev@intel.com>,
	"jerin.jacob@caviumnetworks.com" <jerin.jacob@caviumnetworks.com>,
	Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
Subject: Re: [dpdk-stable] [PATCH v5 2/2] ring: move the atomic load of head above the loop
Date: Mon, 05 Nov 2018 14:36:00 +0100	[thread overview]
Message-ID: <3120135.SUipPR6RYI@xps> (raw)
In-Reply-To: <20181105094445.oc36ksxstg56ztkc@platinum>

05/11/2018 10:44, Olivier Matz:
> Hi,
> 
> On Sat, Nov 03, 2018 at 01:19:29AM +0000, Gavin Hu (Arm Technology China) wrote:
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > > 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@dpdk.org
> > > >
> > > > Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> > > > Reviewed-by: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> > > > Reviewed-by: Steve Capper <steve.capper@arm.com>
> > > > Reviewed-by: Ola Liljedahl <Ola.Liljedahl@arm.com>
> > > > Reviewed-by: Jia He <justin.he@arm.com>
> > > > Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > > > Tested-by: Jerin Jacob <jerin.jacob@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."
> 
> I prefer your initial proposal which is more concise. What about
> something like this?
> 
> 
> * **Updated the C11 memory model version of ring library.**
> 
>   The latency is decreased for architectures using the C11 memory model
>   version of the ring library.
> 
>   On Cavium ThunderX2 platform, the changes decreased latency by 27~29%
>   and 3~15% for MPMC and SPSC cases respectively (with 2 lcores). The
>   real improvements may vary with the number of contending lcores and
>   the size of ring.
> 
> 
> About the patch itself:
> Acked-by: Olivier Matz <olivier.matz@6wind.com>

Series applied with the suggested notes.

Thanks

  reply	other threads:[~2018-11-05 13:36 UTC|newest]

Thread overview: 125+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1541157688-40012-1-git-send-email-gavin.hu@arm.com>
     [not found] ` <1541066031-29125-1-git-send-email-gavin.hu@arm.com>
     [not found]   ` <1540981587-88590-1-git-send-email-gavin.hu@arm.com>
     [not found]     ` <1540956945-211373-1-git-send-email-gavin.hu@arm.com>
2018-10-31 10:26       ` [dpdk-stable] [PATCH v3 1/2] ring: synchronize the load and store of the tail Gavin Hu
2018-10-31 22:07         ` [dpdk-stable] [dpdk-dev] " Stephen Hemminger
2018-11-01  9:56           ` Gavin Hu (Arm Technology China)
2018-10-31 10:26       ` [dpdk-stable] [PATCH v3 2/2] ring: move the atomic load of head above the loop Gavin Hu
2018-11-01  9:53     ` [dpdk-stable] [PATCH v4 1/2] ring: synchronize the load and store of the tail Gavin Hu
2018-11-01  9:53     ` [dpdk-stable] [PATCH v4 2/2] ring: move the atomic load of head above the loop Gavin Hu
2018-11-01 17:26       ` Stephen Hemminger
2018-11-02  0:53         ` Gavin Hu (Arm Technology China)
2018-11-02  4:30           ` Honnappa Nagarahalli
2018-11-02  7:15             ` Gavin Hu (Arm Technology China)
2018-11-02  9:36               ` Thomas Monjalon
2018-11-02 11:23                 ` Gavin Hu (Arm Technology China)
2018-11-02 11:21   ` [dpdk-stable] [PATCH v5 1/2] ring: synchronize the load and store of the tail Gavin Hu
2018-11-05  9:30     ` Olivier Matz
2018-11-02 11:21   ` [dpdk-stable] [PATCH v5 2/2] ring: move the atomic load of head above the loop Gavin Hu
2018-11-02 11:43     ` Bruce Richardson
2018-11-03  1:19       ` Gavin Hu (Arm Technology China)
2018-11-03  9:34         ` Honnappa Nagarahalli
2018-11-05 13:17           ` Thomas Monjalon
2018-11-05 13:41             ` Jerin Jacob
2018-11-05  9:44         ` Olivier Matz
2018-11-05 13:36           ` Thomas Monjalon [this message]
2018-08-06  1:18 [dpdk-stable] [PATCH] ring: fix c11 memory ordering issue Gavin Hu
2018-08-06  9:19 ` [dpdk-stable] [dpdk-dev] " Thomas Monjalon
2018-08-08  1:39   ` Gavin Hu
2018-08-07  3:19 ` [dpdk-stable] [PATCH v2] " Gavin Hu
2018-08-07  5:56   ` He, Jia
2018-08-07  7:56     ` Gavin Hu
2018-08-08  3:07       ` Jerin Jacob
2018-08-08  7:23         ` Thomas Monjalon
     [not found]   ` <20180917074735.28161-1-gavin.hu@arm.com>
2018-09-17  7:47     ` [dpdk-stable] [PATCH v3 3/3] doc: add cross compile part for sample applications Gavin Hu
2018-09-17  9:48       ` Jerin Jacob
2018-09-17 10:28         ` Gavin Hu (Arm Technology China)
2018-09-17 10:34           ` Jerin Jacob
2018-09-17 10:55             ` Gavin Hu (Arm Technology China)
2018-09-17 10:49       ` [dpdk-stable] [PATCH v4] " Gavin Hu
2018-09-17 10:53         ` [dpdk-stable] [PATCH v5] " Gavin Hu
2018-09-18 11:00           ` Jerin Jacob
2018-09-19  0:33           ` [dpdk-stable] [PATCH v6] " Gavin Hu
2018-09-17  8:11     ` [dpdk-stable] [PATCH v4 1/4] bus/fslmc: fix undefined reference of memsegs Gavin Hu
2018-09-17  8:11       ` [dpdk-stable] [PATCH v4 2/4] ring: read tail using atomic load Gavin Hu
2018-09-20  6:41         ` Jerin Jacob
2018-09-25  9:26           ` Gavin Hu (Arm Technology China)
2018-09-17  8:11       ` [dpdk-stable] [PATCH v4 3/4] ring: synchronize the load and store of the tail Gavin Hu
2018-09-17  8:11       ` [dpdk-stable] [PATCH v4 4/4] ring: move the atomic load of head above the loop Gavin Hu
2018-10-27 14:21         ` Thomas Monjalon
2018-09-17  8:17   ` [dpdk-stable] [PATCH v3 1/3] ring: read tail using atomic load Gavin Hu
2018-09-17  8:17     ` [dpdk-stable] [PATCH v3 2/3] ring: synchronize the load and store of the tail Gavin Hu
2018-09-26  9:29       ` Gavin Hu (Arm Technology China)
2018-09-26  9:59         ` Justin He
2018-09-29 10:57       ` Jerin Jacob
2018-10-17  6:29       ` [dpdk-stable] [PATCH 1/2] " Gavin Hu
2018-10-17  6:29         ` [dpdk-stable] [PATCH 2/2] ring: move the atomic load of head above the loop Gavin Hu
2018-10-17  6:35         ` [dpdk-stable] [PATCH 1/2] ring: synchronize the load and store of the tail Gavin Hu (Arm Technology China)
2018-10-27 14:39           ` [dpdk-stable] [dpdk-dev] " Thomas Monjalon
2018-10-27 15:00             ` Jerin Jacob
2018-10-27 15:13               ` Thomas Monjalon
2018-10-27 15:34                 ` Jerin Jacob
2018-10-27 15:48                   ` Thomas Monjalon
2018-10-29  2:51                   ` Gavin Hu (Arm Technology China)
2018-10-29  2:57                   ` Gavin Hu (Arm Technology China)
2018-10-29 10:16                     ` Jerin Jacob
2018-10-29 10:47                       ` Thomas Monjalon
2018-10-29 11:10                         ` Jerin Jacob
2018-11-03 20:12                 ` Mattias Rönnblom
2018-11-05 21:51                   ` Honnappa Nagarahalli
2018-11-06 11:03                     ` Mattias Rönnblom
     [not found]         ` <1540955698-209159-1-git-send-email-gavin.hu@arm.com>
2018-10-31  3:14           ` [dpdk-stable] [PATCH v2 " Gavin Hu
2018-10-31  3:14           ` [dpdk-stable] [PATCH v2 2/2] ring: move the atomic load of head above the loop Gavin Hu
2018-10-31  3:35         ` [dpdk-stable] [PATCH v2 1/2] ring: synchronize the load and store of the tail Gavin Hu
2018-10-31  3:35         ` [dpdk-stable] [PATCH v2 2/2] ring: move the atomic load of head above the loop Gavin Hu
2018-10-31  9:36           ` Thomas Monjalon
2018-10-31 10:27             ` Gavin Hu (Arm Technology China)
2018-09-17  8:17     ` [dpdk-stable] [PATCH v3 3/3] " Gavin Hu
2018-09-26  9:29       ` Gavin Hu (Arm Technology China)
2018-09-26 10:06         ` Justin He
2018-09-29  7:19           ` [dpdk-stable] [dpdk-dev] " Stephen Hemminger
2018-09-29 10:59       ` [dpdk-stable] " Jerin Jacob
2018-09-26  9:29     ` [dpdk-stable] [PATCH v3 1/3] ring: read tail using atomic load Gavin Hu (Arm Technology China)
2018-09-26 10:09       ` Justin He
2018-09-29 10:48     ` Jerin Jacob
2018-10-05  0:47       ` Gavin Hu (Arm Technology China)
2018-10-05  8:21         ` Ananyev, Konstantin
2018-10-05 11:15           ` Ola Liljedahl
2018-10-05 11:36             ` Ola Liljedahl
2018-10-05 13:44               ` Ananyev, Konstantin
2018-10-05 14:21                 ` Ola Liljedahl
2018-10-05 15:11                 ` Honnappa Nagarahalli
2018-10-05 17:07                   ` Jerin Jacob
2018-10-05 18:05                     ` Ola Liljedahl
2018-10-05 20:06                       ` Honnappa Nagarahalli
2018-10-05 20:17                         ` Ola Liljedahl
2018-10-05 20:29                           ` Honnappa Nagarahalli
2018-10-05 20:34                             ` Ola Liljedahl
2018-10-06  7:41                               ` Jerin Jacob
2018-10-06 19:44                                 ` Ola Liljedahl
2018-10-06 19:59                                   ` Ola Liljedahl
2018-10-07  4:02                                   ` Jerin Jacob
2018-10-07 20:11                                     ` Ola Liljedahl
2018-10-07 20:44                                     ` Ola Liljedahl
2018-10-08  6:06                                       ` Jerin Jacob
2018-10-08  9:22                                         ` Ola Liljedahl
2018-10-08 10:00                                           ` Jerin Jacob
2018-10-08 10:25                                             ` Ola Liljedahl
2018-10-08 10:33                                               ` Gavin Hu (Arm Technology China)
2018-10-08 10:39                                                 ` Ola Liljedahl
2018-10-08 10:41                                                   ` Gavin Hu (Arm Technology China)
2018-10-08 10:49                                                 ` Jerin Jacob
2018-10-10  6:28                                                   ` Gavin Hu (Arm Technology China)
2018-10-10 19:26                                                     ` Honnappa Nagarahalli
2018-10-08 10:46                                               ` Jerin Jacob
2018-10-08 11:21                                                 ` Ola Liljedahl
2018-10-08 11:50                                                   ` Jerin Jacob
2018-10-08 11:59                                                     ` Ola Liljedahl
2018-10-08 12:05                                                       ` Jerin Jacob
2018-10-08 12:20                                                         ` [dpdk-stable] [dpdk-dev] " Jerin Jacob
2018-10-08 12:30                                                           ` Ola Liljedahl
2018-10-09  8:53                                                             ` Olivier Matz
2018-10-09  3:16                                             ` [dpdk-stable] " Honnappa Nagarahalli
2018-10-08 14:43                                           ` [dpdk-stable] [dpdk-dev] " Bruce Richardson
2018-10-08 14:46                                             ` Ola Liljedahl
2018-10-08 15:45                                               ` Ola Liljedahl
2018-10-08  5:27                               ` [dpdk-stable] " Honnappa Nagarahalli
2018-10-08 10:01                                 ` Ola Liljedahl
2018-10-27 14:17     ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3120135.SUipPR6RYI@xps \
    --to=thomas@monjalon.net \
    --cc=Gavin.Hu@arm.com \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=bruce.richardson@intel.com \
    --cc=chaozhu@linux.vnet.ibm.com \
    --cc=dev@dpdk.org \
    --cc=jerin.jacob@caviumnetworks.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=olivier.matz@6wind.com \
    --cc=stable@dpdk.org \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).