Re: [ANNOUNCE] libata EH/NCQ/hotplug/PM git tree

[Tejun Heo <htejun@gmail.com>]

Hi, Zhao.

zhao, forrest wrote:
> On Wed, 2006-05-10 at 10:44 +0900, Tejun Heo wrote:
>> Hello, all.
> Tejun,
> 
> I have comments about definition of macro ata_link_for_each_dev(dev,
> link) and struct ata_port{};.
> 
> In the definition of struct ata_port{}, there's
> ......
> struct ata_link link;
> struct ata_device __dev1;
> ......
> 
> Then macro ata_link_for_each_dev() assumes that the field 'device' in
> struct ata_link is adjacent to field '__dev1' in struct ata_port.
> 
> I think this assumption is not correct in theory. Because the alignment
> may make these two fields not adjacent in memory.
> 
> Although we haven't found the problem so far, it's very dangerous to
> have such assumption in the code.

Well, I think the technique is pretty widespread w/ flexible array
member, which is C99 standard and before C99 GCC had zero length array
for the same purpose.  e.g.

struct asdf {
	int nr_entries;
	void *ar[];
};

struct asdf *p;

p = kmalloc(sizeof(*p) + nr * sizeof(p->ar[0], GFP_KERNEL);
if (p)
	p->nr_entries = nr;

This works because alignof(outer struct) >= alignof(nested struct).  So,
we might end up allocating a few extra bytes due to alignment
requirements but that's perfectly okay.

__dev1 is the same.  It might or might not be allocated right after
link->dev[0] but it's guaranteed that &link->dev[1] <= &link->__dev1.
So, no problem there.  I'm pretty sure that this kind of technique is
used in other parts of the kernel too.

-- 
tejun