From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerin Jacob Subject: Re: [PATCH v5 1/1] ring: guarantee load/load order in enqueue and dequeue Date: Fri, 10 Nov 2017 08:16:10 +0530 Message-ID: <20171110024609.GA5295@jerin> References: <1510118764-29697-1-git-send-email-hejianet@gmail.com> <1510278669-8489-1-git-send-email-hejianet@gmail.com> <1510278669-8489-2-git-send-email-hejianet@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: dev@dpdk.org, olivier.matz@6wind.com, konstantin.ananyev@intel.com, bruce.richardson@intel.com, jianbo.liu@arm.com, hemant.agrawal@nxp.com, Jia He , jie2.liu@hxt-semitech.com, bing.zhao@hxt-semitech.com To: Jia He Return-path: Received: from NAM01-BN3-obe.outbound.protection.outlook.com (mail-bn3nam01on0046.outbound.protection.outlook.com [104.47.33.46]) by dpdk.org (Postfix) with ESMTP id 49F611B202 for ; Fri, 10 Nov 2017 03:46:38 +0100 (CET) Content-Disposition: inline In-Reply-To: <1510278669-8489-2-git-send-email-hejianet@gmail.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" -----Original Message----- > Date: Fri, 10 Nov 2017 01:51:09 +0000 > From: Jia He > To: jerin.jacob@caviumnetworks.com, dev@dpdk.org, olivier.matz@6wind.com > Cc: konstantin.ananyev@intel.com, bruce.richardson@intel.com, > jianbo.liu@arm.com, hemant.agrawal@nxp.com, Jia He , > Jia He , jie2.liu@hxt-semitech.com, > bing.zhao@hxt-semitech.com > Subject: [PATCH v5 1/1] ring: guarantee load/load order in enqueue and > dequeue > X-Mailer: git-send-email 2.7.4 > > We watched a rte panic of mbuf_autotest in our qualcomm arm64 server. I think, the above text can be improved. Something like below. Fix for the rte_panic() in mbuf_autotest on qualcomm arm64 server(...SoC name...) Root cause: > In __rte_ring_move_cons_head() > ... > do { > /* Restore n as it may change every loop */ > n = max; > > *old_head = r->cons.head; //1st load > const uint32_t prod_tail = r->prod.tail; //2nd load > > cpu1(producer) cpu2(consumer) cpu3(consumer) > load r->prod.tail > in enqueue: > load r->cons.tail > load r->prod.head > > store r->prod.tail > > load r->cons.head > load r->prod.tail > ... > store r->cons.{head,tail} > load r->cons.head > > In weak memory order architectures(powerpc,arm), the 2nd load might be > reodered before the 1st load, that makes *entries is bigger than we > wanted. This nasty reording messed enque/deque up. Then, r->cons.head > will be bigger than prod_tail, then make *entries very big and the > consumer will go forward incorrectly. > > After this patch, even with above context switches, the old cons.head > will be recaculated after failure of rte_atomic32_cmpset. So no race > conditions left. > > There is no such issue on X86, because X86 is strong memory order model. > But rte_smp_rmb() doesn't have impact on runtime performance on X86, so > keep the same code without architectures specific concerns. > > Signed-off-by: Jia He > Signed-off-by: jie2.liu@hxt-semitech.com > Signed-off-by: bing.zhao@hxt-semitech.com ➜ [master][dpdk.org] $ ./devtools/checkpatches.sh ### ring: guarantee load/load order in enqueue and dequeue WARNING:BLOCK_COMMENT_STYLE: Block comments use a trailing */ on a separate line #58: FILE: lib/librte_ring/rte_ring.h:414: + * memory model. It is noop on x86 */ WARNING:BLOCK_COMMENT_STYLE: Block comments use a trailing */ on a separate line #70: FILE: lib/librte_ring/rte_ring.h:527: + * memory model. It is noop on x86 */ total: 0 errors, 2 warnings, 22 lines checked 0/1 valid patch ➜ [master][dpdk.org] $ ./devtools/check-git-log.sh Wrong tag: Signed-off-by: jie2.liu@hxt-semitech.com Signed-off-by: bing.zhao@hxt-semitech.com With above fixes: Acked-by: Jerin Jacob