[PATCH net] net/smc: Clear memory when release and reuse buffer

Currently, buffers are clear when smc create connections and reuse
buffer. It will slow down the speed of establishing new connection. In
most cases, the applications hope to establish connections as quickly as
possible.

This patch moves memset() from connection creation path to release and
buffer unuse path, this trades off between speed of establishing and
release.

Test environments:
- CPU Intel Xeon Platinum 8 core, mem 32 GiB, nic Mellanox CX4
- socket sndbuf / rcvbuf: 16384 / 131072 bytes
- w/o first round, 5 rounds, avg, 100 conns batch per round
- smc_buf_create() use bpftrace kprobe, introduces extra latency

Latency benchmarks for smc_buf_create():
  w/o patch : 19040.0 ns
  w/  patch :  1932.6 ns
  ratio :        10.2% (-89.8%)

Latency benchmarks for socket create and connect:
  w/o patch :   143.3 us
  w/  patch :   102.2 us
  ratio :        71.3% (-28.7%)

The latency of establishing connections is reduced by 28.7%.

Signed-off-by: Tony Lu <tonylu@linux.alibaba.com>
Reviewed-by: Wen Gu <guwen@linux.alibaba.com>
---
 net/smc/smc_core.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index bb52c8b5f148..5f0bd547907d 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c

@@ -1102,18 +1102,24 @@ static void smcr_buf_unuse(struct smc_buf_desc *rmb_desc,
 		smc_buf_free(lgr, true, rmb_desc);
 	} else {
 		rmb_desc->used = 0;
+		memset(rmb_desc->cpu_addr, 0, rmb_desc->len);
 	}
 }
 
 static void smc_buf_unuse(struct smc_connection *conn,
 			  struct smc_link_group *lgr)
 {
-	if (conn->sndbuf_desc)
+	if (conn->sndbuf_desc) {
 		conn->sndbuf_desc->used = 0;
-	if (conn->rmb_desc && lgr->is_smcd)
+		memset(conn->sndbuf_desc->cpu_addr, 0, conn->sndbuf_desc->len);
+	}
+	if (conn->rmb_desc && lgr->is_smcd) {
 		conn->rmb_desc->used = 0;
-	else if (conn->rmb_desc)
+		memset(conn->rmb_desc->cpu_addr, 0, conn->rmb_desc->len +
+		       sizeof(struct smcd_cdc_msg));
+	} else if (conn->rmb_desc) {
 		smcr_buf_unuse(conn->rmb_desc, lgr);
+	}
 }
 
 /* remove a finished connection from its link group */

@@ -2149,7 +2155,6 @@ static int __smc_buf_create(struct smc_sock *smc, bool is_smcd, bool is_rmb)
 		if (buf_desc) {
 			SMC_STAT_RMB_SIZE(smc, is_smcd, is_rmb, bufsize);
 			SMC_STAT_BUF_REUSE(smc, is_smcd, is_rmb);
-			memset(buf_desc->cpu_addr, 0, bufsize);
 			break; /* found reusable slot */
 		}
 

-- 
2.32.0.3.g01195cf9f

On Thu, 25 Nov 2021 20:28:59 +0800 Tony Lu wrote:

Currently, buffers are clear when smc create connections and reuse
buffer. It will slow down the speed of establishing new connection. In
most cases, the applications hope to establish connections as quickly as
possible.

This patch moves memset() from connection creation path to release and
buffer unuse path, this trades off between speed of establishing and
release.

Test environments:
- CPU Intel Xeon Platinum 8 core, mem 32 GiB, nic Mellanox CX4
- socket sndbuf / rcvbuf: 16384 / 131072 bytes
- w/o first round, 5 rounds, avg, 100 conns batch per round
- smc_buf_create() use bpftrace kprobe, introduces extra latency

Latency benchmarks for smc_buf_create():
  w/o patch : 19040.0 ns
  w/  patch :  1932.6 ns
  ratio :        10.2% (-89.8%)

Latency benchmarks for socket create and connect:
  w/o patch :   143.3 us
  w/  patch :   102.2 us
  ratio :        71.3% (-28.7%)

The latency of establishing connections is reduced by 28.7%.

Signed-off-by: Tony Lu <tonylu@linux.alibaba.com>
Reviewed-by: Wen Gu <guwen@linux.alibaba.com>

The tag in the subject seems incorrect, we tag things as [PATCH net] 
if they are fixes, and as [PATCH net-next] if they are new features,
code refactoring or performance improvements.

Is this a fix for a regression? In which case we need a Fixes tag to
indicate where it was introduced. Otherwise it needs to be tagged as
[PATCH net-next].

I'm assuming Karsten will take it via his tree, otherwise you'll need
to repost.

On 26/11/2021 20:28, Jakub Kicinski wrote:

The tag in the subject seems incorrect, we tag things as [PATCH net] 
if they are fixes, and as [PATCH net-next] if they are new features,
code refactoring or performance improvements.

Is this a fix for a regression? In which case we need a Fixes tag to
indicate where it was introduced. Otherwise it needs to be tagged as
[PATCH net-next].

I'm assuming Karsten will take it via his tree, otherwise you'll need
to repost.

We are testing this change atm and will submit it via our tree.
Very nice change, I like it!

On Fri, Nov 26, 2021 at 11:28:55AM -0800, Jakub Kicinski wrote:

On Thu, 25 Nov 2021 20:28:59 +0800 Tony Lu wrote:

quoted

Currently, buffers are clear when smc create connections and reuse
buffer. It will slow down the speed of establishing new connection. In
most cases, the applications hope to establish connections as quickly as
possible.

This patch moves memset() from connection creation path to release and
buffer unuse path, this trades off between speed of establishing and
release.

Test environments:
- CPU Intel Xeon Platinum 8 core, mem 32 GiB, nic Mellanox CX4
- socket sndbuf / rcvbuf: 16384 / 131072 bytes
- w/o first round, 5 rounds, avg, 100 conns batch per round
- smc_buf_create() use bpftrace kprobe, introduces extra latency

Latency benchmarks for smc_buf_create():
  w/o patch : 19040.0 ns
  w/  patch :  1932.6 ns
  ratio :        10.2% (-89.8%)

Latency benchmarks for socket create and connect:
  w/o patch :   143.3 us
  w/  patch :   102.2 us
  ratio :        71.3% (-28.7%)

The latency of establishing connections is reduced by 28.7%.

Signed-off-by: Tony Lu <tonylu@linux.alibaba.com>
Reviewed-by: Wen Gu <guwen@linux.alibaba.com>

The tag in the subject seems incorrect, we tag things as [PATCH net] 
if they are fixes, and as [PATCH net-next] if they are new features,
code refactoring or performance improvements.

Is this a fix for a regression? In which case we need a Fixes tag to
indicate where it was introduced. Otherwise it needs to be tagged as
[PATCH net-next].

I'm assuming Karsten will take it via his tree, otherwise you'll need
to repost.

Sorry for the unclear tag. This patch introduces a performance
improvement. It should be with net-next.

I will fix it and send v2. Thank you.

Thanks,
Tony Lu

On 30/11/2021 03:52, Tony Lu wrote:

Sorry for the unclear tag. This patch introduces a performance
improvement. It should be with net-next.

I will fix it and send v2. Thank you.

Will you now send a v2 to net-next, or should I pick your v1 and 
submit it via our tree?

On Thu, Dec 02, 2021 at 03:23:07PM +0100, Karsten Graul wrote:

On 30/11/2021 03:52, Tony Lu wrote:

quoted

Sorry for the unclear tag. This patch introduces a performance
improvement. It should be with net-next.

I will fix it and send v2. Thank you.

Will you now send a v2 to net-next, or should I pick your v1 and 
submit it via our tree?

Sorry about my unclear reply in the previous mail. It's nice to pick v1
to your tree. If v2 is needed, I will send it out soon. Thank you.

Thanks,
Tony Lu

On Fri, Nov 26, 2021 at 11:28:55AM -0800, Jakub Kicinski wrote:

On Thu, 25 Nov 2021 20:28:59 +0800 Tony Lu wrote:

quoted

Currently, buffers are clear when smc create connections and reuse
buffer. It will slow down the speed of establishing new connection. In
most cases, the applications hope to establish connections as quickly as
possible.

This patch moves memset() from connection creation path to release and
buffer unuse path, this trades off between speed of establishing and
release.

Test environments:
- CPU Intel Xeon Platinum 8 core, mem 32 GiB, nic Mellanox CX4
- socket sndbuf / rcvbuf: 16384 / 131072 bytes
- w/o first round, 5 rounds, avg, 100 conns batch per round
- smc_buf_create() use bpftrace kprobe, introduces extra latency

Latency benchmarks for smc_buf_create():
  w/o patch : 19040.0 ns
  w/  patch :  1932.6 ns
  ratio :        10.2% (-89.8%)

Latency benchmarks for socket create and connect:
  w/o patch :   143.3 us
  w/  patch :   102.2 us
  ratio :        71.3% (-28.7%)

The latency of establishing connections is reduced by 28.7%.

Signed-off-by: Tony Lu <tonylu@linux.alibaba.com>
Reviewed-by: Wen Gu <guwen@linux.alibaba.com>

The tag in the subject seems incorrect, we tag things as [PATCH net] 
if they are fixes, and as [PATCH net-next] if they are new features,
code refactoring or performance improvements.

Is this a fix for a regression? In which case we need a Fixes tag to
indicate where it was introduced. Otherwise it needs to be tagged as
[PATCH net-next].

I'm assuming Karsten will take it via his tree, otherwise you'll need
to repost.

Sorry to miss Graul’s email, he said he would put it in his own tree. I
would send v2 out if he needed. Thank you.

Thanks,
Tony Lu

On 03/12/2021 04:31, Tony Lu wrote:

On Thu, Dec 02, 2021 at 03:23:07PM +0100, Karsten Graul wrote:

quoted

On 30/11/2021 03:52, Tony Lu wrote:

quoted

Sorry for the unclear tag. This patch introduces a performance
improvement. It should be with net-next.

I will fix it and send v2. Thank you.

Will you now send a v2 to net-next, or should I pick your v1 and 
submit it via our tree?

Sorry about my unclear reply in the previous mail. It's nice to pick v1
to your tree. If v2 is needed, I will send it out soon. Thank you.

Thanks,
Tony Lu

Okay, I pick it up now. Thank you.