diff mbox series

RDMA/irdma: Slightly optimize irdma_form_ah_cm_frame()

Message ID 098e3c397be0436f1867899245ecfe656c472110.1675369386.git.christophe.jaillet@wanadoo.fr
State New
Headers
Series RDMA/irdma: Slightly optimize irdma_form_ah_cm_frame() |

Commit Message

Christophe JAILLET Feb. 2, 2023, 8:23 p.m. UTC 
  There is no need to zero 'pktsize' bytes of 'buf', only the header needs
to be cleared, to be safe.
All the other bytes are already written with some memcpy() at the end of
the function.

Doing so also gives the opportunity to the compiler to avoid the memset()
call. It can be inlined now that the length is known as compile time.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Just in case, here is the diff of what is generated by gcc 11.3.0 before
and after the patch.

 .L736:
-# drivers/infiniband/hw/irdma/cm.c:340: 	memset(buf, 0, pktsize);
+# drivers/infiniband/hw/irdma/cm.c:340: 	memset(buf, 0, sizeof(*tcph));
 	call	__sanitizer_cov_trace_pc	#
-	xorl	%esi, %esi	#
-	movzwl	%r13w, %edx	# _194, __fortify_size
-	movq	%rbp, %rdi	# buf,
-	call	memset	#
-	leaq	104(%r12), %rax	#, _259
+	movl	$0, 16(%rbp)	#, MEM <char[1:20]> [(void *)buf_114]
+	leaq	104(%r12), %rax	#, _295
+# drivers/infiniband/hw/irdma/cm.c:342: 	sqbuf->totallen = pktsize;
+	movzwl	%r13w, %r13d	# _192, _192
+# drivers/infiniband/hw/irdma/cm.c:340: 	memset(buf, 0, sizeof(*tcph));
+	movq	$0, 0(%rbp)	#, MEM <char[1:20]> [(void *)buf_114]
+# drivers/infiniband/hw/irdma/cm.c:342: 	sqbuf->totallen = pktsize;
+	movq	%rax, %rdi	# _295,
+# drivers/infiniband/hw/irdma/cm.c:340: 	memset(buf, 0, sizeof(*tcph));
+	movq	$0, 8(%rbp)	#, MEM <char[1:20]> [(void *)buf_114]
+	movq	%rax, 64(%rsp)	# _295, %sfp
 # drivers/infiniband/hw/irdma/cm.c:342: 	sqbuf->totallen = pktsize;
---
 drivers/infiniband/hw/irdma/cm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jason Gunthorpe April 12, 2023, 4:02 p.m. UTC | #1 
On Thu, Feb 02, 2023 at 09:23:24PM +0100, Christophe JAILLET wrote:
> There is no need to zero 'pktsize' bytes of 'buf', only the header needs
> to be cleared, to be safe.
> All the other bytes are already written with some memcpy() at the end of
> the function.
> 
> Doing so also gives the opportunity to the compiler to avoid the memset()
> call. It can be inlined now that the length is known as compile time.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> Just in case, here is the diff of what is generated by gcc 11.3.0 before
> and after the patch.
> 
>  .L736:
> -# drivers/infiniband/hw/irdma/cm.c:340: 	memset(buf, 0, pktsize);
> +# drivers/infiniband/hw/irdma/cm.c:340: 	memset(buf, 0, sizeof(*tcph));
>  	call	__sanitizer_cov_trace_pc	#
> -	xorl	%esi, %esi	#
> -	movzwl	%r13w, %edx	# _194, __fortify_size
> -	movq	%rbp, %rdi	# buf,
> -	call	memset	#
> -	leaq	104(%r12), %rax	#, _259
> +	movl	$0, 16(%rbp)	#, MEM <char[1:20]> [(void *)buf_114]
> +	leaq	104(%r12), %rax	#, _295
> +# drivers/infiniband/hw/irdma/cm.c:342: 	sqbuf->totallen = pktsize;
> +	movzwl	%r13w, %r13d	# _192, _192
> +# drivers/infiniband/hw/irdma/cm.c:340: 	memset(buf, 0, sizeof(*tcph));
> +	movq	$0, 0(%rbp)	#, MEM <char[1:20]> [(void *)buf_114]
> +# drivers/infiniband/hw/irdma/cm.c:342: 	sqbuf->totallen = pktsize;
> +	movq	%rax, %rdi	# _295,
> +# drivers/infiniband/hw/irdma/cm.c:340: 	memset(buf, 0, sizeof(*tcph));
> +	movq	$0, 8(%rbp)	#, MEM <char[1:20]> [(void *)buf_114]
> +	movq	%rax, 64(%rsp)	# _295, %sfp
>  # drivers/infiniband/hw/irdma/cm.c:342: 	sqbuf->totallen = pktsize;
> ---
>  drivers/infiniband/hw/irdma/cm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Shiraz??

Jason
Saleem, Shiraz April 12, 2023, 5:03 p.m. UTC | #2 
> Subject: Re: [PATCH] RDMA/irdma: Slightly optimize
> irdma_form_ah_cm_frame()
> 
> On Thu, Feb 02, 2023 at 09:23:24PM +0100, Christophe JAILLET wrote:
> > There is no need to zero 'pktsize' bytes of 'buf', only the header
> > needs to be cleared, to be safe.
> > All the other bytes are already written with some memcpy() at the end
> > of the function.
> >
> > Doing so also gives the opportunity to the compiler to avoid the
> > memset() call. It can be inlined now that the length is known as compile time.
> >
> > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> > ---
> > Just in case, here is the diff of what is generated by gcc 11.3.0
> > before and after the patch.
> >
> >  .L736:
> > -# drivers/infiniband/hw/irdma/cm.c:340: 	memset(buf, 0, pktsize);
> > +# drivers/infiniband/hw/irdma/cm.c:340: 	memset(buf, 0, sizeof(*tcph));
> >  	call	__sanitizer_cov_trace_pc	#
> > -	xorl	%esi, %esi	#
> > -	movzwl	%r13w, %edx	# _194, __fortify_size
> > -	movq	%rbp, %rdi	# buf,
> > -	call	memset	#
> > -	leaq	104(%r12), %rax	#, _259
> > +	movl	$0, 16(%rbp)	#, MEM <char[1:20]> [(void *)buf_114]
> > +	leaq	104(%r12), %rax	#, _295
> > +# drivers/infiniband/hw/irdma/cm.c:342: 	sqbuf->totallen = pktsize;
> > +	movzwl	%r13w, %r13d	# _192, _192
> > +# drivers/infiniband/hw/irdma/cm.c:340: 	memset(buf, 0, sizeof(*tcph));
> > +	movq	$0, 0(%rbp)	#, MEM <char[1:20]> [(void *)buf_114]
> > +# drivers/infiniband/hw/irdma/cm.c:342: 	sqbuf->totallen = pktsize;
> > +	movq	%rax, %rdi	# _295,
> > +# drivers/infiniband/hw/irdma/cm.c:340: 	memset(buf, 0, sizeof(*tcph));
> > +	movq	$0, 8(%rbp)	#, MEM <char[1:20]> [(void *)buf_114]
> > +	movq	%rax, 64(%rsp)	# _295, %sfp
> >  # drivers/infiniband/hw/irdma/cm.c:342: 	sqbuf->totallen = pktsize;
> > ---
> >  drivers/infiniband/hw/irdma/cm.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Shiraz??
> 

Sorry for the delay as I didn't see this earlier.

Reviewed-by: Shiraz Saleem <shiraz.saleem@intel.com>
Jason Gunthorpe April 12, 2023, 11:06 p.m. UTC | #3 
On Thu, Feb 02, 2023 at 09:23:24PM +0100, Christophe JAILLET wrote:
> There is no need to zero 'pktsize' bytes of 'buf', only the header needs
> to be cleared, to be safe.
> All the other bytes are already written with some memcpy() at the end of
> the function.
> 
> Doing so also gives the opportunity to the compiler to avoid the memset()
> call. It can be inlined now that the length is known as compile time.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> Reviewed-by: Shiraz Saleem <shiraz.saleem@intel.com>
> ---
> Just in case, here is the diff of what is generated by gcc 11.3.0 before
> and after the patch.
> 
>  .L736:
> -# drivers/infiniband/hw/irdma/cm.c:340: 	memset(buf, 0, pktsize);
> +# drivers/infiniband/hw/irdma/cm.c:340: 	memset(buf, 0, sizeof(*tcph));
>  	call	__sanitizer_cov_trace_pc	#
> -	xorl	%esi, %esi	#
> -	movzwl	%r13w, %edx	# _194, __fortify_size
> -	movq	%rbp, %rdi	# buf,
> -	call	memset	#
> -	leaq	104(%r12), %rax	#, _259
> +	movl	$0, 16(%rbp)	#, MEM <char[1:20]> [(void *)buf_114]
> +	leaq	104(%r12), %rax	#, _295
> +# drivers/infiniband/hw/irdma/cm.c:342: 	sqbuf->totallen = pktsize;
> +	movzwl	%r13w, %r13d	# _192, _192
> +# drivers/infiniband/hw/irdma/cm.c:340: 	memset(buf, 0, sizeof(*tcph));
> +	movq	$0, 0(%rbp)	#, MEM <char[1:20]> [(void *)buf_114]
> +# drivers/infiniband/hw/irdma/cm.c:342: 	sqbuf->totallen = pktsize;
> +	movq	%rax, %rdi	# _295,
> +# drivers/infiniband/hw/irdma/cm.c:340: 	memset(buf, 0, sizeof(*tcph));
> +	movq	$0, 8(%rbp)	#, MEM <char[1:20]> [(void *)buf_114]
> +	movq	%rax, 64(%rsp)	# _295, %sfp
>  # drivers/infiniband/hw/irdma/cm.c:342: 	sqbuf->totallen = pktsize;
> ---
>  drivers/infiniband/hw/irdma/cm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to for-next

Thanks,
Jason
diff mbox series

Patch

diff --git a/drivers/infiniband/hw/irdma/cm.c b/drivers/infiniband/hw/irdma/cm.c
index 195aa9ea18b6..48c2a303e9ec 100644
--- a/drivers/infiniband/hw/irdma/cm.c
+++ b/drivers/infiniband/hw/irdma/cm.c
@@ -337,7 +337,7 @@  static struct irdma_puda_buf *irdma_form_ah_cm_frame(struct irdma_cm_node *cm_no
 
 	pktsize = sizeof(*tcph) + opts_len + hdr_len + pd_len;
 
-	memset(buf, 0, pktsize);
+	memset(buf, 0, sizeof(*tcph));
 
 	sqbuf->totallen = pktsize;
 	sqbuf->tcphlen = sizeof(*tcph) + opts_len;