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

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

[Bernard Metzler]

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

Fixes: b9be6f18cf9e ("rdma/siw: transmit path")
Reported-by: ionut_n2001@yahoo.com
Link: https://bugzilla.kernel.org/show_bug.cgi?id=218375
Signed-off-by: Bernard Metzler <bmt@zurich.ibm.com>
---
 drivers/infiniband/sw/siw/siw_qp_tx.c | 29 +++++++++++++++------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_qp_tx.c b/drivers/infiniband/sw/siw/siw_qp_tx.c
index 64ad9e0895bd..7ffc91bac606 100644
--- a/drivers/infiniband/sw/siw/siw_qp_tx.c
+++ b/drivers/infiniband/sw/siw/siw_qp_tx.c
@@ -432,7 +432,7 @@ static int siw_tx_hdt(struct siw_iwarp_tx *c_tx, struct socket *s)
 	struct siw_wqe *wqe = &c_tx->wqe_active;
 	struct siw_sge *sge = &wqe->sqe.sge[c_tx->sge_idx];
 	struct kvec iov[MAX_ARRAY];
-	struct page *page_array[MAX_ARRAY];
+	struct page *page_array[MAX_ARRAY-2];
 	struct msghdr msg = { .msg_flags = MSG_DONTWAIT | MSG_EOR };
 
 	int seg = 0, do_crc = c_tx->do_crc, is_kva = 0, rv;
@@ -491,7 +491,6 @@ static int siw_tx_hdt(struct siw_iwarp_tx *c_tx, struct socket *s)
 
 		while (sge_len) {
 			size_t plen = min((int)PAGE_SIZE - fp_off, sge_len);
-			void *kaddr;
 
 			if (!is_kva) {
 				struct page *p;
@@ -503,14 +502,12 @@ static int siw_tx_hdt(struct siw_iwarp_tx *c_tx, struct socket *s)
 					rv = -EFAULT;
 					goto done_crc;
 				}
-				page_array[seg] = p;
-
 				if (!c_tx->use_sendpage) {
-					void *kaddr = kmap_local_page(p);
+					void *pa = kmap_local_page(p);
 
 					/* Remember for later kunmap() */
 					kmap_mask |= BIT(seg);
-					iov[seg].iov_base = kaddr + fp_off;
+					iov[seg].iov_base = pa + fp_off;
 					iov[seg].iov_len = plen;
 
 					if (do_crc)
@@ -518,12 +515,17 @@ static int siw_tx_hdt(struct siw_iwarp_tx *c_tx, struct socket *s)
 							c_tx->mpa_crc_hd,
 							iov[seg].iov_base,
 							plen);
-				} else if (do_crc) {
-					kaddr = kmap_local_page(p);
-					crypto_shash_update(c_tx->mpa_crc_hd,
-							    kaddr + fp_off,
-							    plen);
-					kunmap_local(kaddr);
+				} else {
+					page_array[seg] = p;
+					if (do_crc) {
+						void *pa = kmap_local_page(p);
+
+						crypto_shash_update(
+							c_tx->mpa_crc_hd,
+							pa + fp_off,
+							plen);
+						kunmap_local(pa);
+					}
 				}
 			} else {
 				/*
@@ -545,7 +547,8 @@ static int siw_tx_hdt(struct siw_iwarp_tx *c_tx, struct socket *s)
 			data_len -= plen;
 			fp_off = 0;
 
-			if (++seg >= (int)MAX_ARRAY) {
+			if (++seg >= (int)MAX_ARRAY ||
+			    (c_tx->use_sendpage && seg >= (int)MAX_ARRAY-2)) {
 				siw_dbg_qp(tx_qp(c_tx), "to many fragments\n");
 				siw_unmap_pages(iov, kmap_mask, seg-1);
 				wqe->processed -= c_tx->bytes_unsent;
-- 
2.38.1

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

[Guoqing Jiang]

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

# 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 | }
       | ^

However with gcc-13.1, I can't see the warning with or without the patch.

Thanks,
Guoqing

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

[Bernard Metzler]

> -----Original Message-----
> From: Guoqing Jiang <guoqing.jiang@linux.dev>
> Sent: Tuesday, January 23, 2024 3:43 AM
> To: Bernard Metzler <BMT@zurich.ibm.com>; linux-rdma@vger.kernel.org
> Cc: jgg@ziepe.ca; leon@kernel.org; ionut_n2001@yahoo.com
> 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?

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 | }
>        | ^
> 
> However with gcc-13.1, I can't see the warning with or without the
> patch.
> 
> Thanks,
> Guoqing

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

[Guoqing Jiang]

Hi Bernard,

On 1/25/24 03:59, Bernard Metzler wrote:
>> -----Original Message-----
>> From: Guoqing Jiang <guoqing.jiang@linux.dev>
>> Sent: Tuesday, January 23, 2024 3:43 AM
>> To: Bernard Metzler <BMT@zurich.ibm.com>; linux-rdma@vger.kernel.org
>> Cc: jgg@ziepe.ca; leon@kernel.org; ionut_n2001@yahoo.com
>> 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.

Thanks,
Guoqing

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

[Bernard Metzler]

> -----Original Message-----
> From: Guoqing Jiang <guoqing.jiang@linux.dev>
> Sent: Thursday, January 25, 2024 1:15 AM
> To: Bernard Metzler <BMT@zurich.ibm.com>; linux-rdma@vger.kernel.org
> Cc: jgg@ziepe.ca; leon@kernel.org; ionut_n2001@yahoo.com
> 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@linux.dev>
> >> Sent: Tuesday, January 23, 2024 3:43 AM
> >> To: Bernard Metzler <BMT@zurich.ibm.com>; linux-rdma@vger.kernel.org
> >> Cc: jgg@ziepe.ca; leon@kernel.org; ionut_n2001@yahoo.com
> >> 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.

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

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

[Guoqing Jiang]

On 1/26/24 01:27, Bernard Metzler wrote:
>
>> -----Original Message-----
>> From: Guoqing Jiang <guoqing.jiang@linux.dev>
>> Sent: Thursday, January 25, 2024 1:15 AM
>> To: Bernard Metzler <BMT@zurich.ibm.com>; linux-rdma@vger.kernel.org
>> Cc: jgg@ziepe.ca; leon@kernel.org; ionut_n2001@yahoo.com
>> 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@linux.dev>
>>>> Sent: Tuesday, January 23, 2024 3:43 AM
>>>> To: Bernard Metzler <BMT@zurich.ibm.com>; linux-rdma@vger.kernel.org
>>>> Cc: jgg@ziepe.ca; leon@kernel.org; ionut_n2001@yahoo.com
>>>> 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?

The warning still appears with KASAN enabled from my side.

> I am sure we do not want to optimize the driver for building with KASAN
> debug options enabled.

Agreed.

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

I am not sure about it after check the relevant bugzilla, maybe the reporter
can verify it.

Thanks,
Guoqing

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

[Leon Romanovsky]

On Thu, Jan 25, 2024 at 05:27:52PM +0000, Bernard Metzler wrote:
> 
> 
> > -----Original Message-----
> > From: Guoqing Jiang <guoqing.jiang@linux.dev>
> > Sent: Thursday, January 25, 2024 1:15 AM
> > To: Bernard Metzler <BMT@zurich.ibm.com>; linux-rdma@vger.kernel.org
> > Cc: jgg@ziepe.ca; leon@kernel.org; ionut_n2001@yahoo.com
> > 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@linux.dev>
> > >> Sent: Tuesday, January 23, 2024 3:43 AM
> > >> To: Bernard Metzler <BMT@zurich.ibm.com>; linux-rdma@vger.kernel.org
> > >> Cc: jgg@ziepe.ca; leon@kernel.org; ionut_n2001@yahoo.com
> > >> 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.

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
> 

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

[Bernard Metzler]

> -----Original Message-----
> From: Leon Romanovsky <leon@kernel.org>
> Sent: Friday, January 26, 2024 12:06 PM
> To: Bernard Metzler <BMT@zurich.ibm.com>
> Cc: Guoqing Jiang <guoqing.jiang@linux.dev>; linux-rdma@vger.kernel.org;
> jgg@ziepe.ca; ionut_n2001@yahoo.com
> 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@linux.dev>
> > > Sent: Thursday, January 25, 2024 1:15 AM
> > > To: Bernard Metzler <BMT@zurich.ibm.com>; linux-rdma@vger.kernel.org
> > > Cc: jgg@ziepe.ca; leon@kernel.org; ionut_n2001@yahoo.com
> > > 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@linux.dev>
> > > >> Sent: Tuesday, January 23, 2024 3:43 AM
> > > >> To: Bernard Metzler <BMT@zurich.ibm.com>; linux-rdma@vger.kernel.org
> > > >> Cc: jgg@ziepe.ca; leon@kernel.org; ionut_n2001@yahoo.com
> > > >> 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
> >

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

[Leon Romanovsky]

On Thu, Feb 08, 2024 at 11:32:37AM +0000, Bernard Metzler wrote:
> 
> 
> > -----Original Message-----
> > From: Leon Romanovsky <leon@kernel.org>
> > Sent: Friday, January 26, 2024 12:06 PM
> > To: Bernard Metzler <BMT@zurich.ibm.com>
> > Cc: Guoqing Jiang <guoqing.jiang@linux.dev>; linux-rdma@vger.kernel.org;
> > jgg@ziepe.ca; ionut_n2001@yahoo.com
> > 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@linux.dev>
> > > > Sent: Thursday, January 25, 2024 1:15 AM
> > > > To: Bernard Metzler <BMT@zurich.ibm.com>; linux-rdma@vger.kernel.org
> > > > Cc: jgg@ziepe.ca; leon@kernel.org; ionut_n2001@yahoo.com
> > > > 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@linux.dev>
> > > > >> Sent: Tuesday, January 23, 2024 3:43 AM
> > > > >> To: Bernard Metzler <BMT@zurich.ibm.com>; linux-rdma@vger.kernel.org
> > > > >> Cc: jgg@ziepe.ca; leon@kernel.org; ionut_n2001@yahoo.com
> > > > >> 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? ;)

Yes, report them to ML and someone will fix them :).

Thanks

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