DPDK patches and discussions
 help / color / mirror / Atom feed
From: Neil Horman <nhorman@tuxdriver.com>
To: David Marchand <david.marchand@redhat.com>
Cc: Aaron Conole <aconole@redhat.com>,
	Adrien Mazarguil <adrien.mazarguil@6wind.com>,
	Gage Eads <gage.eads@intel.com>, dev <dev@dpdk.org>,
	Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
	Van Haaren Harry <harry.van.haaren@intel.com>,
	Nikhil Rao <nikhil.rao@intel.com>,
	Erik Gabriel Carrillo <erik.g.carrillo@intel.com>,
	Bruce Richardson <bruce.richardson@intel.com>,
	Pablo de Lara <pablo.de.lara.guarch@intel.com>
Subject: Re: [dpdk-dev] [PATCH] eal: promote some service core functions to stable
Date: Fri, 21 Jun 2019 13:40:23 -0400	[thread overview]
Message-ID: <20190621174023.GC21895@hmswarspite.think-freely.org> (raw)
In-Reply-To: <CAJFAV8xMtfxLp6=ULU-dH8qFHbmwZzRmS06EDcMgZ9-J631kFg@mail.gmail.com>

On Fri, Jun 21, 2019 at 06:47:31PM +0200, David Marchand wrote:
> On Fri, Jun 21, 2019 at 6:28 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> > On Fri, Jun 21, 2019 at 02:45:45PM +0200, David Marchand wrote:
> > > Ok, did a new pass on the tree.. found quite some sites where we have
> > > issues (and other discrepancies... I started a new patchset).
> > > Looked at gcc documentation [1], and to me the safer approach would be to
> > > enforce that __rte_experimental is the first thing of a symbol
> > declaration.
> > >
> > > Comments?
> > >
> > Yes, thats the only way it works, in fact I'm suprised gcc didn't throw an
> > error
> > about expecting an asm statement if you put it anywhere else
> >
> 
> - I tried this, but then I hit issues with inlines.
> Like for example:
> 
> static inline char * __rte_experimental
> rte_mbuf_buf_addr(struct rte_mbuf *mb, struct rte_mempool *mp)
> {
>   return (char *)mb + sizeof(*mb) + rte_pktmbuf_priv_size(mp);
> }
> 
> I did not find a way to move the __rte_experimental tag without getting
> warnings.
Right, thats the way its supposed to work on gcc/icc/clang.  function attributes
must be declared between the return type and the function name, anything else
will generate compiler warnings/errors.  Because __rte_experimental expands to a
__attribute__(...), you have to place it there.

> If I try to compile some sources which includes rte_mbuf.h but without
> -DALLOW_EXPERIMENTAL_API, then gcc errors at including the header,
> complaining that rte_mbuf_buf_addr() is deprecated, even if this inline is
> not called.
> 
Thats...odd.  I wonder if thats an artifact of the function being marked as
inline.  The compiler is supposed to insert the warning for any remaining calls
after dead code eliminitaion.  If the function is inline, I wonder if the
compiler conservatively inserts the warning because it got expanded into another
function, when it can't tell if it will be entirely elimintated.  Can you
provide a code sample that demonstrates this?

> For now, I hid all those inlines under the ALLOW_EXPERIMENTAL_API flag.
> 
> 
> - I have accumulated other fixes and I dropped all the marking in the .c
> files.
> This ended up with a huge series...
> 
>  118 files changed, 867 insertions(+), 646 deletions(-)
> 
> https://github.com/david-marchand/dpdk/commits/experimental
> 
> I will let the week end pass on this.
> 
> 
> -- 
> David Marchand

  reply	other threads:[~2019-06-21 17:41 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-20 16:42 Gage Eads
2019-06-20 18:25 ` Aaron Conole
2019-06-20 18:39   ` Eads, Gage
2019-06-20 19:45   ` David Marchand
2019-06-20 20:16     ` David Marchand
2019-06-21 12:45       ` David Marchand
2019-06-21 16:27         ` Neil Horman
2019-06-21 16:47           ` David Marchand
2019-06-21 17:40             ` Neil Horman [this message]
2019-06-21 19:58               ` David Marchand
2019-06-22 16:17                 ` Neil Horman
2019-06-22 17:51                   ` David Marchand
2019-06-22 19:33                     ` Neil Horman
2019-06-20 19:02 ` [dpdk-dev] [PATCH v2] " Gage Eads
2019-06-27 12:48   ` David Marchand
2019-06-27 16:25     ` Eads, Gage
2019-07-08 10:23   ` Thomas Monjalon

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=20190621174023.GC21895@hmswarspite.think-freely.org \
    --to=nhorman@tuxdriver.com \
    --cc=aconole@redhat.com \
    --cc=adrien.mazarguil@6wind.com \
    --cc=bruce.richardson@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=erik.g.carrillo@intel.com \
    --cc=gage.eads@intel.com \
    --cc=harry.van.haaren@intel.com \
    --cc=jerinj@marvell.com \
    --cc=nikhil.rao@intel.com \
    --cc=pablo.de.lara.guarch@intel.com \
    /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).