On Mon, May 30, 2022 at 4:08 PM Jani Nikula jani.nikula@intel.com wrote:
On Mon, 30 May 2022, Arnd Bergmann arnd@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.
Arnd