From: Tyler Retzlaff <roretzla@linux.microsoft.com>
To: "Morten Brørup" <mb@smartsharesystems.com>
Cc: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>,
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 11:00:23 -0800 [thread overview]
Message-ID: <20230202190023.GA32597@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> (raw)
In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35D876EE@smartserver.smartshare.dk>
On Thu, Feb 02, 2023 at 09:43:58AM +0100, Morten Brørup wrote:
> > 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 think it's reasonable to assume that we are talking years here.
We've had a few discussions about minimum C standard. I think my first
mailing list exchanges about C99 was almost 2 years ago. Given that we
still aren't on C99 now (though i know Bruce has a series up) indicates
that progression to C11 isn't going to happen any time soon and even if
it was the baseline we still can't just use it (reasons described
later).
Also, i'll point out that we seem to have accepted moving to C99 with
one of the holdback compilers technically being non-conformant but it
isn't blocking us because it provides the subset of C99 features without
being conforming that we happen to be using.
>
> >
> > 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?
I think this is even more of a concern with language standard than it is
with a platform standard. Because the language standard is used across
platforms.
On the surface it looks appealing to just go through all the dpdk code
one last time and #include <stdatomic.h> and directly depend on names
that "look" standard. In practice though we aren't depending on the
toolchain / libc surface we are binding ourselves to the shim and the
implementation it provides.
This is aside from the mechanics of making it work in the different
contexts we now have to care about. Here is a story of how things
become tricky.
When i #include <stdatomic.h> which one gets used if the implementation
provides one? Let's force our stdatomic.h
Now i need to force the build system to prefer my shim header? Keeping
in mind that the presence of a libc stdatomic.h does not mean that the
toolchain in fact supports standard atomics. Okay, that's under our
control by editing some meson.build files maybe it isn't so bad but...
It seems my application also has to do the same in their build system
now because...
The type definitions (size, alignment) and code generated from the
body of inline functions as seen by the application built translation
units may differ from those in the dpdk translation units if they don't
use our header. The potential for ABI compat problems is increasing but
maybe it is managable? it can be worse...
We can't limit our scope to thinking that there is just an
application (a single binary) and dpdk. Complex applications will
invariably depend on other libraries and if the application needs to
interface with those compatibily at the ABI level using standard atomics
then we've made it very difficult since the application has to choose to
use our conflicting named atomic types which may not be compatible or
the real standard atomics. They can of course produce horrible shims
of their own to interoperate.
We need consistency across the entire binary at runtime and i don't
think it's practical to say that anyone who uses dpdk has to compile
their whole world with our shim. So dealing with all this complexity
for the sake of asthetics "looking" like the standard api seems kind
of not worth it. Sure it saves having to deprecate later and one last
session of shotgun surgery but that's kind of all we get.
Don't think i'm being biased in favor of windows/msvc here. From the
perspective of the windows/msvc combination i intend to use only the
standard C ABI provided by the implementation. I have no intention of
trying to introduce support for the current ABI that doesn't use the
standard atomic types. my discouraging of this approach is about avoiding
subtle to detect but very painful problems on {linux,unix}/compiler<version>
combinations that already have a shipped/stable ABI.
> >
> > 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.)
I am eager to see this happen, but as explained in my original proposal
it doesn't eliminate the need for an abstraction. Unless we are willing
to break our compatibility promises and potentially take a performance
hit on some platform/compiler combinations which as i understand is not
acceptable.
>
> 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.
I think the key to success with rte_ prefixed names is making absolutely
sure we mirror the semantics and types in the standard.
I will point out one bit of fine print here is that we will not support
atomic operations on struct/union types (something the standard supports).
With the rte_ namespace i think this becomes less ambiguous, if we present
standard C names though what's to avoid the confusion? Aside from it fails
to compile with one compiler vs another.
I agree that this may be around for years. But how many years depends a
lot on how long we have to maintain compatibility for the existing
platform/compiler combinations that can't (and aren't enabled) to use
the standard.
Even if we introduced standard names we still have to undergo some kind
of mutant deprecation process to get the world to recompile everything
against the actual standard, so it doesn't give us forward
compatibility.
Let me know what folks would like to do, i guess i'm firmly leaned
toward no-shim and just rte_ explicit. But as a community i'll pursue
whatever you decide.
Thanks!
next prev parent reply other threads:[~2023-02-02 19:00 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
2023-02-02 19:00 ` Tyler Retzlaff [this message]
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=20230202190023.GA32597@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net \
--to=roretzla@linux.microsoft.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=mb@smartsharesystems.com \
--cc=nd@arm.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).