RE: [RESEND PATCH v2] devres: Really align data field to unsigned long long

From: David Laight
Date: Mon Jul 09 2018 - 05:15:14 EST


From: Alexey Brodkin
> Sent: 09 July 2018 05:45
> Depending on ABI "long long" type of a particular 32-bit CPU
> might be aligned by either word (32-bits) or double word (64-bits).
> Make sure "data" is really 64-bit aligned for any 32-bit CPU.
>
> At least for 32-bit ARC cores ABI requires "long long" types
> to be aligned by normal 32-bit word. This makes "data" field aligned to
> 12 bytes. Which is still OK as long as we use 32-bit data only.
>
> But once we want to use native atomic64_t type (i.e. when we use special
> instructions LLOCKD/SCONDD for accessing 64-bit data) we easily hit
> misaligned access exception.

Shouldn't there be a typedef for the actual type.
Perhaps it is even atomic64_t ?
And have the __aligned(8) applied to that typedef ??

> That's because even on CPUs capable of non-aligned data access LL/SC
> instructions require strict alignment.
...
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -24,8 +24,12 @@ struct devres_node {
>
> struct devres {
> struct devres_node node;
> - /* -- 3 pointers */
> - unsigned long long data[]; /* guarantee ull alignment */
> + /*
> + * Depending on ABI "long long" type of a particular 32-bit CPU
> + * might be aligned by either word (32-bits) or double word (64-bits).
> + * Make sure "data" is really 64-bit aligned for any 32-bit CPU.

Just:
/* data[] must be 64 bit aligned even on 32 bit architectures
* because it might be accessed by instructions that require
* aligned memory arguments.

> + */
> + unsigned long long data[] __aligned(sizeof(unsigned long long));

One day assuming that 'unsigned long long' is exactly 64 bits will bite.
This probably ought to be u64 (or similar).
(If not atomic64_t)

David