From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [PATCH v3 1/1] ring: enforce reading the tail before reading ring slots Date: Thu, 28 Mar 2019 01:21:55 +0100 Message-ID: <1963883.zIr16fWVUr@xps> References: <1551841661-42892-1-git-send-email-gavin.hu@arm.com> <1552409933-45684-2-git-send-email-gavin.hu@arm.com> <2601191342CEEE43887BDE71AB977258013655C014@irsmsx105.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: dev@dpdk.org, "Ananyev, Konstantin" , "nd@arm.com" , "jerinj@marvell.com" , "hemant.agrawal@nxp.com" , "nipun.gupta@nxp.com" , "Honnappa.Nagarahalli@arm.com" , "i.maximets@samsung.com" , "chaozhu@linux.vnet.ibm.com" , "stable@dpdk.org" To: Gavin Hu Return-path: In-Reply-To: <2601191342CEEE43887BDE71AB977258013655C014@irsmsx105.ger.corp.intel.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > > In weak memory models, like arm64, reading the prod.tail may get > > reordered after reading the ring slots, which corrupts the ring and > > stale data is observed. > > > > This issue was reported by NXP on 8-A72 DPAA2 board. The problem is most > > likely caused by missing the acquire semantics when reading > > prod.tail (in SC dequeue) which makes it possible to read a > > stale value from the ring slots. > > > > For MP (and MC) case, rte_atomic32_cmpset() already provides the required > > ordering. For SP case, the control depependency between if-statement(which > > depends on the read of r->cons.tail) and the later stores to the ring slots > > make RMB unnecessary. About the control dependency, read more at: > > https://www.cl.cam.ac.uk/~pes20/ppc-supplemental/test7.pdf > > > > This patch is adding the required read barrier to prevent reading the ring > > slots get reordered before reading prod.tail for SC case. > > > > Fixes: c9fb3c62896f ("ring: move code in a new header file") > > Cc: stable@dpdk.org > > > > Signed-off-by: gavin hu > > Reviewed-by: Ola Liljedahl > > Tested-by: Nipun Gupta > > Acked-by: Konstantin Ananyev Applied, thanks