From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 80E8FA057C; Fri, 27 Mar 2020 15:47:57 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 511601C20F; Fri, 27 Mar 2020 15:47:57 +0100 (CET) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 45F0E1C20F for ; Fri, 27 Mar 2020 15:47:55 +0100 (CET) IronPort-SDR: gZjwygjlnxP1vpszLGnmgxgYNa94M9Z4W7FDyZH9pSZIystJxtZ12s0AHsgGjdp8/PR7uEmRfW pQleLVXQ/7vA== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Mar 2020 07:47:54 -0700 IronPort-SDR: oDZdrJRb8GVaerQL+H8+oYmDdd0tVD6A6T/cOKpdSEfXA1DSZMo7WhgmV3ZTYikk5g+UEczJ7Q 6U+FaL2sZslA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,312,1580803200"; d="scan'208";a="241318482" Received: from orsmsx105.amr.corp.intel.com ([10.22.225.132]) by orsmga008.jf.intel.com with ESMTP; 27 Mar 2020 07:47:53 -0700 Received: from orsmsx151.amr.corp.intel.com (10.22.226.38) by ORSMSX105.amr.corp.intel.com (10.22.225.132) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 27 Mar 2020 07:47:53 -0700 Received: from ORSEDG001.ED.cps.intel.com (10.7.248.4) by ORSMSX151.amr.corp.intel.com (10.22.226.38) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 27 Mar 2020 07:47:53 -0700 Received: from NAM02-SN1-obe.outbound.protection.outlook.com (104.47.36.57) by edgegateway.intel.com (134.134.137.100) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 27 Mar 2020 07:47:53 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=IhRu3ULKqIYxtbWhMS3288NQqqCTSh9z4h6MotiHwpkHtazKoMSq33Fh84BfMgbL0rOFV8mHp31282szLA5Sp4fIab4dPNq1O79k7gNmSddhEl5WICsGF6kOzN99Xv6Cq3uNf9nL1Xj7ooCVMGhRf9LQ4uf9pioiYzGJbjD0bPWYA4Euf+P9mNJoFDePXwsh6BhnT2laS9G5+QCaXI8gFlaYU0CVl0Y74ZXkddahmS0xZozezkqWnn43Jx2YKpLzI77cisQQ0A7JYRmzt/2CB/zmtp8cPQ1zXkQ5fyZGQPLVyi1svEcoiKxaYSKZYE+hW0rtkv5rwmVXzuCb7fpIuA== 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=+5I30uAXZTETanwPgn2lB2IFaNZ4q04gbeto1z2Y/hk=; b=nveVarVukgyzOPS8Ejxx5DFZ8uXLUIKwi+JkweweiEDr2BC8+JVRC2ke915MZGgC1ftsUaOPrZqycltFnZoSArqwy86REGokTPgN68/0aBFPVwKEgvDrWpLc3N2o9sK5UD4TvsdMuLoh8bMf3vAtdnntYeIpXTM54tgQ5y81u2TyPy0sJmztyhjHT7j89JUhu+GvZEOGftejR7GqnPgoNVnI8waevlnN2HA+8ewlpA/QmEI/qf4AM1T5Vf8wz6+y7U60lWkBMz3S92P+lxvpESqxTRFb7r0waj/mX9MXno6UpE24Mzxjz242pPtS6lFqAMDYJoVIasD1PnZFUWm3nA== 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=+5I30uAXZTETanwPgn2lB2IFaNZ4q04gbeto1z2Y/hk=; b=KJ01HEcM62KlcM+5W6PDTwcIs0J+rkyeLOQ3jYRT5NfIajr1bpLnfuqop6rdCcqeqRN5BOZh/pbQ8CrYimDuApBd5JmMN64bTLgP6eMv5fuPoFK91J2b7WLS3hwcXghqCsaXP2f1yNaz/Oum+ol6mXNz2TTBy1ncpz8nxeLLSAc= Received: from BYAPR11MB3143.namprd11.prod.outlook.com (2603:10b6:a03:92::32) by BYAPR11MB3510.namprd11.prod.outlook.com (2603:10b6:a03:8b::27) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2856.19; Fri, 27 Mar 2020 14:47:51 +0000 Received: from BYAPR11MB3143.namprd11.prod.outlook.com ([fe80::2c49:cfd1:b7a5:a915]) by BYAPR11MB3143.namprd11.prod.outlook.com ([fe80::2c49:cfd1:b7a5:a915%3]) with mapi id 15.20.2856.019; Fri, 27 Mar 2020 14:47:51 +0000 From: "Van Haaren, Harry" To: Honnappa Nagarahalli , Phil Yang , "thomas@monjalon.net" , "Ananyev, Konstantin" , "stephen@networkplumber.org" , "maxime.coquelin@redhat.com" , "dev@dpdk.org" CC: "david.marchand@redhat.com" , "jerinj@marvell.com" , "hemant.agrawal@nxp.com" , Gavin Hu , Ruifeng Wang , Joyce Kong , "Carrillo, Erik G" , nd , nd Thread-Topic: [PATCH v3 00/12] generic rte atomic APIs deprecate proposal Thread-Index: AQHV+/oHYvInwZkrnEm8WRIHQJ4aXqhOXJwQgAKR1oCAAOVZgIAKvpgw Date: Fri, 27 Mar 2020 14:47:51 +0000 Message-ID: References: <1583999071-22872-1-git-send-email-phil.yang@arm.com> <1584407863-774-1-git-send-email-phil.yang@arm.com> In-Reply-To: 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.189] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 9780133c-ee40-4f0f-5ff6-08d7d25dd37a x-ms-traffictypediagnostic: BYAPR11MB3510: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-forefront-prvs: 0355F3A3AE x-forefront-antispam-report: SFV:NSPM; SFS:(10019020)(366004)(136003)(396003)(346002)(39860400002)(376002)(53546011)(66946007)(66556008)(7696005)(64756008)(66446008)(86362001)(478600001)(110136005)(316002)(54906003)(966005)(52536014)(66476007)(76116006)(5660300002)(2906002)(7416002)(33656002)(6506007)(71200400001)(186003)(8676002)(81156014)(26005)(81166006)(4326008)(9686003)(8936002)(55016002)(41533002)(15398625002); DIR:OUT; SFP:1102; SCL:1; SRVR:BYAPR11MB3510; H:BYAPR11MB3143.namprd11.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: NOWUxEQHZQVtZA7StK+gTcXv6+nUk1+G/VMQyH+KtEYHKRq4OPZ8EEU0iH//PA3yi0nWvQk4rGQbwNfOjebmWOCLOuKQJIuhHgdUZPBt4XtRTryqMYb2FCUAAyMF+UZVTTAcvwxEQqkCqR7cgTlp2302vplvQYT0KcnONE4eWH9awUafSPL593Gc42aZ/tSa1j5vB6XYXIcTcMdZQTKDjWwfBkcVGNz9mZbiM+sqioyxs1oYzN3MxbfSfIHjpJhW5rsWdL8uvEd6cYzLTMSJLSgGShF33JupXKT8AAjcy5KY01y52P+coLAwdsUSGjnTo2/0u95rVXY+SkEFQd69SHj5gILnxOTDOneppA6lQDH0jdNY8Lj59j7mfzmtXZo5Syc3d/f5WbJvtDoOpY2XsYKzRTLnxvl0ZQDXFHJ8qEUHWRWPki+FGQtVNpL6fNM3VqqrHD2mSCgTdixIuR1TK385YUh0tLFIm8qrtaZba8yEi1kn+scvXdRi7nha8S/oKzh7a8LKfMlh3nSeWnxrZS3rGNNmDtuMLeNDo+AG6m83vloFsPGeem/oetVQerQytBgrcZgE163S/iKlhbS9p9Ei4CGyzNOnc4P8A8bzsdc= x-ms-exchange-antispam-messagedata: E7aSRj7uinhiKQxMwfJSDlO90pazM7yOMqWLFdtfab5i6igdK0xfwr+BHBLs049RO4m7iAkLCTMggTh91drJudDWNhlu/vW+qijIisKTi98oQmRpDMwAfQJ5mX83YHPQQgZTqUnwZsCJZFmscoNX5w== Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: 9780133c-ee40-4f0f-5ff6-08d7d25dd37a X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Mar 2020 14:47:51.6749 (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: q7zajl3fnx6Wm6mvEFeJSJShdHKdGnOeD7pBd9V5bzOcWils3LnYvCZrVxECgdV5gcuWgPq2/9Po6lU3n8t0ts7Sozfu5xRL9/HEqJLy7nY= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR11MB3510 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > -----Original Message----- > From: Honnappa Nagarahalli > Sent: Friday, March 20, 2020 6:32 PM > To: Van Haaren, Harry ; Phil Yang > ; thomas@monjalon.net; Ananyev, Konstantin > ; stephen@networkplumber.org; > maxime.coquelin@redhat.com; dev@dpdk.org > Cc: david.marchand@redhat.com; jerinj@marvell.com; hemant.agrawal@nxp.com= ; > Gavin Hu ; Ruifeng Wang ; Joyce K= ong > ; Carrillo, Erik G ; nd > ; Honnappa Nagarahalli ; nd > > Subject: RE: [PATCH v3 00/12] generic rte atomic APIs deprecate proposal >=20 > + Erik as there are similar changes to timer library >=20 > >=20 > > > > > > Subject: [PATCH v3 00/12] generic rte atomic APIs deprecate proposa= l > > > > > > > > DPDK provides generic rte_atomic APIs to do several atomic operatio= ns. > > > > 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. > > > > > > > > But the existing rte_atomic API implementations cannot be changed a= s > > > > 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: > > > > > > > > [1] deprecate rte_atomic APIs so that future patches do not use > > > > rte_atomic 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 > > > checkpatch is a bit sudden. Perhaps noting that in a future release > > > (20.11?) DPDK will move to a > > > C11 based atomics model is a more gradual step to achieving the goal, > > > and at that point add a checkpatch warning for additions of rte_atomi= c*? > > We have been working on changing existing usages of rte_atomic APIs in > DPDK > > to use C11 atomics. Usually, the x.11 releases have significant amount = of > > changes (not sure how many would use rte_atomic APIs). I would prefer t= hat > > in 20.11 no additional code is added using rte_atomics APIs. However, I= am > > open to suggestions on the exact time frame. > > Once we decide on the release, I think it makes sense to add a 'warning= ' > in the > > checkpatch to indicate the deprecation timeline and add an 'error' afte= r > the > > release. The above sounds reasonable - mainly let's not block any code that exists or is being developed today using rte_atomic_* APIs from making a release. > > > More on [2] in context below. > > > > > > The above is my point-of-view, of course I'd like more people from th= e > > > 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 fo= r > > > aarch64. > > > > 3) changes to various libraries to make use of c11 atomic APIs. > > > > > > > > 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 > > > libraries. > > > > > > 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) rte_atomic implementation tod= ay. > > I agree that it C11 changes are complex, especially if one is starting = out > to > > understand what these APIs provide. From my experience, once few > > underlying concepts are understood, reviewing or making changes do not > take > > too much time. > > > > > I see other patchsets enabling C11 across other DPDK components, so > > > maybe we should also discuss C11 enabling in a wider context that jus= t > > service cores? > > Yes, agree. We are in the process of making changes to other areas as > well. > > > > > > > > I don't think it fair to expect all developers to be well versed in > > > C11 atomic semantics like understanding the complex interactions > > > between the various > > > C11 RELEASE, AQUIRE barriers requires. > > C11 has been around from sometime now. To my surprise, OVS already uses > > C11 APIs extensively. VPP has been accepting C11 related changes from p= ast > > couple of years. Having said that, I agree in general that not everyone= is > > well versed. Fair point - like so many things, once familiar with it, it becomes easy :) > > > As maintainer of Service Cores I'm hesitant to accept the large-scale > > > refactor > > Right now, the patches are split into multiple commits. If required I c= an > host a > > call to go over simple C11 API usages (sufficient to cover the usage in > service > > core) and the changes in this patch. If you find that particular areas > need more > > understanding I can work on providing additional information such as > memory > > order ladder diagrams. Please let me know what you think. Thanks for the offer - I will need to do my due diligence on reiview before taking up any of your or other C11 folks time. > When I started working with C11 APIs, I had referred to the following blo= gs. > https://preshing.com/20120913/acquire-and-release-semantics/ > https://preshing.com/20130702/the-happens-before-relation/ > https://preshing.com/20130823/the-synchronizes-with-relation/ >=20 > These will be helpful to understand the changes. Thanks, indeed good articles. I found the following slide deck particularly informative due to the fantastic diagrams (eg, slide 23): https://mariadb.org/wp-content/uploads/2017/11/2017-11-Memory-barriers.pdf That said, finding a nice diagram and understanding the implications of actually using it is different! I hope to properly review the service-cores patches next week. > > > of atomic-implementation, as it could lead to racey bugs that are > > > likely extremely difficult to track down. (The recent race-on-exit ha= s > > > proven the difficulty in reproducing, and that's with an atomics mode= l > > > I'm quite familiar with). > > > > > > Let me be very clear: I don't wish to block a C11 atomic > > > implementation, but I'd like to discuss how we (DPDK community) can > > > best port-to and maintain a complex multi-threaded service 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? > > I think shared maintainer ship could be too much as there are many > changes. > > But, I and other engineers from Arm (I would include Arm ecosystem as > well) > > can definitely help out on debug and reviews involving C11 APIs. (I see > > Thomas's reply on this topic). Thanks for the offer - as above, ball on my side, I'll go review.