On Mon, 30 May 2022, Arnd Bergmann [off-list ref] wrote:

On Mon, May 30, 2022 at 11:33 AM Jani Nikula [off-list ref] wrote:

quoted

That is, for EDID. Makes you wonder about all the other packed structs
with enum members across the kernel.

It is not the 'enum' that is special here, it's the 'union' having
unpacked members,

Obviously meant union not enum, that was just a -ENOCOFFEE on my part.

and the same thing happens when you have nested structs: both the inner
and the outer aggregate need to be packed, either with __packed at the
end, or on each individual member that is not fully aligned to
max(sizeof(member), 4)).

I think in general, most __packed annotations we have in the kernel are
completely pointless because they do not change the structure layout on
any architecture but instead just make member access slower on

Please explain.

They are used quite a bit for parsing blob data, or
serialization/deserialization, like in the EDID case at hand. Try
removing __attribute__((packed)) from include/drm/drm_edid.h and see the
sizeof(struct edid) on any architecture.

BR,
Jani.

architectures that lack unaligned load/store instructions. There have
definitely been other cases though where a __packed annotation is
not needed on any sane architecture but is needed for OABI ARM.

Overall I'm not that worried because the only machines running OABI
kernels would be on really old hardware that runs a limited set of
driver code.

A completely different matter are the extraneous __packed annotations
that lead to possible problems when accessed through a misaligned
pointer. We ignore -Waddress-of-packed-member and -Wcast-align
in the kernel, so these never get caught at build time, but we have
seen bugs from gcc making incorrect assumptions about alignment
even on architectures that have unaligned load/store instructions.

      Arnd

-- 
Jani Nikula, Intel Open Source Graphics Center

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel