Re: mainline build failure due to f1e4c916f97f ("drm/edid: add EDID block count and size helpers")

[Arnd Bergmann]

On Mon, May 30, 2022 at 3:10 PM Jani Nikula <jani.nikula@xxxxxxxxx> 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:

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 */

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 */

       Arnd