Re: [PATCH][next] RDMA/rxe: Avoid -Wflex-array-member-not-at-end warnings

["Gustavo A. R. Silva" <gustavo@embeddedor.com>]

On 11/12/25 18:32, Leon Romanovsky wrote:
> On Wed, Nov 12, 2025 at 05:49:05PM +0900, Gustavo A. R. Silva wrote:
>>
>>
>> On 11/11/25 23:19, Leon Romanovsky wrote:
>>> On Tue, Nov 11, 2025 at 09:14:05PM +0900, Gustavo A. R. Silva wrote:
>>>>
>>>>
>>>> On 11/11/25 20:56, Leon Romanovsky wrote:
>>>>> On Tue, Nov 11, 2025 at 12:35:02PM +0900, Gustavo A. R. Silva wrote:
>>>>>> -Wflex-array-member-not-at-end was introduced in GCC-14, and we are
>>>>>> getting ready to enable it, globally.
>>>>>>
>>>>>> Use the new TRAILING_OVERLAP() helper to fix the following warning:
>>>>>>
>>>>>> 21 drivers/infiniband/sw/rxe/rxe_verbs.h:271:33: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
>>>>>>
>>>>>> This helper creates a union between a flexible-array member (FAM) and a
>>>>>> set of MEMBERS that would otherwise follow it.
>>>>>>
>>>>>> This overlays the trailing MEMBER struct ib_sge sge[RXE_MAX_SGE]; onto
>>>>>> the FAM struct rxe_recv_wqe::dma.sge, while keeping the FAM and the
>>>>>> start of MEMBER aligned.
>>>>>>
>>>>>> The static_assert() ensures this alignment remains, and it's
>>>>>> intentionally placed inmediately after the related structure --no
>>>>>> blank line in between.
>>>>>>
>>>>>> Lastly, move the conflicting declaration struct rxe_resp_info resp;
>>>>>> to the end of the corresponding structure.
>>>>>>
>>>>>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>>>>>> ---
>>>>>>     drivers/infiniband/sw/rxe/rxe_verbs.h | 18 +++++++++++-------
>>>>>>     1 file changed, 11 insertions(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
>>>>>> index fd48075810dd..6498d61e8956 100644
>>>>>> --- a/drivers/infiniband/sw/rxe/rxe_verbs.h
>>>>>> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
>>>>>> @@ -219,12 +219,6 @@ struct rxe_resp_info {
>>>>>>     	u32			rkey;
>>>>>>     	u32			length;
>>>>>> -	/* SRQ only */
>>>>>> -	struct {
>>>>>> -		struct rxe_recv_wqe	wqe;
>>>>>> -		struct ib_sge		sge[RXE_MAX_SGE];
>>>>>> -	} srq_wqe;
>>>>>> -
>>>>>>     	/* Responder resources. It's a circular list where the oldest
>>>>>>     	 * resource is dropped first.
>>>>>>     	 */
>>>>>> @@ -232,7 +226,15 @@ struct rxe_resp_info {
>>>>>>     	unsigned int		res_head;
>>>>>>     	unsigned int		res_tail;
>>>>>>     	struct resp_res		*res;
>>>>>> +
>>>>>> +	/* SRQ only */
>>>>>> +	/* Must be last as it ends in a flexible-array member. */
>>>>>> +	TRAILING_OVERLAP(struct rxe_recv_wqe, wqe, dma.sge,
>>>>>> +		struct ib_sge		sge[RXE_MAX_SGE];
>>>>>> +	) srq_wqe;
>>>>>
>>>>> Will this change be enough?
>>>>>
>>>>> diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
>>>>> index fd48075810dd..9ab11421a585 100644
>>>>> --- a/drivers/infiniband/sw/rxe/rxe_verbs.h
>>>>> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
>>>>> @@ -219,12 +219,6 @@ struct rxe_resp_info {
>>>>>            u32                     rkey;
>>>>>            u32                     length;
>>>>> -       /* SRQ only */
>>>>> -       struct {
>>>>> -               struct rxe_recv_wqe     wqe;
>>>>> -               struct ib_sge           sge[RXE_MAX_SGE];
>>>>> -       } srq_wqe;
>>>>> -
>>>>>            /* Responder resources. It's a circular list where the oldest
>>>>>             * resource is dropped first.
>>>>>             */
>>>>> @@ -232,6 +226,12 @@ struct rxe_resp_info {
>>>>>            unsigned int            res_head;
>>>>>            unsigned int            res_tail;
>>>>>            struct resp_res         *res;
>>>>> +
>>>>> +       /* SRQ only */
>>>>> +       struct {
>>>>> +               struct ib_sge           sge[RXE_MAX_SGE];
>>>>> +               struct rxe_recv_wqe     wqe;
>>>>> +       } srq_wqe;
>>>>>     };
>>>>
>>>> The question is if this is really what you want?
>>>>
>>>> sge[RXE_MAX_SGE] is of the following type:
>>>>
>>>> struct ib_sge {
>>>>           u64     addr;
>>>>           u32     length;
>>>>           u32     lkey;
>>>> };
>>>>
>>>> and struct rxe_recv_wqe::dma.sge[] is of type:
>>>>
>>>> struct rxe_sge {
>>>>           __aligned_u64 addr;
>>>>           __u32   length;
>>>>           __u32   lkey;
>>>> };
>>>>
>>>> Both types are basically the same, and the original code looks
>>>> pretty much like what people do when they want to pre-allocate
>>>> a number of elements (of the same element type as the flex array)
>>>> for a flexible-array member.
>>>>
>>>> Based on the above, the change you suggest seems a bit suspicious,
>>>> and I'm not sure that's actually what you want?
>>>
>>> You wrote about this error: "warning: structure containing a flexible array
>>> member is not at the end of another structure".
>>>
>>> My suggestion was simply to move that flex array to be the last element
>>> and save us from the need to have some complex, magic macro in RXE.
>>
>> Yep, but as I commented above, that doesn't seem to be the right change.
>>
>> Look at the following couple of lines:
>>
>> drivers/infiniband/sw/rxe/rxe_resp.c-286-       size = sizeof(*wqe) + wqe->dma.num_sge*sizeof(struct rxe_sge);
>> drivers/infiniband/sw/rxe/rxe_resp.c-287-       memcpy(&qp->resp.srq_wqe, wqe, size);
>>
>> Notice that line 286 is the open-coded arithmetic (struct_size(wqe,
>> dma.sge, wqe->dma.num_sge) is preferred) to get the number of bytes
>> to allocate for a flexible structure, in this case struct rxe_recv_wqe,
>> and its flexible-array member, in this case struct rxe_recv_wqe::dma.sge[].
>>
>> So, `size` bytes are written in qp->resp.srq_wqe, and the reason this works
>> seems to be because of the pre-allocation of RXE_MAX_SGE number of elements
>> for flex array struct rxe_recv_wqe::dma.sge[] given by:
>>
>> struct {
>> 	struct rxe_recv_wqe	wqe;
>> 	struct ib_sge		sge[RXE_MAX_SGE];
>> } srq_wqe;
> 
> So you are saying that it works because it is written properly, so what
> is the problem? Why do we need to fix properly working and written code
> to be less readable?

No one said the original code is not working as expected. The issue here is
that the FAM is not at the end, and this causes a -Wflex-array-member-not-at-end
warning. The change I propose places the FAM at the end, and the functionality
remains exactly the same.

You're probably not aware of the work we've been doing to enable
-Wflex-array-member-not-at-end in mainline. If you're interested, below you
can take a look at other similar changes I (and others) have been doing to
complete this work:

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log/?qt=grep&q=-Wflex-array-member-not-at-end

> 
>>
>> So, unless I'm missing something, struct ib_sge sge[RXE_MAX_SGE];
>> should be aligned with struct rxe_recv_wqe wqe::dma.sge[].
> 
> It is and moving to the end of struct will continue to keep it aligned.

I think there is something you are missing here. The following pieces of
code are no equivalent:

struct {
	struct rxe_recv_wqe	wqe;
  	struct ib_sge		sge[RXE_MAX_SGE];
} srq_wqe;

struct {
  	struct ib_sge		sge[RXE_MAX_SGE];
	struct rxe_recv_wqe	wqe;
} srq_wqe;

What I'm understanding from your last couple of responses is that you think
the above are equivalent. My previous response tried to explain why that is
not the case.

> 
>>
>> The TRAILING_OVERLAP() macro is also designed to ensure alignment in these
>> cases (and the static_assert() to preserve it). See this thread:
>>
>> https://lore.kernel.org/linux-hardening/aLiYrQGdGmaDTtLF@kspp/
>>

Thanks
-Gustavo