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

[Julia Lawall]

> On 30 May 2022, at 15:27, Arnd Bergmann <arnd at arndb.de> wrote:
> 
> On Mon, May 30, 2022 at 4:08 PM Jani Nikula <jani.nikula at intel.com> wrote:
>>> On Mon, 30 May 2022, Arnd Bergmann <arnd at arndb.de> wrote:
>>> 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.
> 
> I think one would first have to make a list of data types that are not
> meant to be in a packed structure. It could be a good start to
> search for any packed aggregates with a pointer, atomic_t or spinlock_t
> in them, but there are of course many more types that you won't
> find in hardware structures.
> 
>>> 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.
> 
> When you annotate the entire structure as __packed without an
> extra __aligned() annotation, the compiler has to assume that the
> structure itself is unaligned as well. On many of the older architectures,
> this will result in accessing the values one byte at a time. Marking
> the structure as "__packed __aligned(8)" instead would be harmless.
> 
> When I have a structure with a few misaligned members, I generally
> prefer to only annotate the members that are not naturally aligned,
> but this approach is not very common.

Searching for specific types in a packed structure would be easy.

Coccinelle could duplicate the structure without the packed and see if any offsets change, using build bug on, but that would be architecture specific so maybe not useful.

Julia



>         Arnd