On 5/20/21 7:48 AM, Jason Gunthorpe wrote:
> On Tue, May 11, 2021 at 08:27:50PM -0700, Bart Van Assche wrote:
>> @@ -107,10 +107,10 @@ struct srp_direct_buf {
>> * having the 20-byte structure padded to 24 bytes on 64-bit architectures.
>> */
>> struct srp_indirect_buf {
>> - struct srp_direct_buf table_desc;
>> + struct srp_direct_buf table_desc __packed;
>> __be32 len;
>> - struct srp_direct_buf desc_list[];
>> -} __attribute__((packed));
>> + struct srp_direct_buf desc_list[] __packed;
>> +};
>>
>> /* Immediate data buffer descriptor as defined in SRP2. */
>> struct srp_imm_buf {
>> @@ -175,13 +175,13 @@ struct srp_login_rsp {
>> u8 opcode;
>> u8 reserved1[3];
>> __be32 req_lim_delta;
>> - u64 tag;
>> + u64 tag __packed;
>
> What you really want is just something like this:
>
> typedef u64 __attribute__((aligned(4))) compat_u64;
>
> And then use that every place you have the u64 and forget about packed
> entirely.
Really? My understanding is that the aligned attribute can be used to
increase alignment of a structure member but not to decrease it. From
https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#Common-Variable-Attributes:
"When used on a struct, or struct member, the aligned attribute can only
increase the alignment; in order to decrease it, the packed attribute
must be specified as well."
Additionally, there is a recommendation in
Documentation/process/coding-style.rst not to introduce new typedefs.
> Except for a couple exceptions IBA mads are always aligned to 4 bytes,
> only the 64 bit quantities are unaligned.
>
> But really this whole thing should be replaced with the IBA_FIELD
> macros like include/rdma/ibta_vol1_c12.h demos.
>
> Then it would be sparse safe and obviously endian correct as well.
I prefer a structure over the IBA_FIELD macros so I will change __packed
into __packed __aligned(4).
Thanks,
Bart.
