DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Morten Brørup" <mb@smartsharesystems.com>
To: "Tyler Retzlaff" <roretzla@linux.microsoft.com>,
	"Honnappa Nagarahalli" <Honnappa.Nagarahalli@arm.com>
Cc: <thomas@monjalon.net>, <dev@dpdk.org>,
	<bruce.richardson@intel.com>, <david.marchand@redhat.com>,
	<jerinj@marvell.com>, <konstantin.ananyev@huawei.com>,
	<ferruh.yigit@amd.com>, "nd" <nd@arm.com>
Subject: RE: [PATCH] eal: introduce atomics abstraction
Date: Thu, 2 Feb 2023 09:43:58 +0100	[thread overview]
Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D876EE@smartserver.smartshare.dk> (raw)
In-Reply-To: <20230201214111.GA30564@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>

> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> Sent: Wednesday, 1 February 2023 22.41
> 
> On Wed, Feb 01, 2023 at 01:07:59AM +0000, Honnappa Nagarahalli wrote:
> >
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > Sent: Tuesday, January 31, 2023 4:42 PM
> > >
> > > Honnappa, please could you give your view on the future of atomics
> in DPDK?
> > Thanks Thomas, apologies it has taken me a while to get to this
> discussion.
> >
> > IMO, we do not need DPDK's own abstractions. APIs from stdatomic.h
> (stdatomics as is called here) already serve the purpose. These APIs
> are well understood and documented.
> 
> i agree that whatever atomics APIs we advocate for should align with
> the
> standard C atomics for the reasons you state including implied
> semantics.
> 
> >
> > For environments where stdatomics are not supported, we could have a
> stdatomic.h in DPDK implementing the same APIs (we have to support only
> _explicit APIs). This allows the code to use stdatomics APIs and when
> we move to minimum supported standard C11, we just need to get rid of
> the file in DPDK repo.

Perhaps we can use something already existing, such as this:
https://android.googlesource.com/platform/bionic/+/lollipop-release/libc/include/stdatomic.h

> 
> my concern with this is that if we provide a stdatomic.h or introduce
> names
> from stdatomic.h it's a violation of the C standard.
> 
> references:
>  * ISO/IEC 9899:2011 sections 7.1.2, 7.1.3.
>  * GNU libc manual
>    https://www.gnu.org/software/libc/manual/html_node/Reserved-
> Names.html
> 
> in effect the header, the names and in some instances namespaces
> introduced
> are reserved by the implementation. there are several reasons in the
> GNU libc
> manual that explain the justification for these reservations and if
> if we think about ODR and ABI compatibility we can conceive of others.

I we are going to move to C11 soon, I consider the shim interim, and am inclined to ignore these warning factors.

If we are not moving to C11 soon, I would consider these disadvantages more seriously.

> 
> i'll also remark that the inter-mingling of names from the POSIX
> standard implicitly exposed as a part of the EAL public API has been
> problematic for portability.

This is a very important remark, which should be considered carefully! Tyler has firsthand experience with DPDK portability. If he thinks porting to Windows is going to be a headache if we expose the stdatomic.h API, we must listen! So, what is your gut feeling here, Tyler?

> 
> let's discuss this from here. if there's still overwhelming desire to
> go
> this route then we'll just do our best.
> 
> ty

I have a preference for exposing the stdatomic.h API. Tyler listed the disadvantages above. (I also have a preference for moving to C11 soon.)

Exposing a 1:1 similar API with RTE prefixes would also be acceptable for me. The disadvantage is that the names are different than the C11 names, which might lead to some confusion. And from an ABI stability perspective, such an important API should not be marked experimental. This means that years will pass before we can get rid of it again, due to ABI stability policies.

-Morten


  reply	other threads:[~2023-02-02  8:44 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-12 21:26 [PATCH] eal: abstract compiler atomics Tyler Retzlaff
2023-01-12 21:26 ` [PATCH] eal: introduce atomics abstraction Tyler Retzlaff
2023-01-31 22:42   ` Thomas Monjalon
2023-02-01  1:07     ` Honnappa Nagarahalli
2023-02-01  8:09       ` Morten Brørup
2023-02-01 21:41       ` Tyler Retzlaff
2023-02-02  8:43         ` Morten Brørup [this message]
2023-02-02 19:00           ` Tyler Retzlaff
2023-02-02 20:44             ` Morten Brørup
2023-02-03 13:56               ` Bruce Richardson
2023-02-03 14:25                 ` Morten Brørup
2023-02-03 12:19             ` Bruce Richardson
2023-02-03 20:49               ` Tyler Retzlaff
2023-02-07 15:16                 ` Morten Brørup
2023-02-07 21:58                   ` Tyler Retzlaff
2023-02-07 23:34         ` Honnappa Nagarahalli
2023-02-08  1:20           ` Tyler Retzlaff
2023-02-08  8:31             ` Morten Brørup
2023-02-08 16:35               ` Tyler Retzlaff
2023-02-09  0:16                 ` Honnappa Nagarahalli
2023-02-09  8:34                   ` Morten Brørup
2023-02-09 17:30                   ` Tyler Retzlaff
2023-02-10  5:30                     ` Honnappa Nagarahalli
2023-02-10 20:30                       ` Tyler Retzlaff
2023-02-13  5:04                         ` Honnappa Nagarahalli
2023-02-13 15:28                           ` Ben Magistro
2023-02-13 15:55                             ` Bruce Richardson
2023-02-13 16:46                               ` Ben Magistro
2023-02-13 17:49                                 ` Morten Brørup
2023-02-13 23:18                           ` Tyler Retzlaff
2023-01-31 21:33 ` [PATCH] eal: abstract compiler atomics Tyler Retzlaff
2023-02-08 21:43 ` [PATCH v2] " Tyler Retzlaff
2023-02-08 21:43   ` [PATCH v2] eal: introduce atomics abstraction Tyler Retzlaff
2023-02-09  8:05     ` Morten Brørup
2023-02-09 18:15       ` Tyler Retzlaff
2023-02-09 19:19         ` Morten Brørup
2023-02-09 22:04           ` Tyler Retzlaff
2023-04-03 21:17       ` Mattias Rönnblom
2023-02-09  9:04     ` Bruce Richardson
2023-02-09 12:53       ` Ferruh Yigit
2023-02-09 17:40         ` Tyler Retzlaff
2023-02-09 22:13           ` Ferruh Yigit
2023-02-10  0:36             ` Tyler Retzlaff
2023-02-09 17:38       ` Tyler Retzlaff
2023-04-03 21:32         ` Mattias Rönnblom
2023-04-03 21:11     ` Mattias Rönnblom
2023-04-03 21:25       ` Honnappa Nagarahalli
2023-04-04  2:24       ` Tyler Retzlaff
2023-02-22 18:09   ` [PATCH v2] eal: abstract compiler atomics Tyler Retzlaff
2023-02-22 20:07     ` Honnappa Nagarahalli
2023-02-23 19:11       ` Tyler Retzlaff

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=98CBD80474FA8B44BF855DF32C47DC35D876EE@smartserver.smartshare.dk \
    --to=mb@smartsharesystems.com \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=bruce.richardson@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@amd.com \
    --cc=jerinj@marvell.com \
    --cc=konstantin.ananyev@huawei.com \
    --cc=nd@arm.com \
    --cc=roretzla@linux.microsoft.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).