From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 4632FA057B;
	Wed, 18 Mar 2020 15:01:35 +0100 (CET)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 3FCBA1AFF;
	Wed, 18 Mar 2020 15:01:34 +0100 (CET)
Received: from mga05.intel.com (mga05.intel.com [192.55.52.43])
 by dpdk.org (Postfix) with ESMTP id E3AD1FEB
 for <dev@dpdk.org>; Wed, 18 Mar 2020 15:01:32 +0100 (CET)
IronPort-SDR: 1vYYlSjRqFz1M8JJM8xW6ve0hhV/Jg93AV4GzC15MMKupwQtquTXX7mMLwHBQXlIh1Ue+guHhl
 xzNtqYyG3Utg==
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from orsmga008.jf.intel.com ([10.7.209.65])
 by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
 18 Mar 2020 07:01:32 -0700
IronPort-SDR: D1aBLbsMVUb2kZwLsIZkggBcaGj9zglFhUIK92HG0Od4dVe4hlRlgFeATZVwtuUNHFb5vNSj6F
 ByWMbyBPk37Q==
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.70,567,1574150400"; d="scan'208";a="238615123"
Received: from orsmsx101.amr.corp.intel.com ([10.22.225.128])
 by orsmga008.jf.intel.com with ESMTP; 18 Mar 2020 07:01:31 -0700
Received: from orsmsx155.amr.corp.intel.com (10.22.240.21) by
 ORSMSX101.amr.corp.intel.com (10.22.225.128) with Microsoft SMTP Server (TLS)
 id 14.3.439.0; Wed, 18 Mar 2020 07:01:31 -0700
Received: from ORSEDG002.ED.cps.intel.com (10.7.248.5) by
 ORSMSX155.amr.corp.intel.com (10.22.240.21) with Microsoft SMTP Server (TLS)
 id 14.3.439.0; Wed, 18 Mar 2020 07:01:31 -0700
Received: from NAM12-MW2-obe.outbound.protection.outlook.com (104.47.66.42) by
 edgegateway.intel.com (134.134.137.101) with Microsoft SMTP Server
 (TLS) id 14.3.439.0; Wed, 18 Mar 2020 07:01:31 -0700
ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none;
 b=Kkca1x89mwTk5PQYdTXZODu+maX/iXZsB+1gKmf+TmxhRC4M7afeJHMnKNCOF65GZkDkn1nBEk7q9QsZtXDzAALbfAeIkxi+ixCN+ui0ffhuFIDvtvgR6qTLXQchQPL73XCOuH3mXasZkVPQ96DJ8PivFiXsOvmI6QdhSY0t1lPbrOwOFBtfXjDLPYSxYpBj26Ub47QvAjlV8rJ4DWmJ7wSCGH19rl9u/fOx60wMQjymYcEyyvsFGfKwsxeZ4Ra11kQ53z8PjzhEjkGqUg8EoiMZCO2F1VUO+uL6iTCQ/BcThFor01noMakxxN54mWXDqY061/R9jToe7e+fSPj+bA==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; 
 s=arcselector9901;
 h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;
 bh=QvvYAx5lci8pb0TpW0jnvddSCBGraU2uL5xC1XqgDFc=;
 b=Z8aVBPJMegXa/IQfN/oCJw59t/qcjEWNlZiwJzr/7GbxvpwnHCGWt8/3PfGa2Q7qhIPct9ZMD/xbcwz5sHXws/A5QuWXIu3H0D/rGELpeenxIklkpJcTt18z5sGtArjEzX6sn+Vau1cvghFk7/iMuAPtmWgVBsTme3dXO0cGjDGWl7DUkio2nnFoQ7F2gRjhM/9iKHCpqSifmlOexjCvxt1wKcSaWmce2Bnp9Gsu2rmpRQrn2S6/FxqWh8UfczxLtMkMsrxBvPsHAvne/xybpdcXH4c/XaRWPTYgDw9Ku6oZewCOiTXF6OkKxbrM+6uTFmgMR0b5yIre6qU+SGvIjw==
ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass
 smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com;
 dkim=pass header.d=intel.com; arc=none
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; 
 s=selector2-intel-onmicrosoft-com;
 h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;
 bh=QvvYAx5lci8pb0TpW0jnvddSCBGraU2uL5xC1XqgDFc=;
 b=mZnsXF9VgMkVNXPeEeJ6tMeV6Kxp8XgLLnIxMvGZIjrNwBP7lgC+cjUEr4c8ZllM5aKi8C+lodLVeI0xk1lLhM0FT0xzRg9+UmQ2A7flaFsrxcUay0cL8+nfEH37qTKikyc022BdCUC15wJtCqFe1vZzIzmu9Ghrp+OlBKiaRbg=
Received: from MWHPR1101MB2157.namprd11.prod.outlook.com
 (2603:10b6:301:51::10) by MWHPR1101MB2286.namprd11.prod.outlook.com
 (2603:10b6:301:5b::9) with Microsoft SMTP Server (version=TLS1_2,
 cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2835.19; Wed, 18 Mar
 2020 14:01:29 +0000
Received: from MWHPR1101MB2157.namprd11.prod.outlook.com
 ([fe80::7d16:3f1d:52c9:3116]) by MWHPR1101MB2157.namprd11.prod.outlook.com
 ([fe80::7d16:3f1d:52c9:3116%5]) with mapi id 15.20.2835.017; Wed, 18 Mar 2020
 14:01:29 +0000
From: "Van Haaren, Harry" <harry.van.haaren@intel.com>
To: Phil Yang <phil.yang@arm.com>, "thomas@monjalon.net"
 <thomas@monjalon.net>, "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
 "stephen@networkplumber.org" <stephen@networkplumber.org>,
 "maxime.coquelin@redhat.com" <maxime.coquelin@redhat.com>, "dev@dpdk.org"
 <dev@dpdk.org>
CC: "david.marchand@redhat.com" <david.marchand@redhat.com>,
 "jerinj@marvell.com" <jerinj@marvell.com>, "hemant.agrawal@nxp.com"
 <hemant.agrawal@nxp.com>, "Honnappa.Nagarahalli@arm.com"
 <Honnappa.Nagarahalli@arm.com>, "gavin.hu@arm.com" <gavin.hu@arm.com>,
 "ruifeng.wang@arm.com" <ruifeng.wang@arm.com>, "joyce.kong@arm.com"
 <joyce.kong@arm.com>, "nd@arm.com" <nd@arm.com>
Thread-Topic: [PATCH v3 00/12] generic rte atomic APIs deprecate proposal
Thread-Index: AQHV+/oHYvInwZkrnEm8WRIHQJ4aXqhOXJwQ
Date: Wed, 18 Mar 2020 14:01:29 +0000
Message-ID: <MWHPR1101MB21570F1EDF906A7B3FBF37E1D7F70@MWHPR1101MB2157.namprd11.prod.outlook.com>
References: <1583999071-22872-1-git-send-email-phil.yang@arm.com>
 <1584407863-774-1-git-send-email-phil.yang@arm.com>
In-Reply-To: <1584407863-774-1-git-send-email-phil.yang@arm.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
dlp-product: dlpe-windows
dlp-reaction: no-action
dlp-version: 11.2.0.6
authentication-results: spf=none (sender IP is )
 smtp.mailfrom=harry.van.haaren@intel.com; 
x-originating-ip: [192.198.151.174]
x-ms-publictraffictype: Email
x-ms-office365-filtering-correlation-id: dbb6506c-7369-428a-63eb-08d7cb44db67
x-ms-traffictypediagnostic: MWHPR1101MB2286:
x-ms-exchange-transport-forked: True
x-microsoft-antispam-prvs: <MWHPR1101MB2286410DD972CABC0C23DF9DD7F70@MWHPR1101MB2286.namprd11.prod.outlook.com>
x-ms-oob-tlc-oobclassifiers: OLM:9508;
x-forefront-prvs: 03468CBA43
x-forefront-antispam-report: SFV:NSPM;
 SFS:(10019020)(366004)(199004)(2906002)(86362001)(54906003)(110136005)(33656002)(53546011)(6506007)(7696005)(8936002)(8676002)(498600001)(7416002)(81156014)(81166006)(66476007)(76116006)(5660300002)(66946007)(66556008)(64756008)(66446008)(52536014)(71200400001)(4326008)(26005)(9686003)(186003)(55016002)(41533002);
 DIR:OUT; SFP:1102; SCL:1; SRVR:MWHPR1101MB2286;
 H:MWHPR1101MB2157.namprd11.prod.outlook.com; FPR:; SPF:None; LANG:en;
 PTR:InfoNoRecords; A:1; 
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info: rNvYNHjpDoWVkcwAVdqn3xxhNl8Ar9qRAX3u74HnV/8x9P9L26HeT+lYoVs4opNnFrJMA6gqYmTWtStJ0ESu+8MnsBUl/o1B38+BirjihSB/J6EtoAddZwt6fowujlSmzIRsVO+2u+UY5QEBaYL8xkQLFHmRDOo2UdDAYNBByB66wLiqaeqmOpj11wWhX5N+fQ+dl2JeuUItBGHTs9j3HAm4OgkR1eO6p1xKJ7Gzuvsm0n2djl9spsyN/mSQpKjkx/LI6GFQnZHq2g1QKTmZ6fqwyp8CP0R9rGJCzvx2wXHyVlFuCEzxGqlyj/2G7d9ptIZiGyj6n3bCWBRoV3oT09X5w2D/H9GeMdtVOBzH38KgCehLOJ9ItZKvdaD668nK4gjaipS3GJHVp5xAEb/xMNeCJCIOLY05NSEBnjPS+0CowJkP4d0K1Q9lPI12SoSX/FBqxAm/GU+uCPSYkwM1ufZKfG/UtcT/IoimY9oQ75wCJrc2+zn2o42aMl4aQRW3
x-ms-exchange-antispam-messagedata: huh+qzN92QFADqEESGR7SPrLsrPYHCX/zwVCUR2Lu11TTvb8LIe+KsWUCFbmGbEHFBlGb6GhK3uvMbfRr4+F/f689yUagZdwkdvCBfescdBIQzueCt15ZnCCrhIjhEanauKqYpWnPCF0csGK+ihtzw==
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: dbb6506c-7369-428a-63eb-08d7cb44db67
X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Mar 2020 14:01:29.3995 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: HTMSOGD5w8fYEjRWQ1D7fGCAYQUNBOIMY6d5yEOtqVicXvEKx6BETTOP/xi+5nEFeD1Xo2Sypemz1BurfB3UmLmHjkMnJMDpKQDaP+PVgj0=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR1101MB2286
X-OriginatorOrg: intel.com
Subject: Re: [dpdk-dev] [PATCH v3 00/12] generic rte atomic APIs deprecate
	proposal
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

Hi Phil & Honnappa,

> -----Original Message-----
> From: Phil Yang <phil.yang@arm.com>
> Sent: Tuesday, March 17, 2020 1:18 AM
> To: thomas@monjalon.net; Van Haaren, Harry <harry.van.haaren@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> stephen@networkplumber.org; maxime.coquelin@redhat.com; dev@dpdk.org
> Cc: david.marchand@redhat.com; jerinj@marvell.com; hemant.agrawal@nxp.com=
;
> Honnappa.Nagarahalli@arm.com; gavin.hu@arm.com; ruifeng.wang@arm.com;
> joyce.kong@arm.com; nd@arm.com
> Subject: [PATCH v3 00/12] generic rte atomic APIs deprecate proposal
>=20
> DPDK provides generic rte_atomic APIs to do several atomic operations.
> These APIs are using the deprecated __sync built-ins and enforce full
> memory barriers on aarch64. However, full barriers are not necessary
> in many use cases. In order to address such use cases, C language offers
> C11 atomic APIs. The C11 atomic APIs provide finer memory barrier control
> by making use of the memory ordering parameter provided by the user.
> Various patches submitted in the past [2] and the patches in this series
> indicate significant performance gains on multiple aarch64 CPUs and no
> performance loss on x86.
>=20
> But the existing rte_atomic API implementations cannot be changed as the
> APIs do not take the memory ordering parameter. The only choice available
> is replacing the usage of the rte_atomic APIs with C11 atomic APIs. In
> order to make this change, the following steps are proposed:
>=20
> [1] deprecate rte_atomic APIs so that future patches do not use rte_atomi=
c
> APIs (a script is added to flag the usages).
> [2] refactor the code that uses rte_atomic APIs to use c11 atomic APIs.

On [1] above, I feel deprecating DPDKs atomic functions and failing checkpa=
tch is
a bit sudden. Perhaps noting that in a future release (20.11?) DPDK will mo=
ve to a
C11 based atomics model is a more gradual step to achieving the goal, and a=
t that
point add a checkpatch warning for additions of rte_atomic*?

More on [2] in context below.

The above is my point-of-view, of course I'd like more people from the DPDK=
 community
to provide their input too.


> This patchset contains:
> 1) the checkpatch script changes to flag rte_atomic API usage in patches.
> 2) changes to programmer guide describing writing efficient code for aarc=
h64.
> 3) changes to various libraries to make use of c11 atomic APIs.
>=20
> We are planning to replicate this idea across all the other libraries,
> drivers, examples, test applications. In the next phase, we will add
> changes to the mbuf, the EAL interrupts and the event timer adapter libra=
ries.

About ~6/12 patches of this C11 set are targeting the Service Cores area of=
 DPDK. I have some concerns
over increased complexity of C11 implementation vs the (already complex) rt=
e_atomic implementation today.
I see other patchsets enabling C11 across other DPDK components, so maybe w=
e should also discuss C11
enabling in a wider context that just service cores?

I don't think it fair to expect all developers to be well versed in C11 ato=
mic semantics like
understanding the complex interactions between the various C11 RELEASE, AQU=
IRE barriers requires.

As maintainer of Service Cores I'm hesitant to accept the large-scale refac=
tor of atomic-implementation,
as it could lead to racey bugs that are likely extremely difficult to track=
 down. (The recent race-on-exit
has proven the difficulty in reproducing, and that's with an atomics model =
I'm quite familiar with).

Let me be very clear: I don't wish to block a C11 atomic implementation, bu=
t I'd like to discuss how we
(DPDK community) can best port-to and maintain a complex multi-threaded ser=
vice library with best-in-class
performance for the workload.

To put some discussions/solutions on the table:
- Shared Maintainership of a component?
     Split in functionality and C11 atomics implementation
     Obviously there would be collaboration required in such a case.
- Maybe shared maintainership is too much?
     A gentlemans/womans agreement of "helping out" with C11 atomics debug =
is enough?


Hope my concerns are understandable, and of course input/feedback welcomed!=
 -Harry


PS: Apologies for the delay in reply - was OOO on Irish national holiday.


> v3:
> add libatomic dependency for 32-bit clang
>=20
> v2:
> 1. fix Clang '-Wincompatible-pointer-types' WARNING.
> 2. fix typos.
>=20
> Honnappa Nagarahalli (2):
>   service: avoid race condition for MT unsafe service
>   service: identify service running on another core correctly
>=20
> Phil Yang (10):
>   doc: add generic atomic deprecation section
>   devtools: prevent use of rte atomic APIs in future patches
>   eal/build: add libatomic dependency for 32-bit clang
>   build: remove redundant code
>   vhost: optimize broadcast rarp sync with c11 atomic
>   ipsec: optimize with c11 atomic for sa outbound sqn update
>   service: remove rte prefix from static functions
>   service: remove redundant code
>   service: optimize with c11 one-way barrier
>   service: relax barriers with C11 atomic operations
>=20
>  devtools/checkpatches.sh                         |   9 ++
>  doc/guides/prog_guide/writing_efficient_code.rst |  60 +++++++-
>  drivers/event/octeontx/meson.build               |   5 -
>  drivers/event/octeontx2/meson.build              |   5 -
>  drivers/event/opdl/meson.build                   |   5 -
>  lib/librte_eal/common/rte_service.c              | 175 ++++++++++++-----=
-----
> -
>  lib/librte_eal/meson.build                       |   6 +
>  lib/librte_ipsec/ipsec_sqn.h                     |   3 +-
>  lib/librte_ipsec/sa.h                            |   2 +-
>  lib/librte_rcu/meson.build                       |   5 -
>  lib/librte_vhost/vhost.h                         |   2 +-
>  lib/librte_vhost/vhost_user.c                    |   7 +-
>  lib/librte_vhost/virtio_net.c                    |  16 ++-
>  13 files changed, 181 insertions(+), 119 deletions(-)
>=20
> --
> 2.7.4