From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 5C7691B154; Wed, 26 Sep 2018 13:47:41 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Sep 2018 04:47:40 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,306,1534834800"; d="scan'208";a="260378256" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.237.221.39]) ([10.237.221.39]) by orsmga005.jf.intel.com with ESMTP; 26 Sep 2018 04:45:30 -0700 To: dev-bounces@dpdk.org, dev@dpdk.org Cc: nd@arm.com, jerin.jacob@caviumnetworks.com, kkokkilagadda@caviumnetworks.com, Honnappa.Nagarahalli@arm.com, Gavin.Hu@arm.com References: <1537363820-3827-1-git-send-email-phil.yang@arm.com> <1537364560-4124-1-git-send-email-phil.yang@arm.com> <1537364560-4124-2-git-send-email-phil.yang@arm.com> From: Ferruh Yigit Openpgp: preference=signencrypt Message-ID: Date: Wed, 26 Sep 2018 12:45:29 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <1537364560-4124-2-git-send-email-phil.yang@arm.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2 2/3] kni: fix kni fifo synchronization X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 26 Sep 2018 11:47:42 -0000 On 9/19/2018 2:42 PM, dev-bounces@dpdk.org wrote: > With existing code in kni_fifo_put, rx_q values are not being updated > before updating fifo_write. While reading rx_q in kni_net_rx_normal, > This is causing the sync issue on other core. The same situation happens > in kni_fifo_get as well. > > So syncing the values by adding C11 atomic memory barriers to make sure > the values being synced before updating fifo_write and fifo_read. > > Fixes: 3fc5ca2 ("kni: initial import") > Signed-off-by: Phil Yang > Reviewed-by: Honnappa Nagarahalli > Reviewed-by: Gavin Hu > --- > .../linuxapp/eal/include/exec-env/rte_kni_common.h | 5 ++++ > lib/librte_kni/rte_kni_fifo.h | 30 +++++++++++++++++++++- > 2 files changed, 34 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h > index cfa9448..1fd713b 100644 > --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h > +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h > @@ -54,8 +54,13 @@ struct rte_kni_request { > * Writing should never overwrite the read position > */ > struct rte_kni_fifo { > +#ifndef RTE_USE_C11_MEM_MODEL > volatile unsigned write; /**< Next position to be written*/ > volatile unsigned read; /**< Next position to be read */ > +#else > + unsigned write; /**< Next position to be written*/ > + unsigned read; /**< Next position to be read */ > +#endif > unsigned len; /**< Circular buffer length */ > unsigned elem_size; /**< Pointer size - for 32/64 bit OS */ > void *volatile buffer[]; /**< The buffer contains mbuf pointers */ > diff --git a/lib/librte_kni/rte_kni_fifo.h b/lib/librte_kni/rte_kni_fifo.h > index ac26a8c..f4171a1 100644 > --- a/lib/librte_kni/rte_kni_fifo.h > +++ b/lib/librte_kni/rte_kni_fifo.h > @@ -28,8 +28,13 @@ kni_fifo_put(struct rte_kni_fifo *fifo, void **data, unsigned num) > { > unsigned i = 0; > unsigned fifo_write = fifo->write; > - unsigned fifo_read = fifo->read; > unsigned new_write = fifo_write; > +#ifdef RTE_USE_C11_MEM_MODEL > + unsigned fifo_read = __atomic_load_n(&fifo->read, > + __ATOMIC_ACQUIRE); > +#else > + unsigned fifo_read = fifo->read; > +#endif Why atomic load preferred against "volatile", won't both end up accessing memory, is atomic load faster? > > for (i = 0; i < num; i++) { > new_write = (new_write + 1) & (fifo->len - 1); > @@ -39,7 +44,12 @@ kni_fifo_put(struct rte_kni_fifo *fifo, void **data, unsigned num) > fifo->buffer[fifo_write] = data[i]; > fifo_write = new_write; > } > +#ifdef RTE_USE_C11_MEM_MODEL > + __atomic_store_n(&fifo->write, fifo_write, __ATOMIC_RELEASE); > +#else > + rte_smp_wmb(); > fifo->write = fifo_write; > +#endif How atomic store guaranties "fifo->buffer[fifo_write] = data[i];" will wait "fifo->write = fifo_write;"? Is atomic store also behave as write memory barrier?