From: Anton Protopopov <aspsk@xxxxxxxxxxxxx>
Date: Thu, 4 Apr 2024 09:56:51 +0200
> On 24/04/04 12:31, Arnd Bergmann wrote:
>> On Wed, Apr 3, 2024, at 23:00, Daniel Borkmann wrote:
>>> On 4/3/24 10:09 PM, Arnd Bergmann wrote:
>>>> On Wed, Apr 3, 2024, at 14:33, Anton Protopopov wrote:
>>>>>
>>>>> Declare this inner union as __attribute__((packed, aligned(2))) such
>>>>> that it always is of size 2 and is aligned to 16 bits.
>>>>
>>>> I think you probably want 32-bit alignment for the structure,
>>>> to keep the ABI unchanged on all other architectures.
>>>
>>> Fwiw, on x86 nothing should change on this regard, see below pahole dump
>>> before/after. I think similar might be true for other archs as otherwise
>>> we should have seen a kbuild bot complaint on hitting the size assert.
>>
>> It's not the structure layout that changes, just its alignment.
>> Of course this is unlikely to cause actual bugs, but if there there
>> is no real need to change it, I would leave the alignment the same
>> as before.
>
> I think the struct will now be automatically 4-byte aligned, as it
> has the following layout:
>
> struct {
> u8 a;
> u8 b;
> u16 c;
> u16 d;
> union { u16 e; u16 f; } __aligned__(2);
> ...
> };
>
> So if the union is 2-byte aligned, then the struct is automatically
> 4-byte aligned, because its address is 6 bytes less than the address
> of the union. In fact, as Daniel posted above, pahole shows that the
> struct actually has __aligned__(4) attribute in the patched version.
> I can add explicit __aligned__(4) to make this clear.
I think it would be fine to not introduce an explicit attribute as long
as pahole shows the structure is already 4-byte aligned.
Regarding aligning it to 32 or 64 bytes -- the actual benefit can be
checked via scripts/bloat-o-meter. Usually alignment bigger than 8 bytes
don't change anything in the object code, at least on x86_64.
>
>> Arnd
Thanks,
Olek
