----- On Feb 26, 2021, at 9:11 AM, Piotr Figiel figiel@xxxxxxxxxx wrote: > 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. I personally don't have a strong opinion on this and completely agree with your analysis. Maybe for pre-existing system calls adding more non-packed structures might be kind-of OK if some were already exposed, even though it seems rather fragile wrt ISO C. Thanks, Mathieu > > Best regards and thanks for looking at this, > Piotr. -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
