patches for DPDK stable branches
 help / color / mirror / Atom feed
From: Olivier Matz <olivier.matz@6wind.com>
To: "Gavin Hu (Arm Technology China)" <Gavin.Hu@arm.com>
Cc: Bruce Richardson <bruce.richardson@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"thomas@monjalon.net" <thomas@monjalon.net>,
	"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>,
	"stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [dpdk-stable] [PATCH v5 2/2] ring: move the atomic load of head above the loop
Date: Mon, 5 Nov 2018 10:44:45 +0100	[thread overview]
Message-ID: <20181105094445.oc36ksxstg56ztkc@platinum> (raw)
In-Reply-To: <VI1PR08MB316746F49E356F622E703BB18FC80@VI1PR08MB3167.eurprd08.prod.outlook.com>

Hi,

On Sat, Nov 03, 2018 at 01:19:29AM +0000, Gavin Hu (Arm Technology China) wrote:
> 
> 
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Friday, November 2, 2018 7:44 PM
> > To: Gavin Hu (Arm Technology China) <Gavin.Hu@arm.com>
> > Cc: dev@dpdk.org; thomas@monjalon.net; stephen@networkplumber.org;
> > olivier.matz@6wind.com; chaozhu@linux.vnet.ibm.com;
> > konstantin.ananyev@intel.com; jerin.jacob@caviumnetworks.com;
> > Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; stable@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@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>

Thanks

  parent reply	other threads:[~2018-11-05  9:44 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 [this message]
2018-11-05 13:36           ` Thomas Monjalon
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=20181105094445.oc36ksxstg56ztkc@platinum \
    --to=olivier.matz@6wind.com \
    --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=stable@dpdk.org \
    --cc=stephen@networkplumber.org \
    --cc=thomas@monjalon.net \
    /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).