From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 0DFAB42B8F; Wed, 24 May 2023 18:06:01 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2CAF742D44; Wed, 24 May 2023 18:05:10 +0200 (CEST) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 19B9741101 for ; Wed, 24 May 2023 18:05:09 +0200 (CEST) Received: by linux.microsoft.com (Postfix, from userid 1086) id 628C620FB986; Wed, 24 May 2023 09:05:08 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 628C620FB986 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1684944308; bh=K0nt1L6Yc4eYbQ9hcPVYGf5Kq9iZIvndrduoyjtvqEQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Xz8rv0l5Dh8kX/5lKBcRRl5IvyjmF5bfZouNKnl50U5DcYDnwuufN3z5P+A8gCJYS vyS7ry3wr9aF6gmxX+Hb1C3Rz/FtMqnhKcTZmAZRCL8gjVoCForsbQAqmTlR8dJ/Ir Qpcb6q3fFhOuxSHaoUv4uH6RD/Pvej+i4UDzJ5uw= Date: Wed, 24 May 2023 09:05:08 -0700 From: Tyler Retzlaff To: David Marchand Cc: dev@dpdk.org, Honnappa.Nagarahalli@arm.com, Ruifeng.Wang@arm.com, thomas@monjalon.net, Maxime Coquelin Subject: Re: [PATCH 0/3] use C11 memory model GCC builtin atomics Message-ID: <20230524160508.GA9733@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <1679927420-26737-1-git-send-email-roretzla@linux.microsoft.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Wed, May 24, 2023 at 02:51:50PM +0200, David Marchand wrote: > On Mon, Mar 27, 2023 at 4:30 PM Tyler Retzlaff > wrote: > > > > Replace the use of __sync__and_fetch and __sync_fetch_and_ atomics > > with GCC C11 memory model __atomic builtins. > > > > This series contributes to converging on standard atomics in 23.11 but is > > kept separate as there may be sensitivity to converting from __sync to the > > C11 memory model builtins. > > - Looking at the patches, I thought the conversion was rather straightforward. > But this mention about "sensitivity" stopped me from merging. > Did I miss some risk with the changes of this series? > > > > > > Tyler Retzlaff (3): > > bus/vmbus: use C11 memory model GCC builtin atomics > > crypto/ccp: use C11 memory model GCC builtin atomics > > eal: use C11 memory model GCC builtin atomics > > > > drivers/bus/vmbus/vmbus_channel.c | 2 +- > > drivers/crypto/ccp/ccp_dev.c | 6 ++++-- > > lib/eal/include/generic/rte_atomic.h | 32 ++++++++++++++++---------------- > > 3 files changed, 21 insertions(+), 19 deletions(-) > > > - I noticed that the vhost library has been providing an internal > wrapper for some __sync atomic with older GCC. > Some details are in the commitlog c16915b87109 ("vhost: improve dirty > pages logging performance"). > > Could it affect the existing legacy API performance? Yes. gcc documents that you can replace __sync_ with __atomic_ using SEQ_CST ordering. When the __atomic_ builtins were initially introduced they generated sub-optimal (you can interpret as slower) codegen relative to the existing __sync_ builtins which was fixed in later gcc releases. I do not know the actual version of gcc, but the commit you reference indicates GCC_VERSION < 70100 is that boundary. I (perhaps incorrectly) assumed that if the CI performance tests didn't indicate a regression that the replacement of the remaining and minimal use of the legacy API would have negligable impact. If this is a bad assumption or there are concerns, I could update the series to do the conditional __sync vs __atomic throughout. Let me know how you'd like to proceed. Thanks! > > -- > David Marchand