Hi, On Mon, Feb 22, 2021 at 09:53:17AM -0500, Mathieu Desnoyers wrote: > I notice that other structures defined in this UAPI header are not > packed as well. Should we add an attribute packed on new structures ? > It seems like it is generally a safer course of action, even though > each field is naturally aligned here (there is no padding/hole in the > structure). I considered this for quite a while. There are some gains for this approach, i.e. it's safer towards the ISO C, as theoretically compiler can generate arbitrary offsets as long as struct elements have correct order in memory. Also with packed attribute it would be harder to make it incorrect in future modifications. User code also could theoretically put the structure on any misaligned address. But the drawback is that all accesses to the structure contents are inefficient and some compilers may generate large chunks of code whenever the structure elements are accessed (I recall at least one ARM compiler which generates series of single-byte accesses for those). For kernel it doesn't matter much because the structure type is used in one place, but it may be different for the application code. The change would be also inconsistent with the rest of the file and IMO the gains are only theoretical. If there are more opinions on this or you have some argument I'm missing please let me know I can send v3 with packed and explicit padding removed. I think this is rather borderline trade off. Best regards and thanks for looking at this, Piotr.
