On Fri, Jun 11, 2021 at 5:39 AM Viresh Kumar via Stratos-dev stratos-dev@op-lists.linaro.org wrote:
On 10-06-21, 19:40, Jean-Philippe Brucker wrote:
On Thu, Jun 10, 2021 at 12:16:46PM +0000, Viresh Kumar via Stratos-dev wrote:
+} __packed;
No need for __packed, because the fields are naturally aligned (as required by the virtio spec)
Yeah, I know, but I tend to add that for structures which aren't very simple (like the request/response ones), just to avoid human errors and hours of debugging someone need to go through. __packed won't harm at least :)
Extraneous __packed annotations do cause real problems:
- On architectures without hardware unaligned accesses, the compiler is forced to emit byte load/store instructions, which is slower and breaks atomic updates to shared variables
- If a function takes a pointer of a packed struct member, and passes that pointer to a function that expects a regular aligned pointer, you get undefined behavior. Newer compilers produce a warning if you do that (we currently shut up that warning because there are many false positives in the kernel), but you can also run into CPU exceptions or broken code even on CPUs that do support unaligned accesses when the variable ends up being actually unaligned (as you just told the compiler that it is allowed to do).
Arnd
On 11-06-21, 10:34, Arnd Bergmann wrote:
Extraneous __packed annotations do cause real problems:
On architectures without hardware unaligned accesses, the compiler is forced to emit byte load/store instructions, which is slower and breaks atomic updates to shared variables
If a function takes a pointer of a packed struct member, and passes that pointer to a function that expects a regular aligned pointer, you
get undefined behavior. Newer compilers produce a warning if you do that (we currently shut up that warning because there are many false positives in the kernel), but you can also run into CPU exceptions or broken code even on CPUs that do support unaligned accesses when the variable ends up being actually unaligned (as you just told the compiler that it is allowed to do).
I understand that these problems will happen if the structure isn't aligned, but in this case the structure is aligned properly, just that we are explicitly telling the compiler to not add any padding (it won't have added any in here). Is it still harmful ?
stratos-dev@op-lists.linaro.org
-
Arnd Bergmann -
Viresh Kumar