From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id DCB0A2BF5; Thu, 20 Dec 2018 00:50:59 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 19 Dec 2018 15:50:58 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,374,1539673200"; d="scan'208";a="111953089" Received: from irsmsx154.ger.corp.intel.com ([163.33.192.96]) by orsmga003.jf.intel.com with ESMTP; 19 Dec 2018 15:50:56 -0800 Received: from irsmsx106.ger.corp.intel.com ([169.254.8.227]) by IRSMSX154.ger.corp.intel.com ([169.254.12.217]) with mapi id 14.03.0415.000; Wed, 19 Dec 2018 23:50:55 +0000 From: "Ananyev, Konstantin" To: Joyce Kong , "dev@dpdk.org" CC: "nd@arm.com" , Gavin Hu , "thomas@monjalon.net" , "jerin.jacob@caviumnetworks.com" , "hemant.agrawal@nxp.com" , "honnappa.nagarahalli@arm.com" , "stable@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v1 2/2] rwlock: reimplement with __atomic builtins Thread-Index: AQHUkpVVrzSffE7wUkqJnq61Wa8rWKWGwduQ Date: Wed, 19 Dec 2018 23:50:54 +0000 Message-ID: <2601191342CEEE43887BDE71AB977258010D8BCD7E@IRSMSX106.ger.corp.intel.com> References: <1544672265-219262-1-git-send-email-joyce.kong@arm.com> <1544672265-219262-3-git-send-email-joyce.kong@arm.com> In-Reply-To: <1544672265-219262-3-git-send-email-joyce.kong@arm.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMTgyMzZlNjUtYzdmMi00ZjQ1LTkzMGUtOTY1Y2Q4ZmRkMTVhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiWE1BNTFzcjVOUys1dUszb2p1djNYd0dhR3plZitFcjIyWnhsRG1hbU82dFBIYUpyTVh2eFRPQWF4b21TSEFRYSJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v1 2/2] rwlock: reimplement with __atomic builtins 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, 19 Dec 2018 23:51:00 -0000 > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Joyce Kong > Sent: Thursday, December 13, 2018 3:38 AM > To: dev@dpdk.org > Cc: nd@arm.com; Gavin Hu ; thomas@monjalon.net; jerin.j= acob@caviumnetworks.com; hemant.agrawal@nxp.com; > honnappa.nagarahalli@arm.com; joyce.kong@arm.com; stable@dpdk.org > Subject: [dpdk-dev] [PATCH v1 2/2] rwlock: reimplement with __atomic buil= tins >=20 > From: Gavin Hu >=20 > The __sync builtin based implementation generates full memory barriers > ('dmb ish') on Arm platforms. Using C11 atomic builtins to generate one > way barriers. >=20 > Here is the assembly code of __sync_compare_and_swap builtin. > __sync_bool_compare_and_swap(dst, exp, src); > 0x000000000090f1b0 <+16>: e0 07 40 f9 ldr x0, [sp, #8] > 0x000000000090f1b4 <+20>: e1 0f 40 79 ldrh w1, [sp, #6] > 0x000000000090f1b8 <+24>: e2 0b 40 79 ldrh w2, [sp, #4] > 0x000000000090f1bc <+28>: 21 3c 00 12 and w1, w1, #0xffff > 0x000000000090f1c0 <+32>: 03 7c 5f 48 ldxrh w3, [x0] > 0x000000000090f1c4 <+36>: 7f 00 01 6b cmp w3, w1 > 0x000000000090f1c8 <+40>: 61 00 00 54 b.ne 0x90f1d4 > // b.any > 0x000000000090f1cc <+44>: 02 fc 04 48 stlxrh w4, w2, [x0] > 0x000000000090f1d0 <+48>: 84 ff ff 35 cbnz w4, 0x90f1c0 > > 0x000000000090f1d4 <+52>: bf 3b 03 d5 dmb ish > 0x000000000090f1d8 <+56>: e0 17 9f 1a cset w0, eq // eq =3D non= e >=20 > Fixes: af75078faf ("first public release") > Cc: stable@dpdk.org >=20 > Signed-off-by: Gavin Hu > Reviewed-by: Honnappa Nagarahalli > Reviewed-by: Ola Liljedahl > Reviewed-by: Joyce Kong > Tested-by: Joyce Kong > --- > lib/librte_eal/common/include/generic/rte_rwlock.h | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) Looks ok to me in general, but I think needs to run extra perf testing to check for regression/correctness on other platforms (obviously IA is my = main concern). Another thing - my personal preference would be to have it rebased with '_t= rylock' functions also updated - for consistency, plus extra functional tests will = be available for new method.=20 >=20 > diff --git a/lib/librte_eal/common/include/generic/rte_rwlock.h b/lib/lib= rte_eal/common/include/generic/rte_rwlock.h > index 5751a0e..51d3aac 100644 > --- a/lib/librte_eal/common/include/generic/rte_rwlock.h > +++ b/lib/librte_eal/common/include/generic/rte_rwlock.h > @@ -64,14 +64,14 @@ rte_rwlock_read_lock(rte_rwlock_t *rwl) > int success =3D 0; >=20 > while (success =3D=3D 0) { > - x =3D rwl->cnt; > + x =3D __atomic_load_n(&rwl->cnt, __ATOMIC_RELAXED); > /* write lock is held */ > if (x < 0) { > rte_pause(); > continue; > } > - success =3D rte_atomic32_cmpset((volatile uint32_t *)&rwl->cnt, > - (uint32_t)x, (uint32_t)(x + 1)); > + success =3D __atomic_compare_exchange_n(&rwl->cnt, &x, x+1, 1, As a nit: 'x + 1' > + __ATOMIC_ACQUIRE, __ATOMIC_RELAXED); > } > } >=20 > @@ -84,7 +84,7 @@ rte_rwlock_read_lock(rte_rwlock_t *rwl) > static inline void > rte_rwlock_read_unlock(rte_rwlock_t *rwl) > { > - rte_atomic32_dec((rte_atomic32_t *)(intptr_t)&rwl->cnt); > + __atomic_fetch_sub(&rwl->cnt, 1, __ATOMIC_RELEASE); > } >=20 > /** > @@ -100,14 +100,14 @@ rte_rwlock_write_lock(rte_rwlock_t *rwl) > int success =3D 0; >=20 > while (success =3D=3D 0) { > - x =3D rwl->cnt; > + x =3D __atomic_load_n(&rwl->cnt, __ATOMIC_RELAXED); > /* a lock is held */ > if (x !=3D 0) { > rte_pause(); > continue; > } > - success =3D rte_atomic32_cmpset((volatile uint32_t *)&rwl->cnt, > - 0, (uint32_t)-1); > + success =3D __atomic_compare_exchange_n(&rwl->cnt, &x, -1, 1, > + __ATOMIC_ACQUIRE, __ATOMIC_RELAXED); > } > } >=20 > @@ -120,7 +120,7 @@ rte_rwlock_write_lock(rte_rwlock_t *rwl) > static inline void > rte_rwlock_write_unlock(rte_rwlock_t *rwl) > { > - rte_atomic32_inc((rte_atomic32_t *)(intptr_t)&rwl->cnt); > + __atomic_store_n(&rwl->cnt, 0, __ATOMIC_RELEASE); > } >=20 > /** > -- > 2.7.4