RE: Re: Re: [PATCH] RDMA/siw: Trim size of page array to max size needed

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Leon Romanovsky <leon@xxxxxxxxxx>
> Sent: Friday, January 26, 2024 12:06 PM
> To: Bernard Metzler <BMT@xxxxxxxxxxxxxx>
> Cc: Guoqing Jiang <guoqing.jiang@xxxxxxxxx>; linux-rdma@xxxxxxxxxxxxxxx;
> jgg@xxxxxxxx; ionut_n2001@xxxxxxxxx
> Subject: [EXTERNAL] Re: Re: [PATCH] RDMA/siw: Trim size of page array to
> max size needed
> 
> On Thu, Jan 25, 2024 at 05:27:52PM +0000, Bernard Metzler wrote:
> >
> >
> > > -----Original Message-----
> > > From: Guoqing Jiang <guoqing.jiang@xxxxxxxxx>
> > > Sent: Thursday, January 25, 2024 1:15 AM
> > > To: Bernard Metzler <BMT@xxxxxxxxxxxxxx>; linux-rdma@xxxxxxxxxxxxxxx
> > > Cc: jgg@xxxxxxxx; leon@xxxxxxxxxx; ionut_n2001@xxxxxxxxx
> > > Subject: [EXTERNAL] Re: [PATCH] RDMA/siw: Trim size of page array to
> max
> > > size needed
> > >
> > > Hi Bernard,
> > >
> > > On 1/25/24 03:59, Bernard Metzler wrote:
> > > >> -----Original Message-----
> > > >> From: Guoqing Jiang <guoqing.jiang@xxxxxxxxx>
> > > >> Sent: Tuesday, January 23, 2024 3:43 AM
> > > >> To: Bernard Metzler <BMT@xxxxxxxxxxxxxx>; linux-rdma@xxxxxxxxxxxxxxx
> > > >> Cc: jgg@xxxxxxxx; leon@xxxxxxxxxx; ionut_n2001@xxxxxxxxx
> > > >> Subject: [EXTERNAL] Re: [PATCH] RDMA/siw: Trim size of page array to
> > > max
> > > >> size needed
> > > >>
> > > >> Hi Bernard,
> > > >>
> > > >> On 1/19/24 21:05, Bernard Metzler wrote:
> > > >>> siw tries sending all parts of an iWarp wire frame in one socket
> > > >>> send operation. If user data can be send without copy, user data
> > > >>> pages for one wire frame are referenced in an fixed size page
> array.
> > > >>> The size of this array can be made 2 elements smaller, since it
> > > >>> does not reference iWarp header and trailer crc. Trimming
> > > >>> the page array reduces the affected siw_tx_hdt() functions frame
> > > >>> size, staying below 1024 bytes. This avoids the following
> > > >>> compile-time warning:
> > > >>>
> > > >>>    drivers/infiniband/sw/siw/siw_qp_tx.c: In function 'siw_tx_hdt':
> > > >>>    drivers/infiniband/sw/siw/siw_qp_tx.c:677:1: warning: the frame
> > > >>>    size of 1040 bytes is larger than 1024 bytes [-Wframe-larger-
> > > than=]
> > > >> I saw similar warning in my ubuntu 22.04 VM which has below gcc.
> > > >>
> > > >> root@buk:/home/gjiang/linux-mirror# make
> M=drivers/infiniband/sw/siw/
> > > >> -j16 W=1
> > > >>     CC [M]  drivers/infiniband/sw/siw/siw_qp_tx.o
> > > >> drivers/infiniband/sw/siw/siw_qp_tx.c: In function ‘siw_tx_hdt’:
> > > >> drivers/infiniband/sw/siw/siw_qp_tx.c:665:1: warning: the frame size
> > > of
> > > >> 1440 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> > > >>     665 | }
> > > >>         | ^
> > > >>
> > > > Whew.. that is quite substantially off the target!
> > > > How come different compilers making so much of a difference.
> > > > Guoqing, can you check if the macro computing the maximum number
> > > > of fragments is broken, i.e., computes different values in
> > > > the cases you refer?
> > >
> > > Sorry, I was wrong 😅.
> > >
> > > The warning is not relevant with compiler, and it also appears with
> gcc-
> > > 13.1
> > > after enable KASAN which is used to find out-of-bounds bugs. Also,
> there
> > > are lots of -Wframe-larger-than warning from other places as well.
> > >
> > > > Thanks a lot!
> > > > Bernard
> > > >> # gcc --version
> > > >> gcc (Ubuntu 12.3.0-1ubuntu1~22.04) 12.3.0
> > > >>
> > > >> And it still appears after apply this patch on top of 6.8-rc1.
> > > >>
> > > >> root@buk:/home/gjiang/linux-mirror# git am
> > > >>
> > > ./20240119_bmt_rdma_siw_trim_size_of_page_array_to_max_size_needed.mbx
> > > >> Applying: RDMA/siw: Trim size of page array to max size needed
> > > >> root@buk:/home/gjiang/linux-mirror# make
> M=drivers/infiniband/sw/siw/
> > > >> -j16 W=1
> > > >>     CC [M]  drivers/infiniband/sw/siw/siw_qp_tx.o
> > > >> drivers/infiniband/sw/siw/siw_qp_tx.c: In function ‘siw_tx_hdt’:
> > > >> drivers/infiniband/sw/siw/siw_qp_tx.c:668:1: warning: the frame size
> > > of
> > > >> 1408 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> > > >>     668 | }
> > > >>         | ^
> > >
> > > The patch actually reduced the frame size from 1440 to 1408 though it
> is
> > > still larger than 1024.
> > >
> >
> > So in your opinion, does this patch fix the issue of having a
> > frame size larger than 1024 bytes for a typical build? I am sure
> > we do not want to optimize the driver for building with KASAN
> > debug options enabled.
> 
> But this is how we are running or supposed to run kernels. In any
> sane regression run, KASAN is enabled.
> 
> I would speculate that most people who run SIW, use it to test their ULPs.
> 
> So I would like to see it fixed for them too.

Understood. I propose to take the patch as I sent for now as a fix
of the problem under the conditions reported. I'll look into
restructuring the transmit path to squeeze its size below 1024
even for KASAN builds. It will require some time.

Kernel builds with KASAN enabled emit lots of similar compile
time warnings reporting frame sizes above 1024 bytes. Our core/nldev.c
alone spills 13 of those. Any action needed? ;)

Best,
Bernard
> 
> Thanks
> 
> >
> > The original bug report claimed a frame size of 1040 bytes for a
> > build w/o KASAN, being larger than 1024 bytes by 16 bytes. I
> > think this patch fixes the reported issue.
> >
> > Thanks a lot,
> > Bernard.
> >
> >
> > > Thanks,
> > > Guoqing
> >




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux