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

Arnd Bergmann arnd at arndb.de
Tue May 31 01:04:19 PDT 2022


On Tue, May 31, 2022 at 8:26 AM Julia Lawall <Julia.Lawall at inria.fr> wrote:
> > 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. ;)
> >>
...
> >>> 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.

As an experiment: what kind of results would we get when looking
for packed structures and unions that contain any of these:

- spinlock_t
- atomic_t
- dma_addr_t
- phys_addr_t
- size_t
- any pointer
- any enum
- struct mutex
- struct device

This is just a list of common data types that are used in a lot of
structures but that one should never find in hardware specific
types. If the output from coccinelle is 90% actual bugs, this would
be really helpful. OTOH if there is no output at all, or all
false-positives, we don't need to look for additional types.

> 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.

I would consider this a separate issue. The first one above is for identifying
structures that are marked as packed but should not be packed at
all, regardless of whether that changes the layout.

       Arnd



More information about the linux-arm-kernel mailing list