[PATCH v2] libertas: Fix alignment issues in libertas core

Dan Williams dcbw at redhat.com
Thu Jan 29 19:04:34 EST 2009


On Thu, 2009-01-29 at 08:59 +0100, Erwin Authried wrote:
> Am Mittwoch, den 28.01.2009, 15:59 -0800 schrieb Colin McCabe:
> > On Wed, Jan 28, 2009 at 3:47 PM, Erwin Authried <eauth at softsys.co.at> wrote:
> > > Am Mittwoch, den 28.01.2009, 14:48 -0800 schrieb Colin McCabe:
> > >> The argument for specifying struct layout explicitly is pretty clear.
> > >> If we relied on gcc to get it right "by accident," we would find that
> > >> gcc upgrades and minor code changes would break the driver in
> > >> mysterious ways.
> > > you are probably right, it's a bit fragile, and there's no warranty
> > > that different/new architectures have the same rules for alignment of
> > > structures.
> > >
> > >>
> > >> I'm surprised that gcc generates poorer code when the layout is
> > >> specified explicitly, since after all, it should be the same layout.
> > >> Have you tried using __attribute__ ((aligned (8))) or similar?
> > >>
> > > it's not surprising because with the packed attribute gcc doesn't make
> > > any assumptions about the the alignment of the start of the structure.
> > > For example, if you compare the code for the following simple structure:
> > >
> > > struct a {
> > >  int i;
> > > };
> > >
> > > struct b {
> > >  int i;
> > > } __attribute__ ((packed));
> > 
> > That's very insightful.
> > 
> > I wonder if there is an __attribute__ or pragma that would force gcc
> > to do the right thing here.
> > Can you put __attribute__((aligned)) on a member of a packed struct? I
> > might have to test that out later...
> > 
> > Colin
> 
> Colin,
> great idea! I did a short test, the "aligned" attribute on the first
> struct member seems to do the job!
> This way, gcc knows that the struct is aligned and optimizes access to
> all members of the struct. We only have to make sure that the buffer for
> the struct is aligned.
> 
> struct a {
>   int i  __attribute__ ((aligned));
>   char x,y,z;
>   int j;
> } __attribute__ ((packed));

This is going to be ugly as sin.  Could somebody ask people who know how
this should work and whether the ((aligned)) thing is the right
approach?  That could be done with a mail to lkml to see what people
suggest.  I can't believe the libertas driver is the first driver to run
into this issue.

Dan





More information about the libertas-dev mailing list