On Thu, Apr 4, 2024, at 09:56, Anton Protopopov wrote:
> 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.
Ah right. I misread your patch and assumed that you
gave the outer structure 2-byte alignment as well, but
it's only the inner union that needs it, which you did
correctly.
Arnd
