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 60F4B42C52; Wed, 7 Jun 2023 18:36:52 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EA1B542C76; Wed, 7 Jun 2023 18:36:51 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id E475E42C4D for ; Wed, 7 Jun 2023 18:36:50 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1686155810; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=mhThmx5CmxqhJSW7PNvhmJodzyESgiitdAhuDvTdris=; b=X5G0P7hLxe6zRX56CNBKLhIdFn3zo1X84T/P9pNQ3tAUKPxUC5zdTew0ktDrRXyUItG5xm Tv+TexTxXqRFM4499T676ZJKg52N44vhyFMll5z+waKGC0fqZs9KFj/CHo+rOjpDmu0pY2 hyQupdv6g32Z9z6ZwkbY9vfwUeNRXcY= Received: from mail-pj1-f70.google.com (mail-pj1-f70.google.com [209.85.216.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-136-WOz_joVUMs20JtiR2V-uGw-1; Wed, 07 Jun 2023 12:36:48 -0400 X-MC-Unique: WOz_joVUMs20JtiR2V-uGw-1 Received: by mail-pj1-f70.google.com with SMTP id 98e67ed59e1d1-25669acf204so6879316a91.2 for ; Wed, 07 Jun 2023 09:36:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686155808; x=1688747808; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=mhThmx5CmxqhJSW7PNvhmJodzyESgiitdAhuDvTdris=; b=Ty7iWPv/V2qXQDSI4zWn0H00ZB9P7ZzR3EHm+Ta5DTcfmrQNGFjpjdfQNFDrC0/8Xm eV0D1bvCJ+Jdm7BG4xHI5kj4/ecnWcys58P7XwbUE8cPCNiGm+/RE1EKShh5gVE+8A0D BiUUDL9ZnBa5mguDyghrWnCnG2Dkv4IDReuVD3nPs83W6rpTtsF0BxRI9/uyRSu148Kq t3APq2B4CcZ13Lur2q2C20H8noIWjR+UhinTb8+354L/Yl+RVwAFrvsC93KaX1f8OD8Z WyoH/PZ9uKQ3rAOtxshpD4YqllHI8DJaO++WIXfAtbZM9MMO0m+iE9kdniFZeOwSVDAU qc6w== X-Gm-Message-State: AC+VfDxBHqpbD8pUw9Fkvr1VurQCqHQ3bGWVAVea6xQayfSbadURIczi 3K8OZ/Alhfs3DVoQYSy5M41gI+DCdginQJ3ZtXuMlmg5q20dOkzXvzozZgUIVVEZ/Whmcln8f+n H31+HX0e1p6Xqgxtxcog= X-Received: by 2002:a17:90a:fa91:b0:250:2384:120d with SMTP id cu17-20020a17090afa9100b002502384120dmr5357053pjb.19.1686155807884; Wed, 07 Jun 2023 09:36:47 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ7R5fNlIGn32EzH4i2awc1JTHWhcqYkEs5LA1PTlCkgkE9J3Ibamt+cq2/R6Beuv2bsd9Sp1dk7ukCZ6KhKQ8s= X-Received: by 2002:a17:90a:fa91:b0:250:2384:120d with SMTP id cu17-20020a17090afa9100b002502384120dmr5357030pjb.19.1686155807324; Wed, 07 Jun 2023 09:36:47 -0700 (PDT) MIME-Version: 1.0 References: <1679927420-26737-1-git-send-email-roretzla@linux.microsoft.com> <20230524160508.GA9733@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <20230602041814.GA27303@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> In-Reply-To: <20230602041814.GA27303@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> From: David Marchand Date: Wed, 7 Jun 2023 18:36:36 +0200 Message-ID: Subject: Re: [PATCH 0/3] use C11 memory model GCC builtin atomics To: Tyler Retzlaff Cc: thomas@monjalon.net, dev@dpdk.org, Honnappa.Nagarahalli@arm.com, Ruifeng.Wang@arm.com, Maxime Coquelin X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 Fri, Jun 2, 2023 at 6:18=E2=80=AFAM Tyler Retzlaff wrote: > > On Wed, May 24, 2023 at 09:05:08AM -0700, Tyler Retzlaff wrote: > > On Wed, May 24, 2023 at 02:51:50PM +0200, David Marchand wrote: > > > On Mon, Mar 27, 2023 at 4:30=E2=80=AFPM 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 straigh= tforward. > > > 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 generate= d > > 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. I don't think all the targets/tests are run in the CI (when compared to what is done during a -rcX validation). But I can understand this assumption. > > > > If this is a bad assumption or there are concerns, I could update the s= eries > > to do the conditional __sync vs __atomic throughout. Nobody else complained/reacted on the topic. The conditional would have to be duplicated in various places. So ok, let's go with this approach and see if the rc1 validation (which may run more tests) catches something. > > > > Let me know how you'd like to proceed. > > Anything further here or want to keep it as is? Nop, thanks. --=20 David Marchand