RE: [PATCH 2/9] ocrdma: Driver for Emulex OneConnect RDMA adapter

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



> -----Original Message-----
> From: Jason Gunthorpe [mailto:jgunthorpe@xxxxxxxxxxxxxxxxxxxx]
> Sent: Friday, March 23, 2012 4:14 AM
> To: Pandit, Parav
> Cc: David.Laight@xxxxxxxxxx; roland@xxxxxxxxxxxxxxx; linux-
> rdma@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 2/9] ocrdma: Driver for Emulex OneConnect RDMA
> adapter
> 
> On Thu, Mar 22, 2012 at 02:20:28PM -0700, Parav.Pandit@xxxxxxxxxx
> wrote:
> > I got a question here lately.
> >
> > aligned directive will ensure that it will fall on boundary.  Say
> > aligned(4) ensures that structure is aligned to 4 byte boundary.
> > Compiler can (at least theoretically) still have 4 byte structure
> > aligned to 8 byte boundary on 64-bit platform (which is 4 byte aligned
> > too).
> 
> There are very specific rules defined in the platform's ABI for how C
> structures are layed out in memory, each ABI (ie CPU) has its own specific
> quirks, but broadly in Linux land you can boil it down to:
> 
> 1) The alignment of a structure is the greatest alignment of all the
>    members
> 2) Each member is aligned to its alignment.
> 
> The alignment of the structure drives the total size of the structure, and
> specifically the padding added at the end to reach that alignment.
> 
> So, no, a compiler that increased the alignment of a struct with one
> u32 to 8 would violate the various ABIs and not be usuable for Linux. It is
> important to bear in mind that Linux targets a particular set of ABI
> conventions, and it is not 'anything goes'.
> 
> > struct {
> > 	u32 field;
> > };
> 
> So in this case: the u32 is aligned to 4, the structure is aligned to
> 4 and the total size of the structure is 4 on everything linux supports.
> 
> > struct {
> >       u64 fielda
> > 	u32 field;
> > };
> 
> In this case: On 64 bit: the u64 is aligned to 8 and the u32 is aligned to 4. So
> the structure is aligned to 8. A pad is inserted at the end of the struct to bring
> it out. On 32 bit, the u64 is aligned to 4, so the struct is aligned to 4, so no pad
> is added.
> 
> > struct {
> >       __float128 fielda
> > 	u32 field;
> > };
> 
> In this case the float128 is aligned to 16 and thus the structure is aligned to 16
> and 12 pad bytes are added.
> 
> > However requirement is to have this structure only 4 byte size(
> > because adapter excepts it to be 4B sise) and therefor packed is used.
> > I don't know the way to ensure size of 4 byte and alignment too.  Or I
> > am misunderstanding?
> 
> Yes, you are mis-understanding the rules for padding.. Structures are only
> padded out to their alignment, which depends on their constituent types.
> This is so arrays of structures have each array element starting on its natural
> alignment.
> 
> The aligned attribute overrides the automatic determination of the
> alignment based on the contents and just forces it.
> 
> So, as an example, if you have this hardware layout:
> 
> struct {
>   u32 fielda;
>   u64 fieldb;
> } attribute ((aligned(4));
> 
> David is saying you will get a 12 byte struct and fieldb will be unaligned. Since
> 12 is aligned to 4 no padding is added.

So I decided to experiment above example before implementing in driver. However I find structure of 16 bytes (instead of 12) with padding after fielda in below example.
Am I missing some compiler option or syntax error in attribute? Sorry to ask this silly question.
I tried __attribute__((__aligned__(4))); too based on usage in other kernel code.

#include <linux/module.h>

struct foo {
        u32 a;
        u64 b;
} __attribute__((aligned(4)));

static int __init main_init(void)
{
        printk("<1> sizeof foo=%ld\n", sizeof(struct foo));
        printk("<1> offset of a=%ld\n", offsetof(struct foo, a));
        printk("<1> offset of b=%ld\n", offsetof(struct foo, b));
        return 0;
}
static void __exit main_exit(void)
{
}
module_init(main_init);
module_exit(main_exit);
Output:
Mar 24 05:44:10 parav-sles11-sp1-64 kernel: [2006356.094931]  sizeof foo=16
Mar 24 05:44:10 parav-sles11-sp1-64 kernel: [2006356.094934]  offset of a=0
Mar 24 05:44:10 parav-sles11-sp1-64 kernel: [2006356.094936]  offset of b=8

Gcc version is below.

Using built-in specs.
Target: x86_64-suse-linux
Configured with: ../configure --prefix=/usr --infodir=/usr/share/info --mandir=/usr/share/man --libdir=/usr/lib64 --libexecdir=/usr/lib64 --enable-languages=c,c++,objc,fortran,obj-c++,java,ada --enable-checking=release --with-gxx-include-dir=/usr/include/c++/4.3 --enable-ssp --disable-libssp --with-bugurl=http://bugs.opensuse.org/ --with-pkgversion='SUSE Linux' --disable-libgcj --disable-libmudflap --with-slibdir=/lib64 --with-system-zlib --enable-__cxa_atexit --enable-libstdcxx-allocator=new --disable-libstdcxx-pch --enable-version-specific-runtime-libs --program-suffix=-4.3 --enable-linux-futex --without-system-libunwind --with-cpu=generic --build=x86_64-suse-linux
Thread model: posix
gcc version 4.3.4 [gcc-4_3-branch revision 152973] (SUSE Linux)

> For hardware facing structures I'd combine this with a static assert to verify
> structure size at compile time.
> 
> So..
> 
> 1) Avoid using attributes unless the structure has unaligned members.
> 2) Avoid creating structures with unaligned members (eg for userspace
>    communication)
> 3) Frown at hardware/firmware developers who make communication
>    structures with unaligned members :)
> 4) Be explicit about padding in your layout for 64/32
>    compatibility.
> 
> Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Home]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]    [Yosemite Photos]    [Free Online Dating]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Devices]

Add to Google Powered by Linux