From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR01-DB5-obe.outbound.protection.outlook.com (mail-eopbgr150050.outbound.protection.outlook.com [40.107.15.50]) by dpdk.org (Postfix) with ESMTP id 7D58B1B485 for ; Thu, 31 Jan 2019 21:06:37 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector1-arm-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=utZvsXdAb84Y2Om3khaqHAS0qyVm+pldRlzNIzmih90=; b=NjRHCnSUvxTZE0vo2eGcW1Zyk45lsKzB9xRK5u4PMVKX0aXmrh6C/Mod/0M5319EAM4YzjAP3yQ2GIefY1fbs9J8FssomZnuyN1b8JYv1DKt2osuriRliBQvgZJzTa054dyQkcSPfP0DNwJ4kPqQJUlYJ8U2kz6ojFkwf2OJEQA= Received: from AM6PR08MB3672.eurprd08.prod.outlook.com (20.177.115.76) by AM6PR08MB3669.eurprd08.prod.outlook.com (20.177.115.26) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1558.23; Thu, 31 Jan 2019 20:06:36 +0000 Received: from AM6PR08MB3672.eurprd08.prod.outlook.com ([fe80::9120:87d6:b17c:fadd]) by AM6PR08MB3672.eurprd08.prod.outlook.com ([fe80::9120:87d6:b17c:fadd%3]) with mapi id 15.20.1580.017; Thu, 31 Jan 2019 20:06:36 +0000 From: Honnappa Nagarahalli To: "Phil Yang (Arm Technology China)" , "dev@dpdk.org" CC: nd , nd Thread-Topic: [dpdk-dev] [PATCH] eal/atomic: reimplement rte atomic APIs with atomic builtins Thread-Index: AQHUo0ikOocrLQmf902qYcw4PjLhcaXJ6Lbw Date: Thu, 31 Jan 2019 20:06:36 +0000 Message-ID: References: <1546508529-12227-1-git-send-email-phil.yang@arm.com> In-Reply-To: <1546508529-12227-1-git-send-email-phil.yang@arm.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=Honnappa.Nagarahalli@arm.com; x-originating-ip: [217.140.111.135] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; AM6PR08MB3669; 6:goLa/0jA08YzHZYcOspybAl5HO9QSKR6nxIwho047uf6y6uPf8tSgjZNJPj1UucbkP/c8lWVIwpYOFfRWfj9lj8/17qlk0YO4Wq6XkV1Ug5yJnCU8yPB3iWPd9VHnVKGJeqHYi4I6mIBS/P8hLUL0p87MXCjfiC6R70OolmPCw0l6E/zaAXRpQCq3pAnm5vigG0TJpQf7HhG3uBMbzYP6Q2ahKIUH+bOgg935qTlO3WtxlF+N786lQgjbRVD8Xp5GzXXKxnAkmKtBM3dhzCoWOvpasDxyPl+B/tazIm+tr2T/5kJ5aGsW0o4GkMNeyChD73c+PUFDZALZEsAHQeNX9c3L2hBiP/ZX4yanR65/gUXCUPUuzRh+zy9bAAVFuhlWxIak5WTNpivCcV7UHyQCCJ++FsQuP+hgXVCsaVBP8BuDpmPsoWGkHUcYiOTOjoJRkYrs+O7wN0a0TAZmFO/hw==; 5:OE+amugZMRAs7UYxBY3ZBl27k2GpWrRLccEsJl2DwfTnvdrmEeIDqzf/h1+YvcPU6nPy+mlxS6CU4hzG05Tuq7t6lf87TP7QJOcEr8YrAAfcLv4q9SU6YE0WPWK/qWNJC1IPx3eztUxEf7+x3e4d4nPta3xjOubo/XJC99BTWgh6tLkY9nX5E904V8BT9wU2KU2b2LpH7zxORRYQ2B710Q==; 7:d4XSO6IS11LinKk2lAsRWOf3zJ6Q3O7xShkhviBqRGwARc4s0CMNd7aUCXEggKAnI0w2gdtF/6WS25ND4Fx0ZH7wOVwKylbZOg1fkKE2LFSO+lQ7q+AcRIfo8vK9VF9o+VwbllzqvVdR6Z/6UhCCWg== x-ms-office365-filtering-correlation-id: 97c0e06c-5743-414b-b268-08d687b79abc x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600110)(711020)(4605077)(4618075)(2017052603328)(7153060)(7193020); SRVR:AM6PR08MB3669; x-ms-traffictypediagnostic: AM6PR08MB3669: nodisclaimer: True x-microsoft-antispam-prvs: x-forefront-prvs: 09347618C4 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(346002)(376002)(136003)(39860400002)(396003)(366004)(189003)(199004)(13464003)(7736002)(106356001)(81166006)(8936002)(53936002)(68736007)(446003)(11346002)(4326008)(26005)(66066001)(14454004)(486006)(71190400001)(6246003)(53546011)(105586002)(2906002)(74316002)(81156014)(72206003)(102836004)(305945005)(6506007)(71200400001)(476003)(186003)(86362001)(54906003)(3846002)(55016002)(9686003)(478600001)(6116002)(33656002)(97736004)(316002)(256004)(76176011)(7696005)(25786009)(229853002)(99286004)(6436002)(110136005)(2501003)(21314003); DIR:OUT; SFP:1101; SCL:1; SRVR:AM6PR08MB3669; H:AM6PR08MB3672.eurprd08.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: cReK63No0G3dxur2Dc7Qr0HoObv7s+muiBpYJ41JJWItmV79FKTFKXKsA6+VTcs8jq1YpsiVw5owFcEFAaWf2RUhyASCyZ85Bk6jdq1P8XPJT36Vz4cInWF4AwUlAMC+JrCyVlK+U9y3DcNfd3TRXYHYLnJ6LQtlkhkAVzYTcgVphjo98FVnISUFKF+BmlxvwnR/w29j8kX1BdvteGzzrDVQ5onAWowpW+KRTZ7Fda+DFaA8tJesQzdgxhIgCsToCOet2NaipwNZBUvKQ+GW77oiZcjFUy7GiQtC1RxNc/cKCkDFaPlPf8UHMmza98flQ8EzlRkhX6Fgsu9nxGolDSxadVTtvEIX9twOU0B0dvXF9Yj4FV6beMo0ELTBYdhpRj70v0rGCsVqaZxCpDUnsozN/xMfcRV/dkaNRUJ25To= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-Network-Message-Id: 97c0e06c-5743-414b-b268-08d687b79abc X-MS-Exchange-CrossTenant-originalarrivaltime: 31 Jan 2019 20:06:36.3225 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM6PR08MB3669 Subject: Re: [dpdk-dev] [PATCH] eal/atomic: reimplement rte atomic APIs 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: Thu, 31 Jan 2019 20:06:37 -0000 Hi Phil, The rte atomic APIs are not designed for adding memory model details. The = memory model to use comes from the API usage (and not from the definition).= For ex: rte_atomic32_add can have RELAXED memory ordering in a statistics= increment kind of use case, but may be RELEASE if the variable is used for= some kind of synchronization. Hence, I think it is not optimal to add memo= ry models to these implementations to gain performance across the board. At the same time, we do not need new APIs in DPDK as __atomic_xxx intrinsic= s act as those APIs. Instead, I think it is better to look at where these APIs are called from a= nd change that code to add the correct memory model using __atomic_xxx intr= insics. Thanks, Honnappa > -----Original Message----- > From: dev On Behalf Of Phil Yang > Sent: Thursday, January 3, 2019 3:42 AM > To: dev@dpdk.org > Cc: nd > Subject: [dpdk-dev] [PATCH] eal/atomic: reimplement rte atomic APIs with > atomic builtins >=20 > '__sync' builtins are deprecated, enable '__atomic' builtins for generic = atomic > operations. >=20 > Signed-off-by: Phil Yang > Reviewed-by: Gavin Hu > Tested-by: Phil Yang >=20 > --- > lib/librte_eal/common/include/generic/rte_atomic.h | 80 > ++++++++++++++++++++++ > 1 file changed, 80 insertions(+) >=20 > diff --git a/lib/librte_eal/common/include/generic/rte_atomic.h > b/lib/librte_eal/common/include/generic/rte_atomic.h > index b99ba46..260cdf3 100644 > --- a/lib/librte_eal/common/include/generic/rte_atomic.h > +++ b/lib/librte_eal/common/include/generic/rte_atomic.h > @@ -186,7 +186,12 @@ rte_atomic16_cmpset(volatile uint16_t *dst, > uint16_t exp, uint16_t src); static inline int rte_atomic16_cmpset(vola= tile > uint16_t *dst, uint16_t exp, uint16_t src) { > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION < 70100) > return __sync_bool_compare_and_swap(dst, exp, src); > +#else > + return __atomic_compare_exchange(dst, &exp, &src, 0, > __ATOMIC_ACQUIRE, > + __ATOMIC_ACQUIRE) ? 1 : 0; > +#endif > } > #endif >=20 > @@ -283,7 +288,11 @@ rte_atomic16_set(rte_atomic16_t *v, int16_t > new_value) static inline void rte_atomic16_add(rte_atomic16_t *v, int16= _t > inc) { > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION < 70100) > __sync_fetch_and_add(&v->cnt, inc); > +#else > + __atomic_fetch_add(&v->cnt, inc, __ATOMIC_ACQUIRE); #endif Could be __ATOMIC_RELAXED for a statistics use case. > } >=20 > /** > @@ -297,7 +306,11 @@ rte_atomic16_add(rte_atomic16_t *v, int16_t inc) > static inline void rte_atomic16_sub(rte_atomic16_t *v, int16_t dec) { > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION < 70100) > __sync_fetch_and_sub(&v->cnt, dec); > +#else > + __atomic_fetch_sub(&v->cnt, dec, __ATOMIC_ACQUIRE); #endif > } >=20 > /** > @@ -350,7 +363,11 @@ rte_atomic16_dec(rte_atomic16_t *v) static inline > int16_t rte_atomic16_add_return(rte_atomic16_t *v, int16_t inc) { > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION < 70100) > return __sync_add_and_fetch(&v->cnt, inc); > +#else > + return __atomic_add_fetch(&v->cnt, inc, __ATOMIC_ACQUIRE); > #endif > } >=20 > /** > @@ -370,7 +387,11 @@ rte_atomic16_add_return(rte_atomic16_t *v, > int16_t inc) static inline int16_t rte_atomic16_sub_return(rte_atomic16= _t > *v, int16_t dec) { > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION < 70100) > return __sync_sub_and_fetch(&v->cnt, dec); > +#else > + return __atomic_sub_fetch(&v->cnt, dec, __ATOMIC_ACQUIRE); > #endif > } >=20 > /** > @@ -389,7 +410,11 @@ static inline int > rte_atomic16_inc_and_test(rte_atomic16_t *v); #ifdef > RTE_FORCE_INTRINSICS static inline int > rte_atomic16_inc_and_test(rte_atomic16_t *v) { > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION < 70100) > return __sync_add_and_fetch(&v->cnt, 1) =3D=3D 0; > +#else > + return __atomic_add_fetch(&v->cnt, 1, __ATOMIC_ACQUIRE) =3D=3D 0; > #endif > } > #endif >=20 > @@ -409,7 +434,11 @@ static inline int > rte_atomic16_dec_and_test(rte_atomic16_t *v); #ifdef > RTE_FORCE_INTRINSICS static inline int > rte_atomic16_dec_and_test(rte_atomic16_t *v) { > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION < 70100) > return __sync_sub_and_fetch(&v->cnt, 1) =3D=3D 0; > +#else > + return __atomic_sub_fetch(&v->cnt, 1, __ATOMIC_ACQUIRE) =3D=3D 0; > #endif > } > #endif >=20 > @@ -469,7 +498,13 @@ rte_atomic32_cmpset(volatile uint32_t *dst, > uint32_t exp, uint32_t src); static inline int rte_atomic32_cmpset(vola= tile > uint32_t *dst, uint32_t exp, uint32_t src) { > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION < 70100) > return __sync_bool_compare_and_swap(dst, exp, src); > +#else > + return __atomic_compare_exchange(dst, &exp, &src, 0, > __ATOMIC_ACQUIRE, > + __ATOMIC_ACQUIRE) ? 1 : 0; These memory orderings would depend on the use case. > +#endif > + > } > #endif >=20 > @@ -566,7 +601,11 @@ rte_atomic32_set(rte_atomic32_t *v, int32_t > new_value) static inline void rte_atomic32_add(rte_atomic32_t *v, int32= _t > inc) { > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION < 70100) > __sync_fetch_and_add(&v->cnt, inc); > +#else > + __atomic_fetch_add(&v->cnt, inc, __ATOMIC_ACQUIRE); #endif > } >=20 > /** > @@ -580,7 +619,11 @@ rte_atomic32_add(rte_atomic32_t *v, int32_t inc) > static inline void rte_atomic32_sub(rte_atomic32_t *v, int32_t dec) { > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION < 70100) > __sync_fetch_and_sub(&v->cnt, dec); > +#else > + __atomic_fetch_sub(&v->cnt, dec, __ATOMIC_ACQUIRE); #endif > } >=20 > /** > @@ -633,7 +676,11 @@ rte_atomic32_dec(rte_atomic32_t *v) static inline > int32_t rte_atomic32_add_return(rte_atomic32_t *v, int32_t inc) { > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION < 70100) > return __sync_add_and_fetch(&v->cnt, inc); > +#else > + return __atomic_add_fetch(&v->cnt, inc, __ATOMIC_ACQUIRE); > #endif > } >=20 > /** > @@ -653,7 +700,11 @@ rte_atomic32_add_return(rte_atomic32_t *v, > int32_t inc) static inline int32_t rte_atomic32_sub_return(rte_atomic32= _t > *v, int32_t dec) { > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION < 70100) > return __sync_sub_and_fetch(&v->cnt, dec); > +#else > + return __atomic_sub_fetch(&v->cnt, dec, __ATOMIC_ACQUIRE); > #endif > } >=20 > /** > @@ -672,7 +723,11 @@ static inline int > rte_atomic32_inc_and_test(rte_atomic32_t *v); #ifdef > RTE_FORCE_INTRINSICS static inline int > rte_atomic32_inc_and_test(rte_atomic32_t *v) { > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION < 70100) > return __sync_add_and_fetch(&v->cnt, 1) =3D=3D 0; > +#else > + return __atomic_add_fetch(&v->cnt, 1, __ATOMIC_ACQUIRE) =3D=3D 0; > #endif > } > #endif >=20 > @@ -692,7 +747,11 @@ static inline int > rte_atomic32_dec_and_test(rte_atomic32_t *v); #ifdef > RTE_FORCE_INTRINSICS static inline int > rte_atomic32_dec_and_test(rte_atomic32_t *v) { > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION < 70100) > return __sync_sub_and_fetch(&v->cnt, 1) =3D=3D 0; > +#else > + return __atomic_sub_fetch(&v->cnt, 1, __ATOMIC_ACQUIRE) =3D=3D 0; > #endif > } > #endif >=20 > @@ -751,7 +810,12 @@ rte_atomic64_cmpset(volatile uint64_t *dst, > uint64_t exp, uint64_t src); static inline int rte_atomic64_cmpset(vola= tile > uint64_t *dst, uint64_t exp, uint64_t src) { > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION < 70100) > return __sync_bool_compare_and_swap(dst, exp, src); > +#else > + return __atomic_compare_exchange(dst, &exp, &src, 0, > __ATOMIC_ACQUIRE, > + __ATOMIC_ACQUIRE) ? 1 : 0; > +#endif > } > #endif >=20 > @@ -902,7 +966,11 @@ rte_atomic64_add(rte_atomic64_t *v, int64_t inc); > static inline void rte_atomic64_add(rte_atomic64_t *v, int64_t inc) { > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION < 70100) > __sync_fetch_and_add(&v->cnt, inc); > +#else > + __atomic_fetch_add(&v->cnt, inc, __ATOMIC_ACQUIRE); #endif > } > #endif >=20 > @@ -921,7 +989,11 @@ rte_atomic64_sub(rte_atomic64_t *v, int64_t dec); > static inline void rte_atomic64_sub(rte_atomic64_t *v, int64_t dec) { > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION < 70100) > __sync_fetch_and_sub(&v->cnt, dec); > +#else > + __atomic_fetch_sub(&v->cnt, dec, __ATOMIC_ACQUIRE); #endif > } > #endif >=20 > @@ -979,7 +1051,11 @@ rte_atomic64_add_return(rte_atomic64_t *v, > int64_t inc); static inline int64_t rte_atomic64_add_return(rte_atomic6= 4_t > *v, int64_t inc) { > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION < 70100) > return __sync_add_and_fetch(&v->cnt, inc); > +#else > + return __atomic_add_fetch(&v->cnt, inc, __ATOMIC_ACQUIRE); > #endif > } > #endif >=20 > @@ -1003,7 +1079,11 @@ rte_atomic64_sub_return(rte_atomic64_t *v, > int64_t dec); static inline int64_t rte_atomic64_sub_return(rte_atomic6= 4_t > *v, int64_t dec) { > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION < 70100) > return __sync_sub_and_fetch(&v->cnt, dec); > +#else > + return __atomic_sub_fetch(&v->cnt, dec, __ATOMIC_ACQUIRE); > #endif > } > #endif >=20 > -- > 2.7.4