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 B0A32A057D; Wed, 18 Mar 2020 16:13:14 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 18C8B2B9E; Wed, 18 Mar 2020 16:13:14 +0100 (CET) Received: from new1-smtp.messagingengine.com (new1-smtp.messagingengine.com [66.111.4.221]) by dpdk.org (Postfix) with ESMTP id 0D3D5292D for ; Wed, 18 Mar 2020 16:13:13 +0100 (CET) Received: from compute7.internal (compute7.nyi.internal [10.202.2.47]) by mailnew.nyi.internal (Postfix) with ESMTP id 5E81558012B; Wed, 18 Mar 2020 11:13:12 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Wed, 18 Mar 2020 11:13:12 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=5qFQRQo7rnjriY6TTrKqaSbA8kBgGunPMPaIwL1arwQ=; b=PWekVGx8IQgh nsKSDC/rm5oz2FYrltPeKvOZ/ZWWeUgjJt7yqiPyRVfJF7v0QEUqQ5kEmOke54jz 6jZN3VcgoOgFeAfHtnjgBQ09QtQPEdkad2LYS1o5ZU9xpNtMxiCSmpexBKxw2pNj oLyD2cFVvg58u3Cd3GKELXqWMLnOHB8= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm2; bh=5qFQRQo7rnjriY6TTrKqaSbA8kBgGunPMPaIwL1ar wQ=; b=VzOwtJlalgTwvCRqrosKjCsdsIQJEYDVRDBL+VWftAILZP+0uxAslKbxd XvUblK3QZ5j4Y5pX5moYtJiATSE4hCq28IV0sSjoTToRnuJnqxWF0HP89FFTfCmW 9SJq2eH3vO6kdcSzHvdaHOGIKAr41ygPYSTkNMQ7AIPfc2j39CFDFzvYVrzRYkOZ jWhUcLidLOzSAOCJ8YcQ6bOtQ7aFkTYVxqfwxbD/nqrj4n49wKFfZhO6Q9NS4iEr Sjq+848CD7iNP/qMLnUHKg/BWvKWXxtUmr4qYmbklIpvE8jOQY+su88y4X8kFrRB 8fl51ScZopcL4YH6sVo2LJLlWqpmw== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedugedrudefjedgjedvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecukf hppeejjedrudefgedrvddtfedrudekgeenucevlhhushhtvghrufhiiigvpedtnecurfgr rhgrmhepmhgrihhlfhhrohhmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvght X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id F2F7930618B7; Wed, 18 Mar 2020 11:13:07 -0400 (EDT) From: Thomas Monjalon To: Phil Yang , "Van Haaren, Harry" , "Honnappa.Nagarahalli@arm.com" Cc: "Ananyev, Konstantin" , "stephen@networkplumber.org" , "maxime.coquelin@redhat.com" , "dev@dpdk.org" , "david.marchand@redhat.com" , "jerinj@marvell.com" , "hemant.agrawal@nxp.com" , "gavin.hu@arm.com" , "ruifeng.wang@arm.com" , "joyce.kong@arm.com" , "nd@arm.com" Date: Wed, 18 Mar 2020 16:13:05 +0100 Message-ID: <2682224.FA0FI3ke8A@xps> In-Reply-To: References: <1583999071-22872-1-git-send-email-phil.yang@arm.com> <1584407863-774-1-git-send-email-phil.yang@arm.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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" 18/03/2020 15:01, Van Haaren, Harry: > Hi Phil & Honnappa, > > From: Phil Yang > > > > 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. > > > > 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: > > > > [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_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 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 today. > I see other patchsets enabling C11 across other DPDK components, so maybe we 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 atomic semantics like > understanding the complex interactions between the various C11 RELEASE, AQUIRE barriers requires. > > As maintainer of Service Cores I'm hesitant to accept the large-scale refactor 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, 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? > > > Hope my concerns are understandable, and of course input/feedback welcomed! -Harry Thanks for raising the issue Harry. I think we should have at least two official maintainers for C11 atomics in general. C11 conversion is a progressive effort being done, and should be merged step by step. If C11 maintainers fail to fix some issues on time, then we can hold the effort. Does it make sense?