From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR01-VE1-obe.outbound.protection.outlook.com (mail-ve1eur01on0062.outbound.protection.outlook.com [104.47.1.62]) by dpdk.org (Postfix) with ESMTP id D07041B60E for ; Mon, 16 Oct 2017 12:51:35 +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=xBjiXOzppz8eD86CVJgH/zqao31YUoJHG4qFLgsBCmk=; b=ZY0Je0DcVfWAG4kh6I7ybi0pd3Df8O2K+uToFuaYUDLLSK3IqdkLoyfzrNeqHP+6IVsVUPhNDNdO/I1jMWg+7rCSfJtFte1M1Msjkw9jwQBhx00LVJMqTCRkBChlwiv+/T23+c6JUAJj2/ze3IbigwXn5GivZ6+v3Sp72yaCVvM= Received: from AM6PR0402MB3398.eurprd04.prod.outlook.com (52.133.19.15) by AM6PR0402MB3399.eurprd04.prod.outlook.com (52.133.19.16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.77.7; Mon, 16 Oct 2017 10:51:35 +0000 Received: from AM6PR0402MB3398.eurprd04.prod.outlook.com ([fe80::54b9:e731:10a7:7e12]) by AM6PR0402MB3398.eurprd04.prod.outlook.com ([fe80::54b9:e731:10a7:7e12%13]) with mapi id 15.20.0077.022; Mon, 16 Oct 2017 10:51:34 +0000 From: "Kuusisaari, Juhamatti" To: "Liu, Jie2" , "Ananyev, Konstantin" , Olivier MATZ , "dev@dpdk.org" , "jerin.jacob@caviumnetworks.com" CC: "He, Jia" , "Zhao, Bing" , Jia He Thread-Topic: [PATCH] ring: guarantee ordering of cons/prod loading when doing enqueue/dequeue Thread-Index: AQHTQ3xl5Run/J4MH0auTk3nx9ZKBqLg7F0AgAVlVxA= Date: Mon, 16 Oct 2017 10:51:34 +0000 Message-ID: References: <20171010095636.4507-1-hejianet@gmail.com> <20171012155350.j34ddtivxzd27pag@platinum> <2601191342CEEE43887BDE71AB9772585FAA859F@IRSMSX103.ger.corp.intel.com> <3dc359ce73394a5293c77109f8b1d717@HXTBJIDCEMVIW01.hxtcorp.net> In-Reply-To: <3dc359ce73394a5293c77109f8b1d717@HXTBJIDCEMVIW01.hxtcorp.net> 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.134.175] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; AM6PR0402MB3399; 6:HLcqeuvDJGbBc9Dlj9iS51DooHC59PYNzspLMSohGqdh0Px25a10VYIHYxwoo9i7JZe2otUYai48y+RXPQRTP+DcLV2C1pUcIE6HPgcOpz0zNsgE+FKCvJUbxzrjd6HfNKfZ2KU+ojCMGwIK+SDg5Q+NOkG8QdzI7VABYq+5ZkxdzHTPRwV3l1xViMODWuEaHaXwyzAlvxIMAPPHUSXF5EdNUelp707Q8VOEMl9wIjtHoCwjLABTiA3XDjnayczvWbDQoANKdnjFKmtesD23ybBeFO9f2vlDzTsLk9C3gSBiOYLE1wf0GBvytfN6/661c4VPZGhiCSGSkfLcPVCk+g==; 5:SUpW4YLyf2L4rM2sBG8EBDSwMf0+5vCQRC6vopU1RPQUXWaRPDlMMjTTCcyCmVL2h46Z1bxd+uCwWI+H6rPouDYEkdkgNYq9CFFsn34lKvPk7Undrg7q8MpIJHktNY2tZGZQj3uDuQGGBnZuj9TLm3e8UCvjGdXdT+ByxmJW+ic=; 24:ei/vELQzhOzRw4ZgirX8wabe88SWwd/MtO3dEL+fyjqaJe12qjPkbsYCGs9PJSQkqjt71nQMo1qNZnMTIjbopaJNI2YkgAabV7CA+CRJAcQ=; 7:54SjIKrbfY8h2mQDGHmg5dwgLAa48f4Ibbnt9v/7LKibqDYmJHtrPQjrYtt9f4H3iKZjWYry+7b1A1laG1+Nv0xIAsk/OFr4ISvElvNGM9R9T5QgwwlX38nR5ocj9ZTpqZ9DlfQuGjRa5z7berTQhBZpl/v0h7Wn9yhKUH2SV2FEhRfr4gJKCEtgwL1ubMSw6vhxJlYI22IDq1vceCyP8pdcs6WjHKnA8zpLQNJmj0E= x-ms-exchange-antispam-srfa-diagnostics: SSOS; x-ms-office365-filtering-correlation-id: f5adb3bc-697c-4e54-da93-08d51483de7b x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(22001)(2017030254152)(48565401081)(2017052603199)(201703131423075)(201703031133081)(201702281549075); SRVR:AM6PR0402MB3399; x-ms-traffictypediagnostic: AM6PR0402MB3399: x-exchange-antispam-report-test: UriScan:(228905959029699); x-microsoft-antispam-prvs: x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(8121501046)(5005006)(3002001)(100000703101)(100105400095)(10201501046)(93006095)(93001095)(6055026)(6041248)(20161123558100)(20161123564025)(20161123560025)(20161123562025)(20161123555025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095); SRVR:AM6PR0402MB3399; BCL:0; PCL:0; RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095); SRVR:AM6PR0402MB3399; x-forefront-prvs: 0462918D61 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(6009001)(39860400002)(376002)(346002)(13464003)(24454002)(377454003)(199003)(189002)(51234002)(53376002)(8936002)(50986999)(76176999)(54356999)(3846002)(6246003)(102836003)(6116002)(39060400002)(101416001)(4326008)(93886005)(3660700001)(2906002)(33656002)(9686003)(6306002)(99286003)(68736007)(81156014)(53936002)(3280700002)(5250100002)(7696004)(5660300001)(55016002)(6506006)(6436002)(81166006)(8676002)(305945005)(2501003)(74316002)(25786009)(2900100001)(66066001)(316002)(189998001)(53546010)(14454004)(110136005)(54906003)(97736004)(105586002)(106356001)(2201001)(966005)(86362001)(72206003)(478600001)(229853002)(7736002)(2950100002); DIR:OUT; SFP:1101; SCL:1; SRVR:AM6PR0402MB3399; H:AM6PR0402MB3398.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="iso-2022-jp" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: coriant.com X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Oct 2017 10:51:34.7785 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 76595477-907e-4695-988b-a6b39087332d X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM6PR0402MB3399 Subject: Re: [dpdk-dev] [PATCH] ring: guarantee ordering of cons/prod loading when doing enqueue/dequeue 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: Mon, 16 Oct 2017 10:51:36 -0000 > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Liu, Jie2 > Sent: Friday, October 13, 2017 3:25 AM > To: Ananyev, Konstantin ; Olivier MATZ > ; dev@dpdk.org; > jerin.jacob@caviumnetworks.com > Cc: He, Jia ; Zhao, Bing semitech.com>; Jia He > Subject: Re: [dpdk-dev] [PATCH] ring: guarantee ordering of cons/prod > loading when doing enqueue/dequeue >=20 > Hi guys, > We found this issue when we run mbuf_autotest. It failed on a aarch64 > platform. I am not sure if it can be reproduced on other platforms. > Regards, > Jie Liu >=20 > -----Original Message----- > From: Ananyev, Konstantin [mailto:konstantin.ananyev@intel.com] > Sent: 2017=1B$BG/=1B(B10=1B$B7n=1B(B13=1B$BF|=1B(B 1:06 > To: Olivier MATZ ; Jia He > Cc: dev@dpdk.org; He, Jia ; Liu, Jie2 > ; Zhao, Bing ; > jerin.jacob@caviumnetworks.com > Subject: RE: [PATCH] ring: guarantee ordering of cons/prod loading when > doing enqueue/dequeue >=20 > Hi guys, >=20 > > -----Original Message----- > > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > > Sent: Thursday, October 12, 2017 4:54 PM > > To: Jia He > > Cc: dev@dpdk.org; jia.he@hxt-semitech.com; jie2.liu@hxt-semitech.com; > > bing.zhao@hxt-semitech.com; Ananyev, Konstantin > > ; jerin.jacob@caviumnetworks.com > > Subject: Re: [PATCH] ring: guarantee ordering of cons/prod loading > > when doing enqueue/dequeue > > > > Hi, > > > > On Tue, Oct 10, 2017 at 05:56:36PM +0800, Jia He wrote: > > > Before this patch: > > > In __rte_ring_move_cons_head() > > > ... > > > do { > > > /* Restore n as it may change every loop */ > > > n =3D max; > > > > > > *old_head =3D r->cons.head; //1st load > > > const uint32_t prod_tail =3D r->prod.tail; //2nd load > > > > > > In weak memory order architectures(powerpc,arm), the 2nd load might > > > be reodered before the 1st load, that makes *entries is bigger than w= e > wanted. > > > This nasty reording messed enque/deque up. > > > > > > 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,t= ail} > > > load r->cons.head > > > > > > THEN,r->cons.head will be bigger than prod_tail, then make *entries > > > very big > > > > > > After this patch, the old cons.head will be recaculated after > > > failure of rte_atomic32_cmpset > > > > > > There is no such issue in X86 cpu, because X86 is strong memory > > > order model > > > > > > Signed-off-by: Jia He > > > Signed-off-by: jia.he@hxt-semitech.com > > > Signed-off-by: jie2.liu@hxt-semitech.com > > > Signed-off-by: bing.zhao@hxt-semitech.com > > > > > > --- > > > lib/librte_ring/rte_ring.h | 8 ++++++++ > > > 1 file changed, 8 insertions(+) > > > > > > diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h > > > index 5e9b3b7..15c72e2 100644 > > > --- a/lib/librte_ring/rte_ring.h > > > +++ b/lib/librte_ring/rte_ring.h > > > @@ -409,6 +409,10 @@ __rte_ring_move_prod_head(struct rte_ring *r, > > > int is_sp, n =3D max; > > > > > > *old_head =3D r->prod.head; > > > + > > > +/* load of prod.tail can't be reordered before cons.head */ > > > +rte_smp_rmb(); > > > + > > > const uint32_t cons_tail =3D r->cons.tail; > > > /* > > > * The subtraction is done between two unsigned 32bits value @@ > > > -517,6 +521,10 @@ __rte_ring_move_cons_head(struct rte_ring *r, int > > > is_sc, n =3D max; > > > > > > *old_head =3D r->cons.head; > > > + > > > +/* load of prod.tail can't be reordered before cons.head */ > > > +rte_smp_rmb(); > > > + > > > const uint32_t prod_tail =3D r->prod.tail; > > > /* The subtraction is done between two unsigned 32bits value > > > * (the result is always modulo 32 bits even if we have > > > -- > > > 2.7.4 > > > > > > > The explanation convinces me. > > > > However, since it's in a critical path, it would be good to have other > > opinions. This patch reminds me this discussion, that was also related > > to memory barrier, but at another place: > > http://dpdk.org/ml/archives/dev/2016-July/043765.html > > Lead to that patch: > > http://dpdk.org/browse/dpdk/commit/?id=3Decc7d10e448e > > But finally reverted: > > http://dpdk.org/browse/dpdk/commit/?id=3Dc3acd92746c3 > > > > Konstatin, Jerin, do you have any comment? >=20 > For IA, as rte_smp_rmb() is just a compiler_barrier, that patch shouldn't > make any difference, but I can't see how read reordering would screw thi= ngs > up here... > Probably just me and arm or ppc guys could explain what will be the probl= em > if let say cons.tail will be read before prod.head in > __rte_ring_move_prod_head(). > I wonder Is there a simple test-case to reproduce that problem (on arm or > ppc)? > Probably new test-case for rte_ring autotest is needed, or is it possible= to > reproduce it with existing one? > Konstantin Hi, I think this is a real problem here. We have fixed it so that we read both = head and tail atomically as otherwise you may have this problem you are see= ing. Works only on 64, of course. Thanks for pointing out the revert, I was unaware of it: http://dpdk.org/browse/dpdk/commit/?id=3Dc3acd92746c3 Nevertheless, the original fix was as such correct on higher level, both PP= C and ARM just happened to use strong enough default read barrier which alr= eady guaranteed the ordering to be good enough. As we optimize cycles here,= I am of course just fine with the revert as long as we all remember what w= as going on there. BR, -- Juhamatti