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

[Leon Romanovsky <leon@kernel.org>]

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?

> 
> 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.

> 
> 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
> 
> 
> 
>