DPDK patches and discussions
 help / color / mirror / Atom feed
From: Tyler Retzlaff <roretzla@linux.microsoft.com>
To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
Cc: Joyce Kong <Joyce.Kong@arm.com>,
	"thomas@monjalon.net" <thomas@monjalon.net>,
	"david.marchand@redhat.com" <david.marchand@redhat.com>,
	Ruifeng Wang <Ruifeng.Wang@arm.com>,
	"dev@dpdk.org" <dev@dpdk.org>, nd <nd@arm.com>
Subject: Re: [dpdk-dev] [PATCH v1] test/ticketlock: use C11 atomic builtins for lcores sync
Date: Fri, 30 Apr 2021 08:51:47 -0700	[thread overview]
Message-ID: <20210430155147.GA24271@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> (raw)
In-Reply-To: <DBAPR08MB58148900FDB366920E068003985F9@DBAPR08MB5814.eurprd08.prod.outlook.com>

On Thu, Apr 29, 2021 at 09:10:04PM +0000, Honnappa Nagarahalli wrote:
> <snip>
> 
> > 
> > your subject line indicates the use of C11 which is a standard [1].
> > 
> > the patch itself uses gcc atomics builtins which are not part of C11 standard so
> > the subject line is incorrect and misleading.
> Ok, understood. How about the following?
> "use gcc's C11 atomic built-ins for lcore synchronization"

drop 'C11' from it and it describes the actual change

> 
> > 
> > [1] http://www.open-std.org/jtc1/sc22/wg14/www/standards.html#9899
> > 
> > >
> > > Not sure if these compilers are supported in DPDK. DPDK officially supports
> > gcc, clang (not sure on icc).
> > 
> > dpdk may incorporate support for other compilers in the future so unless there is
> > substantive justification for moving to non-standard/non-portable code i'm
> > asking that this change not be made as it will complicate those future efforts.
> There is some history [1] behind why we are doing this. I guess new compiler support needs to be discussed in the future.
> 
> [1] https://www.dpdk.org/blog/2021/03/26/dpdk-adopts-the-c11-memory-model/

thanks for the reference. it seems this documents explicitly states the
choice to not use C11 stdatomic.h and the basis of that choice appears
to be to support old versions of gcc.

it doesn't seem particularly forward looking to reduce future compiler
portability to support old versions of gcc thereby excluding standards
compliant compilers.

i would like to hear from the tech board that it is the best
trade-off for the project to reduce compiler portability for older
versions of gcc instead of adopting standard C11 atomics which locks out
the use of other compilers.

if this change does go forward could i at least ask that the builtins
used are abstracted behind either macros or inline functions so that if
alternate implementations appear for the builtins we don't have to
perform shotgun surgery on the broader codebase when it arrives?

thanks!

  parent reply	other threads:[~2021-04-30 15:51 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-21  7:17 Joyce Kong
2021-04-29 19:03 ` Tyler Retzlaff
2021-04-29 19:17   ` Honnappa Nagarahalli
2021-04-29 19:38     ` Tyler Retzlaff
2021-04-29 21:10       ` Honnappa Nagarahalli
2021-04-30  0:53         ` Stephen Hemminger
2021-04-30 15:51         ` Tyler Retzlaff [this message]
2021-05-05  0:37           ` Honnappa Nagarahalli
2021-05-05 17:10             ` 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=20210430155147.GA24271@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net \
    --to=roretzla@linux.microsoft.com \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=Joyce.Kong@arm.com \
    --cc=Ruifeng.Wang@arm.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --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).