public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
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 {
> > > > > 
> > > > > 
> > > 
> 

  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