From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerin Jacob Subject: Re: [PATCH v4 0/4] fix race condition in enqueue/dequeue because of cpu reorder Date: Wed, 8 Nov 2017 21:59:03 +0530 Message-ID: <20171108162902.GA16699@jerin> References: <1510118764-29697-1-git-send-email-hejianet@gmail.com> <1510134881-22987-1-git-send-email-hejianet@gmail.com> <20171108121550.GA9632@bricha3-MOBL3.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Bruce Richardson , dev@dpdk.org, olivier.matz@6wind.com, konstantin.ananyev@intel.com, jianbo.liu@arm.com, hemant.agrawal@nxp.com To: Jia He Return-path: Received: from NAM02-BL2-obe.outbound.protection.outlook.com (mail-bl2nam02on0079.outbound.protection.outlook.com [104.47.38.79]) by dpdk.org (Postfix) with ESMTP id 563271B2C9 for ; Wed, 8 Nov 2017 17:29:25 +0100 (CET) Content-Disposition: inline In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" -----Original Message----- > Date: Wed, 8 Nov 2017 23:11:32 +0800 > From: Jia He > To: Bruce Richardson > Cc: jerin.jacob@caviumnetworks.com, dev@dpdk.org, olivier.matz@6wind.com, > konstantin.ananyev@intel.com, jianbo.liu@arm.com, hemant.agrawal@nxp.com > Subject: Re: [PATCH v4 0/4] fix race condition in enqueue/dequeue because > of cpu reorder > User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 > Thunderbird/52.4.0 > > Hi Bruce > > > On 11/8/2017 8:15 PM, Bruce Richardson Wrote: > > On Wed, Nov 08, 2017 at 09:54:37AM +0000, Jia He wrote: > > > We watched a rte panic of mbuf_autotest in our qualcomm arm64 server > > > due to a possible race condition. > > > > > > To fix this race, there are 2 options as suggested by Jerin: 1. use > > > rte_smp_rmb 2. use load_acquire/store_release(refer to [2]). > > > CONFIG_RTE_RING_USE_C11_MEM_MODEL is provided, and by default it is > > > "y" only on arm64 so far. > > > > > > The reason why providing 2 options is due to the performance benchmark > > > difference in different arm machines. > > > > > > Already fuctionally tested on the machines as follows: - on X86 - on > > > arm64 with CONFIG_RTE_RING_USE_C11_MEM_MODEL=y - on arm64 with > > > CONFIG_RTE_RING_USE_C11_MEM_MODEL=n > > > > > > --- Changelog: V4: split into small patches V3: arch specific > > > implementation for enqueue/dequeue barrier V2: let users choose > > > whether using load_acquire/store_release V1: rte_smp_rmb() between 2 > > > loads > > > > > > Jia He (4): eal/arm64: remove the braces {} for dmb() and dsb() ring: > > > guarantee load/load order in enqueue and dequeue ring: introduce new > > > header file to include common functions ring: introduce new header > > > file to support C11 memory model > > > > > I'm wondering what the merge plans are for this set, given we are now > > past RC3 in 17.11? As the rings are broken on ARM machines we need to > > merge in some fix, but I'm a little concerned about the scope of the > > changes from the 3rd and 4th patches. Would it be acceptable to just > > merge in patches 1 & 2 in 17.11 and leave the rework and C11 memory > > model additions in patches 3 & 4 to 18.02 release? > As far as I'm concerned, it is ok. It is OK to me as well. May be Jia can send 0-1 and 2-3 as separate series with exiting comments. > > Cheers, > Jia >