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 0E08D4266F; Fri, 29 Sep 2023 10:04:36 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9766040277; Fri, 29 Sep 2023 10:04:35 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id C36CF4003C for ; Fri, 29 Sep 2023 10:04:33 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1695974673; 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=T8Ah0qzccMCS7rlpoGLKhepToMhtCCzgxqWjpn48MBs=; b=g94gRwF4yTnwOTNcbJGTVxO4ejffEnyxX84jil84eupnw3II8XyL+Ign7M8kQYKu99s955 8yKZvvAeHYuGqYfKJPKn2LvxU0EecX1rBf4R9DaJgJM56/nzLGv5FBySLLTF3Alju1LTDQ R1mmghvOpdasPSRfMlzQPdeGB0h5rFA= Received: from mail-lj1-f197.google.com (mail-lj1-f197.google.com [209.85.208.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-483-iOPSCokxN5S5mKq9UI_90w-1; Fri, 29 Sep 2023 04:04:31 -0400 X-MC-Unique: iOPSCokxN5S5mKq9UI_90w-1 Received: by mail-lj1-f197.google.com with SMTP id 38308e7fff4ca-2c12bb1f751so208288611fa.2 for ; Fri, 29 Sep 2023 01:04:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695974670; x=1696579470; 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=T8Ah0qzccMCS7rlpoGLKhepToMhtCCzgxqWjpn48MBs=; b=LzGdV5vRGrQgWgbPk7lkSbxWo8yYWUwKEpWmGR+iv26OrrH8GNZqwF4evmspG7DLLR h30n7KlO+Fayji5+q6fUloZPVHXcQNg2rYUpZSFJ4a0nMizlUQgg7gFE8jBTGBK90/Ac xTKs9f39qkziuW7LGiS7n8dSnmQjm7oieUuB2q7btSt4RXXmnet+nyLUUDEnYmndCTZ2 8YLhYSGWhHiCOaeW2AIXO+04t2Q0tZJROFTsToyTdTE3pTDRlSjcbAgSf1a++hyR90pY kyiKawMlNQLB1M81FgxOHB9hw+aQ0ULby4KTzVS2uLIHTSRQ6WFy26gqFaQIqN3fqC2O AzdQ== X-Gm-Message-State: AOJu0YwzrtuiU8AEiNoOE5I4nGB6VuUeH9kus/IZ5gfyzwqrA0zrUIMZ 77qnxjG95CyqoWb6fGd+GZa0VYBpqJjf4H0G0RYMiYu0KSMO0kaupE/K/LPRcPQUsoI8nC7YGox YssfI6u+fcacw0yii7D8= X-Received: by 2002:a2e:9dc9:0:b0:2bc:b54b:c03f with SMTP id x9-20020a2e9dc9000000b002bcb54bc03fmr2871861ljj.5.1695974670009; Fri, 29 Sep 2023 01:04:30 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFBvjjlUacGyZwiMH43mK7usL/ttgLuZ+62sQT/DeVx2Hlv2b3PL2+Sjti5iXPknwEWSJVeRtPscIiE9aFbV3c= X-Received: by 2002:a2e:9dc9:0:b0:2bc:b54b:c03f with SMTP id x9-20020a2e9dc9000000b002bcb54bc03fmr2871844ljj.5.1695974669636; Fri, 29 Sep 2023 01:04:29 -0700 (PDT) MIME-Version: 1.0 References: <1691717521-1025-1-git-send-email-roretzla@linux.microsoft.com> <1692738045-32363-1-git-send-email-roretzla@linux.microsoft.com> <1692738045-32363-2-git-send-email-roretzla@linux.microsoft.com> <5908573.LM0AJKV5NW@thomas> In-Reply-To: <5908573.LM0AJKV5NW@thomas> From: David Marchand Date: Fri, 29 Sep 2023 10:04:18 +0200 Message-ID: Subject: Re: [PATCH v6 1/6] eal: provide rte stdatomics optional atomics API To: Thomas Monjalon , Tyler Retzlaff Cc: dev@dpdk.org, techboard@dpdk.org, Bruce Richardson , Honnappa Nagarahalli , Ruifeng Wang , Jerin Jacob , Sunil Kumar Kori , =?UTF-8?Q?Mattias_R=C3=B6nnblom?= , Joyce Kong , David Christensen , Konstantin Ananyev , David Hunt 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 Thu, Sep 28, 2023 at 10:06=E2=80=AFAM Thomas Monjalon wrote: > > 22/08/2023 23:00, Tyler Retzlaff: > > --- a/lib/eal/include/generic/rte_rwlock.h > > +++ b/lib/eal/include/generic/rte_rwlock.h > > @@ -32,6 +32,7 @@ > > #include > > #include > > #include > > +#include > > I'm not sure about adding the include in patch 1 if it is not used here. Yes, this is something I had already fixed locally. > > > --- /dev/null > > +++ b/lib/eal/include/rte_stdatomic.h > > @@ -0,0 +1,198 @@ > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright(c) 2023 Microsoft Corporation > > + */ > > + > > +#ifndef _RTE_STDATOMIC_H_ > > +#define _RTE_STDATOMIC_H_ > > + > > +#include > > + > > +#ifdef __cplusplus > > +extern "C" { > > +#endif > > + > > +#ifdef RTE_ENABLE_STDATOMIC > > +#ifdef __STDC_NO_ATOMICS__ > > +#error enable_stdatomics=3Dtrue but atomics not supported by toolchain > > +#endif > > + > > +#include > > + > > +/* RTE_ATOMIC(type) is provided for use as a type specifier > > + * permitting designation of an rte atomic type. > > + */ > > +#define RTE_ATOMIC(type) _Atomic(type) > > + > > +/* __rte_atomic is provided for type qualification permitting > > + * designation of an rte atomic qualified type-name. > > Sorry I don't understand this comment. The difference between atomic qualifier and atomic specifier and the need for exposing those two notions are not obvious to me. One clue I have is with one use later in the series: +rte_mcslock_lock(RTE_ATOMIC(rte_mcslock_t *) *msl, rte_mcslock_t *me) ... + prev =3D rte_atomic_exchange_explicit(msl, me, rte_memory_order_acq_re= l); So at least RTE_ATOMIC() seems necessary. > > > + */ > > +#define __rte_atomic _Atomic > > + > > +/* The memory order is an enumerated type in C11. */ > > +typedef memory_order rte_memory_order; > > + > > +#define rte_memory_order_relaxed memory_order_relaxed > > +#ifdef __ATOMIC_RELAXED > > +static_assert(rte_memory_order_relaxed =3D=3D __ATOMIC_RELAXED, > > + "rte_memory_order_relaxed =3D=3D __ATOMIC_RELAXED"); > > Not sure about using static_assert or RTE_BUILD_BUG_ON Do you mean you want no check at all in a public facing header? Or is it that we have RTE_BUILD_BUG_ON and we should keep using it instead of static_assert? I remember some problems with RTE_BUILD_BUG_ON where the compiler would silently drop the whole expression and reported no problem as it could not evaluate the expression. At least, with static_assert (iirc, it is new to C11) the compiler complains with a clear "error: expression in static assertion is not constant". We could fix RTE_BUILD_BUG_ON, but I guess the fix would be equivalent to map it to static_assert(!condition). Using language standard constructs seems a better choice. --=20 David Marchand