From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerin Jacob Subject: Re: [PATCH 2/3] ring: guarantee load ordering of cons/prod when doing enqueue/dequeue Date: Wed, 8 Nov 2017 12:57:32 +0530 Message-ID: <20171108072731.GB21113@jerin> References: <1510121832-16439-1-git-send-email-hejianet@gmail.com> <1510121832-16439-2-git-send-email-hejianet@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: dev@dpdk.org, olivier.matz@6wind.com, konstantin.ananyev@intel.com, bruce.richardson@intel.com, jianbo.liu@arm.com, hemant.agrawal@nxp.com, jie2.liu@hxt-semitech.com, bing.zhao@hxt-semitech.com, jia.he@hxt-semitech.com To: Jia He Return-path: Received: from NAM01-BY2-obe.outbound.protection.outlook.com (mail-by2nam01on0056.outbound.protection.outlook.com [104.47.34.56]) by dpdk.org (Postfix) with ESMTP id 2D8FA1B349 for ; Wed, 8 Nov 2017 08:27:54 +0100 (CET) Content-Disposition: inline In-Reply-To: <1510121832-16439-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: Wed, 8 Nov 2017 06:17:11 +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 , > jie2.liu@hxt-semitech.com, bing.zhao@hxt-semitech.com, > jia.he@hxt-semitech.com > Subject: [PATCH 2/3] ring: guarantee load ordering of cons/prod when doing > enqueue/dequeue > X-Mailer: git-send-email 2.7.4 > It is the V3 patch and subject line does not reflect that. If you send a new version of a patch or series, you must use --in-reply-to with message id of previous version. > We watched a rte panic of mbuf_autotest in our qualcomm arm64 server. > As for the possible race condition, please refer to [1]. Instead of the indirection of [1], add the technical details here to make git comment useful. > > Furthermore, 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. > > The reason why providing 2 options is due to the performance benchmark > difference in different arm machines, please refer to [3]. > > 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 > > We haven't tested the ppc64 case. If anyone verifies it, he can add > CONFIG_RTE_RING_USE_C11_MEM_MODEL=y to ppc64 config files. The testing part can be moved under "---". ie in the comment section below. > > [1] http://dpdk.org/ml/archives/dev/2017-October/078366.html > [2] https://github.com/freebsd/freebsd/blob/master/sys/sys/buf_ring.h#L170 > [3] http://dpdk.org/ml/archives/dev/2017-October/080861.html > Please split this patches into three: - Your original fix - Introduce rte_ring_generic.h and move the common function to that file - Introduce CONFIG_RTE_RING_USE_C11_MEM_MODEL and move C11 memory implementation to lib/librte_ring/rte_ring_c11_mem.h > --- > +#ifndef _RTE_RING_C11_MEM_H_ > +#define _RTE_RING_C11_MEM_H_ > + > +#define UNUSED_PARAMETER(x) ((void)(x)) > +static __rte_always_inline void > +update_tail(struct rte_ring_headtail *ht, uint32_t old_val, uint32_t new_val, > + uint32_t single, uint32_t enqueue) > +{ > + UNUSED_PARAMETER(enqueue); s/UNUSED_PARAMETER/RTE_SET_USED