diff mbox

[PATCHv4,1/2] IB/rxe: remove unnecessary skb_clone

Message ID 1519211457-16285-1-git-send-email-yanjun.zhu@oracle.com (mailing list archive)
State Superseded
Headers show

Commit Message

Zhu Yanjun Feb. 21, 2018, 11:10 a.m. UTC
In send_atomic_ack function, it is not necessary to make a
skb_clone. To gain better performance (high throughput and
low latency), this skb_clone is removed.

The following tests are made.

 server                       client
---------                    ---------
|1.1.1.1|<----rxe-channel--->|1.1.1.2|
---------                    ---------

On server: rping -s -a 1.1.1.1 -v -C 1000 -S 512
On client: rping -c -a 1.1.1.1 -v -C 1000 -S 512

The kernel config CONFIG_DEBUG_KMEMLEAK is enabled on both server
and client.

This test runs for several hours. There is no memory leak and the whole
system can work well.

Based on the above network, the following tests are made.

Server: ibv_rc_pingpong -d rxe0 -g 1
Client: ibv_rc_pingpong -d rxe0 -g 1 1.1.1.1

The test results on Server(10 tests are made).
Before:
Throughput is 137.07 Mbit/sec
Latency is 517.76 usec/iter

After:
Throughput is 148.85 Mbit/sec
Latency is 476.64 usec/iter

The throughput is enhanced and the latency is reduced.

CC: Srinivas Eeda <srinivas.eeda@oracle.com>
CC: Junxiao Bi <junxiao.bi@oracle.com>
Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
---
V3-->V4: Fix typo errors in short logs.
V2-->V3: Fix typo errors.
V1-->V2: 10 tests are made. From throughput and latency, the performance is better.
---
 drivers/infiniband/sw/rxe/rxe_resp.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

Comments

Leon Romanovsky Feb. 22, 2018, 8:31 a.m. UTC | #1
On Wed, Feb 21, 2018 at 06:10:56AM -0500, Zhu Yanjun wrote:
> In send_atomic_ack function, it is not necessary to make a
> skb_clone. To gain better performance (high throughput and
> low latency), this skb_clone is removed.
>
> The following tests are made.
>
>  server                       client
> ---------                    ---------
> |1.1.1.1|<----rxe-channel--->|1.1.1.2|
> ---------                    ---------
>
> On server: rping -s -a 1.1.1.1 -v -C 1000 -S 512
> On client: rping -c -a 1.1.1.1 -v -C 1000 -S 512
>
> The kernel config CONFIG_DEBUG_KMEMLEAK is enabled on both server
> and client.
>
> This test runs for several hours. There is no memory leak and the whole
> system can work well.
>
> Based on the above network, the following tests are made.
>
> Server: ibv_rc_pingpong -d rxe0 -g 1
> Client: ibv_rc_pingpong -d rxe0 -g 1 1.1.1.1
>
> The test results on Server(10 tests are made).
> Before:
> Throughput is 137.07 Mbit/sec
> Latency is 517.76 usec/iter
>
> After:
> Throughput is 148.85 Mbit/sec
> Latency is 476.64 usec/iter
>
> The throughput is enhanced and the latency is reduced.
>
> CC: Srinivas Eeda <srinivas.eeda@oracle.com>
> CC: Junxiao Bi <junxiao.bi@oracle.com>
> Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
> ---
> V3-->V4: Fix typo errors in short logs.
> V2-->V3: Fix typo errors.
> V1-->V2: 10 tests are made. From throughput and latency, the performance is better.
> ---
>  drivers/infiniband/sw/rxe/rxe_resp.c | 18 +++---------------
>  1 file changed, 3 insertions(+), 15 deletions(-)
>

Hi Zhu,

Can you please don't reply and/or place "In-Reply-To" header while
sending patches? Such patches are easily missed by reviewers.

Thanks
Doug Ledford March 7, 2018, 6:53 p.m. UTC | #2
On Wed, 2018-02-21 at 06:10 -0500, Zhu Yanjun wrote:
> In send_atomic_ack function, it is not necessary to make a
> skb_clone. To gain better performance (high throughput and
> low latency), this skb_clone is removed.
> 
> The following tests are made.
> 
>  server                       client
> ---------                    ---------
> > 1.1.1.1|<----rxe-channel--->|1.1.1.2|
> 
> ---------                    ---------
> 
> On server: rping -s -a 1.1.1.1 -v -C 1000 -S 512
> On client: rping -c -a 1.1.1.1 -v -C 1000 -S 512
> 
> The kernel config CONFIG_DEBUG_KMEMLEAK is enabled on both server
> and client.
> 
> This test runs for several hours. There is no memory leak and the whole
> system can work well.
> 
> Based on the above network, the following tests are made.
> 
> Server: ibv_rc_pingpong -d rxe0 -g 1
> Client: ibv_rc_pingpong -d rxe0 -g 1 1.1.1.1
> 
> The test results on Server(10 tests are made).
> Before:
> Throughput is 137.07 Mbit/sec
> Latency is 517.76 usec/iter
> 
> After:
> Throughput is 148.85 Mbit/sec
> Latency is 476.64 usec/iter
> 
> The throughput is enhanced and the latency is reduced.
> 
> CC: Srinivas Eeda <srinivas.eeda@oracle.com>
> CC: Junxiao Bi <junxiao.bi@oracle.com>
> Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
> ---

Thanks, applied to for-next.
diff mbox

Patch

diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index d37bb9b..2ebb07c 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -969,7 +969,6 @@  static int send_atomic_ack(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
 	int rc = 0;
 	struct rxe_pkt_info ack_pkt;
 	struct sk_buff *skb;
-	struct sk_buff *skb_copy;
 	struct rxe_dev *rxe = to_rdev(qp->ibqp.device);
 	struct resp_res *res;
 
@@ -981,15 +980,6 @@  static int send_atomic_ack(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
 		goto out;
 	}
 
-	skb_copy = skb_clone(skb, GFP_ATOMIC);
-	if (skb_copy)
-		rxe_add_ref(qp); /* for the new SKB */
-	else {
-		pr_warn("Could not clone atomic response\n");
-		rc = -ENOMEM;
-		goto out;
-	}
-
 	res = &qp->resp.resources[qp->resp.res_head];
 	free_rd_atomic_resource(qp, res);
 	rxe_advance_resp_resource(qp);
@@ -998,18 +988,16 @@  static int send_atomic_ack(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
 	memset((unsigned char *)SKB_TO_PKT(skb) + sizeof(ack_pkt), 0,
 	       sizeof(skb->cb) - sizeof(ack_pkt));
 
+	refcount_inc(&skb->users);
 	res->type = RXE_ATOMIC_MASK;
 	res->atomic.skb = skb;
 	res->first_psn = ack_pkt.psn;
 	res->last_psn  = ack_pkt.psn;
 	res->cur_psn   = ack_pkt.psn;
 
-	rc = rxe_xmit_packet(rxe, qp, &ack_pkt, skb_copy);
-	if (rc) {
+	rc = rxe_xmit_packet(rxe, qp, &ack_pkt, skb);
+	if (rc)
 		pr_err_ratelimited("Failed sending ack\n");
-		rxe_drop_ref(qp);
-		kfree_skb(skb_copy);
-	}
 
 out:
 	return rc;