DPDK patches and discussions
 help / color / mirror / Atom feed
From: Neil Horman <nhorman@tuxdriver.com>
To: "Richardson, Bruce" <bruce.richardson@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used RHEL 6)
Date: Thu, 18 Sep 2014 14:12:24 -0400
Message-ID: <20140918181224.GO20389@hmsreliant.think-freely.org> (raw)
In-Reply-To: <59AF69C657FD0841A61C55336867B5B0343F358D@IRSMSX103.ger.corp.intel.com>

On Thu, Sep 18, 2014 at 04:01:32PM +0000, Richardson, Bruce wrote:
> > -----Original Message-----
> > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > Sent: Thursday, September 18, 2014 4:51 PM
> > To: Thomas Monjalon
> > Cc: Richardson, Bruce; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used RHEL
> > 6)
> > 
> > On Thu, Sep 18, 2014 at 05:38:58PM +0200, Thomas Monjalon wrote:
> > > Hi Neil,
> > >
> > > 2014-09-18 11:03, Neil Horman:
> > > > Thank you, I've reproduced the problem here, and you're right, it is a
> > > > compiler bug, but I really hate the idea of just throwing braces around
> > > > something to shut the compiler up, especially when the compiler has since
> > > > been fixed.  Looking at it a bit more closely it seems like the the thing
> > > > to do is actually just consistently use rte_mbuf_refcnt_set, since thats
> > > > what the rte_mbuf header documentation says to do, and protects you if the
> > > > internal structure changes, as well as prevents you from having to spread
> > > > ifdef RTE_MBUF_REFCNT all over the place.
> > > >
> > > > This patch does a bit more than that too.  With a little creative
> > > > typedef-ing we don't need the anonymous union at all, and lets us just use
> > > > a single refcnt variable, and I think eliminate that odd refcnt_reserved
> > > > member of the union, that, as far as I can see, does nothing.
> > >
> > > > --- a/config/common_linuxapp
> > > > +++ b/config/common_linuxapp
> > > > @@ -123,7 +123,7 @@ CONFIG_RTE_LOG_HISTORY=256
> > > >  CONFIG_RTE_LIBEAL_USE_HPET=n
> > > >  CONFIG_RTE_EAL_ALLOW_INV_SOCKET_ID=n
> > > >  CONFIG_RTE_EAL_ALWAYS_PANIC_ON_ERROR=n
> > > > -CONFIG_RTE_EAL_IGB_UIO=y
> > > > +CONFIG_RTE_EAL_IGB_UIO=n
> > > >  CONFIG_RTE_EAL_VFIO=y
> > >
> > > Hum, indeed this patch does a bit more ;)
> > >
> > Sorry, meant to clip that out, I was building without kernel headers, so I had
> > to disable igb_uio and kni.  I'll propose this patch properly if theres
> > consensus on the valid bits.
> > Neil
> 
> If we get rid of the REFCNT compile-time option (but not the REFCNT_ATOMIC one), then we should be able to get rid of the union, as we can do the atomic/non-atomic entirely inside the refcnt manipulation routines (since a regular uint16_t can be typecast directly to an atomic form of the same). Once we get rid of the union we can get rid of the extra braces that go along with the union, and we can just have the static initialiser cleanly put in the code.
> Whaddaya think?
> 
Yeah, that sounds good.  Reduces the code complexity by a good amount, makes
configuration simpler and allows for easy static initalization.
> /Bruce
> 

  reply	other threads:[~2014-09-18 18:07 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-18 10:55 Bruce Richardson
2014-09-18 11:09 ` Thomas Monjalon
2014-09-18 12:25   ` Neil Horman
2014-09-18 12:35     ` Richardson, Bruce
2014-09-18 15:03       ` Neil Horman
2014-09-18 15:16         ` Bruce Richardson
2014-09-18 15:46           ` Neil Horman
2014-09-18 15:38         ` Thomas Monjalon
2014-09-18 15:50           ` Neil Horman
2014-09-18 16:01             ` Richardson, Bruce
2014-09-18 18:12               ` Neil Horman [this message]
2014-09-25  2:13 ` Tang, HaifengX
2014-09-25  7:02   ` Thomas Monjalon
2014-09-25 13:07     ` Cao, Waterman
2014-09-25 15:05       ` [dpdk-dev] patches validation Thomas Monjalon
2014-09-25 23:29         ` Ananyev, Konstantin
2014-09-26  9: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=20140918181224.GO20389@hmsreliant.think-freely.org \
    --to=nhorman@tuxdriver.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    /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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git