[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251224065107.GD11869@unreal>
Date: Wed, 24 Dec 2025 08:51:07 +0200
From: Leon Romanovsky <leon@...nel.org>
To: "Gustavo A. R. Silva" <gustavo@...eddedor.com>
Cc: Greg Sword <gregsword0@...il.com>, Zhu Yanjun <yanjun.zhu@...ux.dev>,
zyjzyj2000@...il.com, jgg@...pe.ca, linux-rdma@...r.kernel.org,
linux-kernel@...r.kernel.org,
"Gustavo A. R. Silva" <gustavoars@...nel.org>
Subject: Re: [PATCH v3 1/1] RDMA/rxe: Avoid -Wflex-array-member-not-at-end
warnings
On Wed, Dec 24, 2025 at 02:26:22AM +0900, Gustavo A. R. Silva wrote:
>
>
> On 12/24/25 02:19, Greg Sword wrote:
> > On Wed, Dec 24, 2025 at 12:59 AM Gustavo A. R. Silva
> > <gustavo@...eddedor.com> wrote:
> > >
> > >
> > >
> > > On 12/24/25 01:38, Greg Sword wrote:
> > > > On Tue, Dec 23, 2025 at 5:35 PM Gustavo A. R. Silva
> > > > <gustavo@...eddedor.com> wrote:
> > > > >
> > > > >
> > > > >
> > > > > On 12/23/25 13:41, Zhu Yanjun wrote:
> > > > > > From: "Gustavo A. R. Silva" <gustavoars@...nel.org>
> > > > > >
> > > > > > -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.
> > > > > >
> > > > > > Reviewed-by: Zhu Yanjun <yanjun.zhu@...ux.dev>
> > > > > > Signed-off-by: Gustavo A. R. Silva <gustavoars@...nel.org>
> > > > >
> > > > > NACK.
> > > >
> > > > Just a small reminder about community conventions: reviewers can NACK
> > > > a patch, but authors generally should not NACK their own patches.
> > >
> > > It's obvious that you don't understand what's going on here.
> >
> > I’ve read through the full discussion and understand how this evolved.
> > Based on that, I believe there may be a misunderstanding on your side.
>
> Okay, thanks for your contribution then.
Gustavo,
A more fundamental issue than patch granularity is simply treating others
with respect, even when you disagree with them.
Zhu cares about RXE, and he respected your work by trying to address your
problem, not his own. RXE functions correctly without your patch, and this
hardening effort does not apply to RXE at all.
So please remember that both of you want the same thing, and let's keep the
discussion focused on the technical issues.
Thanks
>
> -Gustavo
>
> >
> > >
> > > > > I didn't write this patch.
> > >
> > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > This line should've given you a clue.
> > >
> > > -Gustavo
> > >
> > > > >
> > > > > Please, don't ever submit modified patches on my behalf.
> > > > >
> > > > > > ---
> > > > > > V2->V3: Replace struct ib_sge with struct rxe_sge
> > > > >
> > > > > Patch granularity is a fundamental thing. Changes addressing different
> > > > > issues should not be mixed together. Previously existing issues (if any)
> > > > > must be addressed in separate patches.
> > > > >
> > > > > -Gustavo
> > > > >
> > > > > > ---
> > > > > > 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..3ffd7be8e7b1 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 rxe_sge sge[RXE_MAX_SGE];
> > > > > > + ) srq_wqe;
> > > > > > };
> > > > > > +static_assert(offsetof(struct rxe_resp_info, srq_wqe.wqe.dma.sge) ==
> > > > > > + offsetof(struct rxe_resp_info, srq_wqe.sge));
> > > > > >
> > > > > > struct rxe_qp {
> > > > > > struct ib_qp ibqp;
> > > > > > @@ -269,7 +271,6 @@ struct rxe_qp {
> > > > > >
> > > > > > struct rxe_req_info req;
> > > > > > struct rxe_comp_info comp;
> > > > > > - struct rxe_resp_info resp;
> > > > > >
> > > > > > atomic_t ssn;
> > > > > > atomic_t skb_out;
> > > > > > @@ -289,6 +290,9 @@ struct rxe_qp {
> > > > > > spinlock_t state_lock; /* guard requester and completer */
> > > > > >
> > > > > > struct execute_work cleanup_work;
> > > > > > +
> > > > > > + /* Must be last as it ends in a flexible-array member. */
> > > > > > + struct rxe_resp_info resp;
> > > > > > };
> > > > > >
> > > > > > enum {
> > > > >
> > > > >
> > >
>
Powered by blists - more mailing lists