From: Leon Romanovsky <leon@kernel.org>
To: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Cc: Greg Sword <gregsword0@gmail.com>,
Zhu Yanjun <yanjun.zhu@linux.dev>,
zyjzyj2000@gmail.com, jgg@ziepe.ca, linux-rdma@vger.kernel.org,
linux-kernel@vger.kernel.org,
"Gustavo A. R. Silva" <gustavoars@kernel.org>
Subject: Re: [PATCH v3 1/1] RDMA/rxe: Avoid -Wflex-array-member-not-at-end warnings
Date: Wed, 24 Dec 2025 08:51:07 +0200 [thread overview]
Message-ID: <20251224065107.GD11869@unreal> (raw)
In-Reply-To: <911ba345-7da6-4d05-955a-d33dd4b1e8c8@embeddedor.com>
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@embeddedor.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@embeddedor.com> wrote:
> > > > >
> > > > >
> > > > >
> > > > > On 12/23/25 13:41, Zhu Yanjun wrote:
> > > > > > From: "Gustavo A. R. Silva" <gustavoars@kernel.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@linux.dev>
> > > > > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.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 {
> > > > >
> > > > >
> > >
>
next prev parent reply other threads:[~2025-12-24 6:51 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-23 4:41 [PATCH v3 1/1] RDMA/rxe: Avoid -Wflex-array-member-not-at-end warnings Zhu Yanjun
2025-12-23 5:03 ` Gustavo A. R. Silva
2025-12-23 5:10 ` Zhu Yanjun
2025-12-23 5:18 ` Gustavo A. R. Silva
2025-12-23 5:26 ` Zhu Yanjun
2025-12-23 5:34 ` Gustavo A. R. Silva
2025-12-23 5:44 ` Zhu Yanjun
2025-12-23 5:54 ` Gustavo A. R. Silva
2025-12-23 6:46 ` Zhu Yanjun
2025-12-23 9:28 ` Gustavo A. R. Silva
2025-12-23 16:02 ` Zhu Yanjun
2025-12-23 14:20 ` Leon Romanovsky
2025-12-23 16:13 ` Zhu Yanjun
2025-12-23 9:34 ` Gustavo A. R. Silva
2025-12-23 16:38 ` Greg Sword
2025-12-23 16:59 ` Gustavo A. R. Silva
2025-12-23 17:19 ` Greg Sword
2025-12-23 17:26 ` Gustavo A. R. Silva
2025-12-24 6:51 ` Leon Romanovsky [this message]
2025-12-24 8:32 ` Gustavo A. R. Silva
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251224065107.GD11869@unreal \
--to=leon@kernel.org \
--cc=gregsword0@gmail.com \
--cc=gustavo@embeddedor.com \
--cc=gustavoars@kernel.org \
--cc=jgg@ziepe.ca \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=yanjun.zhu@linux.dev \
--cc=zyjzyj2000@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox