[PATCH] USB: ehci: use packed,aligned(4) instead of removing the packed attribute
stern at rowland.harvard.edu
Sun Jun 19 15:00:01 EDT 2011
On Sun, 19 Jun 2011, Nicolas Pitre wrote:
> On Thu, 16 Jun 2011, Arnd Bergmann wrote:
> > On Thursday 16 June 2011 22:10:53 Alexander Holler wrote:
> > > Using packed doesn't seem to be necessary (at least not with those
> > > versions of gcc I'm using here), I've tried both versions (on arm,
> > > without packed and with packed, aligned(4)) and both are working. I've
> > > only posted the patch because I found the usage of packed, aligned(4)
> > > much clearer than without packed. And It might help avoiding such
> > > discussions like this with people like me who aren't that deep involved
> > > in gcc-specific implementation details. ;)
> > >
> > > Anyway, feel free to nack that patch. I don't really care and just
> > > thought I should post it (e.g. as an alternative to removing that packed).
> > At least I would be happier without the patch. I'm trying to convince
> > people to not use these attributes unless required because too much
> > harm is done when they are used without understanding the full
> > consequences. I also recommend using __packed as localized as possible,
> > i.e. set it for the members that need it, not the entire struct.
> > I agree that your patch is harmless, it's just the opposite of
> > a cleanup in my opinion.
> The question is: does the structure really has to be packed?
What do you mean? The structure really does need to be allocated
without padding between the fields; is that the same thing? So do a
bunch of other structures that currently have no annotations at all.
> If it does, then the follow-up question is: is a packing on word
> boundaries sufficient?
Again, what do you mean? The structure contains some 32-bit fields and
it must always be allocated at a 4-byte boundary. However there's
nothing wrong with stricter allocation -- I don't recall the details
but it's entirely possible that some of the fields could be 64 bits on
some architectures, in which cases the alignment certainly should be
> If the answer is yes in both cases, then having packed,aligned(4) is not
> a frivolity but rather a correctness issue.
Why so? Current systems work just fine without it.
> We can of course provide a
> define in include/linux/compiler-gcc.hto hide the ugliness of it
> #define __packed_32 __attribute__((packed,aligned(4)))
> I suspect that the vast majority of the __packed uses in the kernel
> would be better with this __packed_32 instead, the actual need and
> intent would be more clearly expressed, and the generated code in the
> presence of those GCC changes would then be way more efficient and still
What if the intent is that the structure should be 4-byte aligned on
32-bit systems and 8-byte aligned on 64-bit systems? The compiler
already does this sort of thing automatically, why mess with it?
More information about the linux-arm-kernel