From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out5-smtp.messagingengine.com (out5-smtp.messagingengine.com [66.111.4.29]) by dpdk.org (Postfix) with ESMTP id D5D89493D; Mon, 5 Nov 2018 14:36:04 +0100 (CET) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 2A77322B62; Mon, 5 Nov 2018 08:36:04 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Mon, 05 Nov 2018 08:36:04 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=bPsU9o52H8SZjcaqkbP85+mwdC3Qa5HWmxsHVogfG3Y=; b=cYOPcpzSrSG+ 6nzr7p7N23p0DoAp/xiPq3yeQaVK1Uks+IeI89XRm2sP+Nbq+jFO+hQYdrcATAsh rPZ6UXEAo7ItYTmLtlhtBGAj/NHYTS63i9uwz3sBR7a+tIs9cSs03NFAuhs9Ukas n6ZMd88XKIMej7Haq6f6zehQMtrfzFk= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=bPsU9o52H8SZjcaqkbP85+mwdC3Qa5HWmxsHVogfG 3Y=; b=sIqOPDGsksPR4+tVNLDNuDjUXD55Eglxbbg1QuYfXXfG5uJtgTGuvLaxK nLz47TBMOBYABaqmoV2DMoBFXTk+qt8pxTZQPCpAu4n+KEW5t9Ma6E8a60IXBH9H n9AJLImwhkAfQEjS1hLjn+8vIpgnEZUg4+SiR/5YvSBnHbI0qKrpqugTd6o0SJWb BU54INk8ydQVABb1JKeBCYOo/N0ZZMi2ahTtHdbQl1knfsaOHAa5PCNDfcVEtFgt 9qHojbPGQUKPgFQzHTuAXS6oXHCoxF1946gn2xzvMXYrVAaLNjhmNc1k3qf0olkm B0Cc8RFbxLVoKqX2o91s5bWAaeIfQ== X-ME-Sender: X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id E36A010338; Mon, 5 Nov 2018 08:36:01 -0500 (EST) From: Thomas Monjalon To: "Gavin Hu (Arm Technology China)" Cc: stable@dpdk.org, Olivier Matz , Bruce Richardson , "dev@dpdk.org" , "stephen@networkplumber.org" , "chaozhu@linux.vnet.ibm.com" , "konstantin.ananyev@intel.com" , "jerin.jacob@caviumnetworks.com" , Honnappa Nagarahalli Date: Mon, 05 Nov 2018 14:36:00 +0100 Message-ID: <3120135.SUipPR6RYI@xps> In-Reply-To: <20181105094445.oc36ksxstg56ztkc@platinum> References: <1541066031-29125-1-git-send-email-gavin.hu@arm.com> <20181105094445.oc36ksxstg56ztkc@platinum> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-stable] [PATCH v5 2/2] ring: move the atomic load of head above the loop X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 Nov 2018 13:36:05 -0000 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 > > > 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 > > > > Reviewed-by: Honnappa Nagarahalli > > > > Reviewed-by: Steve Capper > > > > Reviewed-by: Ola Liljedahl > > > > Reviewed-by: Jia He > > > > Acked-by: Jerin Jacob > > > > Tested-by: Jerin Jacob > > > > --- > > > > 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 Series applied with the suggested notes. Thanks