From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 9C2762BBD for ; Mon, 11 Jul 2016 14:34:05 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga102.jf.intel.com with ESMTP; 11 Jul 2016 05:34:04 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,346,1464678000"; d="scan'208";a="1014531465" Received: from irsmsx105.ger.corp.intel.com ([163.33.3.28]) by orsmga002.jf.intel.com with ESMTP; 11 Jul 2016 05:34:03 -0700 Received: from irsmsx156.ger.corp.intel.com (10.108.20.68) by irsmsx105.ger.corp.intel.com (163.33.3.28) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 11 Jul 2016 13:34:02 +0100 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.51]) by IRSMSX156.ger.corp.intel.com ([10.108.20.68]) with mapi id 14.03.0248.002; Mon, 11 Jul 2016 13:34:02 +0100 From: "Ananyev, Konstantin" To: "Kuusisaari, Juhamatti" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH] lib: move rte_ring read barrier to correct location Thread-Index: AQHR214YM0lag5RGeUO4brWmDxiYS6ATCkbA///7T4CAACNU4A== Date: Mon, 11 Jul 2016 12:34:01 +0000 Message-ID: <2601191342CEEE43887BDE71AB97725836B7C916@irsmsx105.ger.corp.intel.com> References: <20160711102055.14855-1-juhamatti.kuusisaari@coriant.com> <2601191342CEEE43887BDE71AB97725836B7C858@irsmsx105.ger.corp.intel.com> In-Reply-To: Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] lib: move rte_ring read barrier to correct location X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 11 Jul 2016 12:34:06 -0000 > Hi, >=20 > > > -----Original Message----- > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Juhamatti > > > Kuusisaari > > > Sent: Monday, July 11, 2016 11:21 AM > > > To: dev@dpdk.org > > > Subject: [dpdk-dev] [PATCH] lib: move rte_ring read barrier to correc= t > > > location > > > > > > Fix the location of the rte_ring data dependency read barrier. > > > It needs to be called before accessing indexed data to ensure that th= e > > > data itself is guaranteed to be correctly updated. > > > > > > See more details at kernel/Documentation/memory-barriers.txt > > > section 'Data dependency barriers'. > > > > > > Any explanation why? > > From my point smp_rmb()s are on the proper places here :) Konstantin >=20 > The problem here is that on a weak memory model system the CPU is > allowed to load the address data out-of-order in advance. > If the read barrier is after the DEQUEUE, you might end up having the old > data there on a race situation when the buffer is continuously full. > Having it before the DEQUEUE guarantees that the load is not done > in advance. Sorry, still didn't see any race condition in the current code. Can you provide any particular example? >>From other side, moving smp_rmb() before dequeueing the objects, could introduce a race condition, on cpus where later writes can be reorder= ed with earlier reads. Konstantin >=20 > On Intel, it should not matter due to different memory model, so this is > limited to weak memory model systems. >=20 > -- > Juhamatti >=20 > > > > > > Signed-off-by: Juhamatti Kuusisaari > > > --- > > > lib/librte_ring/rte_ring.h | 6 ++++-- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h > > > index eb45e41..a923e49 100644 > > > --- a/lib/librte_ring/rte_ring.h > > > +++ b/lib/librte_ring/rte_ring.h > > > @@ -662,9 +662,10 @@ __rte_ring_mc_do_dequeue(struct rte_ring *r, > > void **obj_table, > > > cons_next); > > > } while (unlikely(success =3D=3D 0)); > > > > > > + rte_smp_rmb(); > > > + > > > /* copy in table */ > > > DEQUEUE_PTRS(); > > > - rte_smp_rmb(); > > > > > > /* > > > * If there are other dequeues in progress that preceded us, > > > @@ -746,9 +747,10 @@ __rte_ring_sc_do_dequeue(struct rte_ring *r, > > void **obj_table, > > > cons_next =3D cons_head + n; > > > r->cons.head =3D cons_next; > > > > > > + rte_smp_rmb(); > > > + > > > /* copy in table */ > > > DEQUEUE_PTRS(); > > > - rte_smp_rmb(); > > > > > > __RING_STAT_ADD(r, deq_success, n); > > > r->cons.tail =3D cons_next; > > > -- > > > 2.9.0 > > > > > > > > > > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > =3D=3D > > > The information contained in this message may be privileged and > > > confidential and protected from disclosure. If the reader of this > > > message is not the intended recipient, or an employee or agent > > > responsible for delivering this message to the intended recipient, yo= u > > > are hereby notified that any reproduction, dissemination or > > > distribution of this communication is strictly prohibited. If you hav= e > > > received this communication in error, please notify us immediately by > > > replying to the message and deleting it from your computer. Thank you= . > > > Coriant-Tellabs > > > > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > =3D=3D