From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR01-DB5-obe.outbound.protection.outlook.com (mail-db5eur01on0043.outbound.protection.outlook.com [104.47.2.43]) by dpdk.org (Postfix) with ESMTP id A048F2BBE for ; Tue, 12 Jul 2016 07:27:32 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=coriant.onmicrosoft.com; s=selector1-coriant-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=38FtlITN8BY0iMAD8KWJ1iXT7AVbUPJqq9xZB0dyxSU=; b=oW0WOLNTz3GAvD8xtEJeMsTkKkVOjI+wh25dM1V+EJy2y5c7GX6AbKao2qqMyKexOH0tYldf5w/b9UwquKIEJh9l4rysgnOLpA7urSrRKE+m5avGsy3CLh6JstrtonKknqkEDsVpUl5GC+eT6gu2RQv9agwUpg51nZbgp7BF+/s= Received: from HE1PR04MB1337.eurprd04.prod.outlook.com (10.163.175.23) by HE1PR04MB1337.eurprd04.prod.outlook.com (10.163.175.23) with Microsoft SMTP Server (TLS) id 15.1.539.14; Tue, 12 Jul 2016 05:27:31 +0000 Received: from HE1PR04MB1337.eurprd04.prod.outlook.com ([10.163.175.23]) by HE1PR04MB1337.eurprd04.prod.outlook.com ([10.163.175.23]) with mapi id 15.01.0539.018; Tue, 12 Jul 2016 05:27:31 +0000 From: "Kuusisaari, Juhamatti" To: "Ananyev, Konstantin" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH] lib: move rte_ring read barrier to correct location Thread-Index: AQHR214MoG1fIZBOPEC9vEAt0w2bq6ATCsYAgAAIC6CAABd0gIABBgWw Date: Tue, 12 Jul 2016 05:27:30 +0000 Message-ID: References: <20160711102055.14855-1-juhamatti.kuusisaari@coriant.com> <2601191342CEEE43887BDE71AB97725836B7C858@irsmsx105.ger.corp.intel.com> <2601191342CEEE43887BDE71AB97725836B7C916@irsmsx105.ger.corp.intel.com> In-Reply-To: <2601191342CEEE43887BDE71AB97725836B7C916@irsmsx105.ger.corp.intel.com> Accept-Language: fi-FI, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=Juhamatti.Kuusisaari@coriant.com; x-originating-ip: [138.111.130.175] x-ms-office365-filtering-correlation-id: bedf5f2a-cc28-4ba4-4d5f-08d3aa1538ae x-microsoft-exchange-diagnostics: 1; HE1PR04MB1337; 6:lUMDmK11KK38Fl0lWa7WKz2/9C0MSAUQxtt23qJUjnoRTx/9xYC/JW2wTjEJiJZYrWoUhrzyUwnWkbCamiI2PXjw+fwykhThEfqfqH1bfkQE0Qe8c+TM/JXvYwkbcK5oMogonu7LkrTE0h3sLtqOvu+S/5sFVRruuawNxfVBm2HCG+IyeGnUmwLs2DFKcik2i54D69L1KhesZ56lglRaeTS0YBvekOv9VXxkT87wKPaTl3QK9gjllQ99lW/GBxAw5xdOzB/6WRc7RvYm7A7W2CkXhMj/phdUhEQsoOpBiMcKqtRdlDoDF4FB+PS9G1S1Y44gLAJVk1RFvYZ8yaGZDw==; 5:bj+k2z3ohI3+SfIp5LY6yIWD8ojHMs6a4SPGjOEcruCME4LzcAXIj1cZcC+re6vUjgdmOWgWEdu6rQaEH1d4wzMnvL2UmVnzbD6TaNYpv2eiaW4GdvZmEQG9KV9UEk6UNIqqxCQpZZujBGLHsop1zQ==; 24:LV2yoI408sAiWiospmfMGjZBjMo9gALmakxoQtzP0uqYywwjVJ/ymJJE/XqNthgig6RGcrGjAHCM1D6DuGm5jhP6KEqm+583Nn7eWrPhdSM=; 7:KWTaliek6Dpnj0Vo7UmD65C4nI2JBE4WyvXfh819E2JZ30GVeGaf/nekBovtOXDu5IZwAfQExax+Ikm63gtV6K7LpHfwIDl0TYic9Wg1GT0IZeEgVSssCuDoOqolclTK+eRswKvcJiFAcEeHoYNvM1wzzsGCU1Cuzu8UCqlcWd5eYDUYlsnGf3MkyFPwc52V5LMXlOfk8qi6y9vQmb1h2AqnAqXgMPlo8D6ECT09VhPMi37u3iJRnK0eW6ktWYnl x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:HE1PR04MB1337; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(51653755401839); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001)(6055026); SRVR:HE1PR04MB1337; BCL:0; PCL:0; RULEID:; SRVR:HE1PR04MB1337; x-forefront-prvs: 0001227049 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(6009001)(7916002)(13464003)(199003)(189002)(377454003)(66066001)(2906002)(92566002)(106116001)(5002640100001)(9686002)(76576001)(106356001)(86362001)(93886004)(97736004)(586003)(5001770100001)(107886002)(101416001)(74316002)(102836003)(6116002)(3846002)(189998001)(50986999)(54356999)(11100500001)(10400500002)(8936002)(87936001)(19580405001)(3280700002)(19580395003)(81166006)(81156014)(33656002)(3660700001)(2950100001)(2900100001)(76176999)(68736007)(305945005)(2501003)(77096005)(122556002)(7846002)(105586002)(5003600100003)(7736002)(7696003); DIR:OUT; SFP:1101; SCL:1; SRVR:HE1PR04MB1337; H:HE1PR04MB1337.eurprd04.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; received-spf: None (protection.outlook.com: coriant.com does not designate permitted sender hosts) spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: coriant.com X-MS-Exchange-CrossTenant-originalarrivaltime: 12 Jul 2016 05:27:30.8672 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 76595477-907e-4695-988b-a6b39087332d X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR04MB1337 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: Tue, 12 Jul 2016 05:27:32 -0000 Hello, > > > > -----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 > > > > correct location > > > > > > > > Fix the location of the rte_ring data dependency read barrier. > > > > It needs to be called before accessing indexed data to ensure that > > > > the 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 > > > > 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. >=20 > 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 reordered w= ith > earlier reads. Here is a simplified example sequence from time perspective: 1. Consumer CPU (CCPU) loads value y from r->ring[x] out-of-order=20 (the key of the problem) 2. Producer CPU (PCPU) updates r->ring[x] to value be z 3. PCPU updates prod_tail to be x 4. CCPU updates cons_head to be x 5. CCPU loads r->ring[x] by using out-of-order loaded value y [is z in real= ity] The problem here is that on weak memory model, the CCPU is allowed to load r->ring[x] value in advance, if it decides to do so (CCPU needs to be able = to see in advance that x will be an interesting index worth loading). The index va= lue x=20 is updated atomically, but it does not matter here. Also, the write barrie= r on PCPU=20 side guarantees that CCPU cannot see update of x before PCPU has really upd= ated=20 the r->ring[x] to z and moved the tail, but still allows to do the out-of-o= rder loads=20 without proper read barrier.=20 When the read barrier is moved between steps 4 and 5, it disallows to use=20 any out-of-order loads so far and forces to drop r->ring[x] y value and load current value z.=20 The ring queue appears to work well as this is a rare corner case. Due to t= he=20 head,tail-structure the problem needs queue to be full and also CCPU needs= =20 to see r->ring[x] update later than it does the out-of-order load. In addit= ion, the HW needs to be able to predict and choose the load to the future index= =20 (which should be quite possible, considering modern CPUs). If you have seen= =20 in the past problems and noticed that a larger ring queue works better as a= =20 workaround, you may have encountered the problem already. It is quite safe to move the barrier before DEQUEUE because after the DEQUE= UE=20 there is nothing really that we would want to protect with a read barrier. = The read=20 barrier is mapped to a compiler barrier on strong memory model systems and = this=20 works fine too as the order of the head,tail updates is still guaranteed on= the new=20 location. Even if the problem would be theoretical on most systems, it is w= orth fixing=20 as the risk for problems is very low. -- Juhamatti > Konstantin > > > > > > > > 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, > > > > you are hereby notified that any reproduction, dissemination or > > > > distribution of this communication is strictly prohibited. If you > > > > have 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