From: "Morten Brørup" <mb@smartsharesystems.com>
To: "Tyler Retzlaff" <roretzla@linux.microsoft.com>
Cc: <dev@dpdk.org>, <Honnappa.Nagarahalli@arm.com>,
<Ruifeng.Wang@arm.com>, <thomas@monjalon.net>
Subject: RE: rte_atomic API compatibility & standard atomics
Date: Wed, 29 Mar 2023 10:43:27 +0200 [thread overview]
Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D8781A@smartserver.smartshare.dk> (raw)
In-Reply-To: <20230328184607.GA19745@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> Sent: Tuesday, 28 March 2023 20.46
>
> On Mon, Mar 27, 2023 at 10:08:10PM +0200, Morten Brørup wrote:
> > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > > Sent: Monday, 27 March 2023 21.39
> > >
> > > Hi folks,
> > >
> > > I don't think we discussed it specifically but what is the expectation
> > > in relation to converting to standard atomics and compatibility of the
> > > legacy rte_atomic APIs?
> > >
> > > We can't really convert the inline function implementations of the
> > > rte_atomic APIs because doing so would break compatibility. This is
> > > because if the implementation uses standard atomics APIs then we are
> > > required to pass _Atomic types to the generic atomic intrinsics.
> > >
> > > We can choose to just leave the rte_atomic API implementations as they
> > > are using the GCC builtins and i'm fine with that, but I do need some
> > > help with what to do with msvc then since it doesn't have those
> > > builtins.
> > >
> > > The options seem to be as follows.
> > >
> > > 1.
> > > Just cast the non-atomic types in the rte_atomic APIs implementation
> > > to _Atomic which may work but i'm pretty sure is undefined behavior
> > > since
> > > you can't qualify a non _Atomic type to suddenly be _Atomic.
This could also be an option, wrapped in #ifdef MSVC, so they are still unchanged for other build environments.
That limits your concern about undefined behavior to specifically how MSVC behaves.
> > >
> > > 2.
> > > We could conditionally compile (hide) the legacy rte_atomic APIs when
> > > msvc is in use, this seems not bad since there technically aren't any
> > > Windows/MSVC consumers, but if someone wanted to port an existing
> > > application they would have to adapt the code to avoid use of
> > > rte_atomic.
> > >
> > > For now I think the safest option is to go with 2 since it doesn't
> > > impose any compatibility risk and conditional compilation only exists
> > > until we deprecate and remove the old rte_atomic APIs.
> > >
> > > Are there any other options i'm missing here?
> > >
> > > Thanks
> >
> > As a variant of your second option, you could make most of the legacy
> rte_atomic APIs available to MSVC by changing the atomic counter types from
> volatile to _Atomic. Then only the atomic cmpset() and exchange() functions
> are unavailable for the application. E.g. for the 32 bit atomic counter type:
> >
> > typedef struct {
> > - volatile int32_t cnt; /**< An internal counter value. */
> > + _Atomic int32_t cnt; /**< An internal counter value. */
> > } rte_atomic32_t;
> >
>
> it's a good suggestion. but i'm not sure i want to get bogged down
> making an old api available that hopefully we will remove soon.
>
> though i'm still torn because i would really like the path to use msvc
> for any application to be lower burden.
>
> unless there are objections i think i'll do 2 as is. if good progress is
> made we can re-evaluate doing the extra work to make available the old apis
> as you suggest or potentially leave them unavailable forever subject to
> any plans to deprecate and remove them.
No objections from me, either way.
From a high level perspective, I consider it perfectly reasonable to get up and running with very limited support. When MSVC gets more traction, and MSVC users want more of DPDK, I expect to see questions on the mailing list, or directly to you or the MSVC team. Then you can focus catching up on the features in demand.
prev parent reply other threads:[~2023-03-29 8:43 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-27 19:39 Tyler Retzlaff
2023-03-27 20:08 ` Morten Brørup
2023-03-28 18:46 ` Tyler Retzlaff
2023-03-29 8:43 ` Morten Brørup [this message]
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=98CBD80474FA8B44BF855DF32C47DC35D8781A@smartserver.smartshare.dk \
--to=mb@smartsharesystems.com \
--cc=Honnappa.Nagarahalli@arm.com \
--cc=Ruifeng.Wang@arm.com \
--cc=dev@dpdk.org \
--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).