-----"Jason Gunthorpe" <jgg@xxxxxxxx> wrote: -----
>To: "Bernard Metzler" <BMT@xxxxxxxxxxxxxx>
>From: "Jason Gunthorpe" <jgg@xxxxxxxx>
>Date: 08/06/2019 06:39PM
>Cc: linux-rdma@xxxxxxxxxxxxxxx
>Subject: Re: Re: [PATCH 1/1] Make user mmapped CQ arming flags field
>32 bit size to remove 64 bit architecture dependency of siw.
>
>On Tue, Aug 06, 2019 at 04:36:26PM +0000, Bernard Metzler wrote:
>>
>> >To: "Bernard Metzler" <BMT@xxxxxxxxxxxxxx>
>> >From: "Jason Gunthorpe" <jgg@xxxxxxxx>
>> >Date: 08/06/2019 05:31PM
>> >Cc: linux-rdma@xxxxxxxxxxxxxxx
>> >Subject: Re: Re: [PATCH 1/1] Make user mmapped CQ arming flags
>field
>> >32 bit size to remove 64 bit architecture dependency of siw.
>> >
>> >On Tue, Aug 06, 2019 at 02:53:49PM +0000, Bernard Metzler wrote:
>> >
>> >> >> index 7de68f1dc707..af735f55b291 100644
>> >> >> +++ b/include/uapi/rdma/siw-abi.h
>> >> >> @@ -180,6 +180,7 @@ struct siw_cqe {
>> >> >> * to control CQ arming.
>> >> >> */
>> >> >> struct siw_cq_ctrl {
>> >> >> - __aligned_u64 notify;
>> >> >> + __u32 flags;
>> >> >> + __u32 pad;
>> >> >
>> >> >The commit message needs to explain why this is compatible with
>> >> >existing user space, if it is even is safe..
>> >> >
>> >> Old libsiw would remain compatible with the new layout, since it
>> >> simply reads the 32bit 'flags' and zeroed 32bit 'pad' into a
>64bit
>> >> 'notify', ending with reading the same bits.
>> >
>> >Even on big endian?
>> >
>> Well I do not have access to a BE system right now to verify.
>> But on a BE system, the lowest 3 bits (which are in use) of the
>first
>> 32bit variable 'flags' shall be the lowest (leftmost) 3 bits of an
>> 'overlayed' 64bit variable 'notify' as well...
>
>One of LE or BE won't work with this scheme, it can't, the flag bit
>will end up in the pad.
>
Sitting here on a x86 (LE), and it works. On a 64bits machine,
two consecutive 32bits are obviously reordered in memory. Leaves
32bit LE broken, which is currently not supported.
Anyway, what would you suggest as the best path forward? A new ABI
version? If we move to test_and_clear_bit(), 'flags' size would
become architecture dependent...and we would break the ABI as well,
no?
Best regards,
Bernard.
