Messages in this thread | | | From | Jani Nikula <> | Subject | Re: mainline build failure due to f1e4c916f97f ("drm/edid: add EDID block count and size helpers") | Date | Mon, 30 May 2022 17:08:11 +0300 |
| |
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.
-- Jani Nikula, Intel Open Source Graphics Center
| |