DPDK patches and discussions
 help / color / mirror / Atom feed
From: Bruce Richardson <bruce.richardson@intel.com>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: "Burakov, Anatoly" <anatoly.burakov@intel.com>,
	David Marchand <david.marchand@redhat.com>,
	dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] eal: move compat includes to common meson file
Date: Thu, 10 Jan 2019 10:02:21 +0000	[thread overview]
Message-ID: <20190110100220.GA13196@bricha3-MOBL.ger.corp.intel.com> (raw)
In-Reply-To: <2632071.H5f8aPozaM@xps>

On Thu, Jan 10, 2019 at 09:52:43AM +0100, Thomas Monjalon wrote:
> 10/01/2019 09:45, Burakov, Anatoly:
> > On 09-Jan-19 9:36 PM, Thomas Monjalon wrote:
> > > 09/01/2019 21:51, Bruce Richardson:
> > >> On Wed, Jan 09, 2019 at 06:29:32PM +0100, David Marchand wrote:
> > >>>     On Wed, Jan 9, 2019 at 6:18 PM Anatoly Burakov
> > >>>     <[1]anatoly.burakov@intel.com> wrote:
> > >>>
> > >>>       Currently, while EAL does depend on librte_compat as far as common
> > >>>       meson build is concerned, for some reason the headers for that
> > >>>       library are not added into the list of includes. This is fixed in
> > >>>       Linuxapp-specific meson file, but is absent from FreeBSD meson file.
> > >>>       This worked fine up until recently, when an rte_compat dependency
> > >>>       was added to rte_log, which is a common header. Fix this issue by
> > >>>       adding librte_compat includes to common EAL meson file.
> > >>>       Fixes: 844514c73569 ("eal: build with meson")
> > >>>       Fixes: a8499f65a1d1 ("log: add missing experimental tag")
> > >>>       Cc: [2]bruce.richardson@intel.com
> > >>>       Cc: [3]david.marchand@redhat.com
> > >>>       Cc: [4]stable@dpdk.org
> > >>>       Signed-off-by: Anatoly Burakov <[5]anatoly.burakov@intel.com>
> > >>>
> > >>>     lgtm, thanks.
> > >>>     Reviewed-by: David Marchand <[6]david.marchand@redhat.com>
> > >>>     --
> > >>>     David Marchand
> > >>>
> > >> Agreed.
> > >> One other point I'd make is that having compat as a separate library seems
> > >> a little like overkill to me - it's just one header file! Is there a reason
> > >> why we don't just move it into the EAL where everyone can use it without
> > >> having to specify another dependency?
> > > 
> > > +1 for moving compat into EAL.
> > > 
> > 
> > Presumably the reason to keep it separate would be to enable depending 
> > on it for libraries that EAL itself depends on (i.e. kvargs).
> 
> Yes, forgot this one.
> 

I also forgot that. However, that does bring up the thought that for some
of our headers we may want them available for all libraries even before EAL
is compiled up. [I think this hits on the idea Thomas was discussing of
splitting EAL up into two, where we have a core part that is always
compiled first and has no dependencies followed by another which is built
later and can have dependencies.]

For this specific issue, my thought is that we should indeed move compat.h
into EAL, but also add the EAL common/include/ path to the global include
path for the project, so all EAL headers (e.g. including those with defines
for cacheline size, and other common macros) are just always available.
Libs like kvargs and others should be able to use RTE_DIM etc. for
consistency with the rest of DPDK.

/Bruce

  reply	other threads:[~2019-01-10 10:02 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-09 17:18 Anatoly Burakov
2019-01-09 17:29 ` David Marchand
2019-01-09 20:51   ` Bruce Richardson
2019-01-09 21:36     ` Thomas Monjalon
2019-01-10  8:45       ` Burakov, Anatoly
2019-01-10  8:52         ` Thomas Monjalon
2019-01-10 10:02           ` Bruce Richardson [this message]
2019-01-10 10:33             ` Thomas Monjalon
2019-01-10 10:34             ` Burakov, Anatoly
2019-01-10 10:41               ` Bruce Richardson
2019-01-10 10:44                 ` David Marchand
2019-01-10 10:48                   ` Bruce Richardson
2019-01-10 11:13                     ` Bruce Richardson

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=20190110100220.GA13196@bricha3-MOBL.ger.corp.intel.com \
    --to=bruce.richardson@intel.com \
    --cc=anatoly.burakov@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --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).