On Mon, 30 May 2022, Arnd Bergmann arnd@arndb.de wrote:
On Mon, May 30, 2022 at 3:10 PM Jani Nikula jani.nikula@intel.com wrote:
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.
The annotations for edid are completely correct and necessary. However other driver authors just slap __packed annotations on any structure even if the layout is not fixed at all like:
Right. Thanks for the examples.
struct my_driver_priv { struct device dev; u8 causes_misalignment; spinlock_t lock; atomic_t counter; } __packed; /* this annotation is harmful because it breaks the atomics */
I wonder if this is something that could be caught with coccinelle. Or sparse. Are there any cases where this combo is necessary? (I can't think of any, but it's a low bar. ;)
Cc: Julia.
or if the annotation does not change the layout like
struct my_dma_descriptor { __le64 address; __le64 length; } __packed; /* does not change layout but makes access slow on some architectures */
Why is this the case, though? I'd imagine the compiler could figure this out.
BR, Jani.