Re: [dpdk-dev] [PATCH] ixgbe: fix compile error with gcc4.4 (used RHEL 6)

[Neil Horman <nhorman@tuxdriver.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
>