From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
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 <dev@dpdk.org>; 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 <dev@dpdk.org>; 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 <david.marchand@redhat.com>
Date: Fri, 29 Sep 2023 10:04:18 +0200
Message-ID: <CAJFAV8zh6duDWABvPW3f0q93xwpSNin5bBo9msvYHkpA0UWz=g@mail.gmail.com>
Subject: Re: [PATCH v6 1/6] eal: provide rte stdatomics optional atomics API
To: Thomas Monjalon <thomas@monjalon.net>,
Tyler Retzlaff <roretzla@linux.microsoft.com>
Cc: dev@dpdk.org, techboard@dpdk.org,
Bruce Richardson <bruce.richardson@intel.com>,
Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>,
Ruifeng Wang <ruifeng.wang@arm.com>,
Jerin Jacob <jerinj@marvell.com>, Sunil Kumar Kori <skori@marvell.com>,
=?UTF-8?Q?Mattias_R=C3=B6nnblom?= <mattias.ronnblom@ericsson.com>,
Joyce Kong <joyce.kong@arm.com>, David Christensen <drc@linux.vnet.ibm.com>,
Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>,
David Hunt <david.hunt@intel.com>
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
On Thu, Sep 28, 2023 at 10:06=E2=80=AFAM Thomas Monjalon <thomas@monjalon.n=
et> 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 <rte_common.h>
> > #include <rte_lock_annotations.h>
> > #include <rte_pause.h>
> > +#include <rte_stdatomic.h>
>
> 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 <assert.h>
> > +
> > +#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 <stdatomic.h>
> > +
> > +/* 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