Re: [PATCHv3 1/1] IB/rxe: optimize the recv process

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

 





On 2018/3/20 17:56, Yuval Shaia wrote:
Hi Zhu,
Sorry but i have few more questions.

Yuval

On Tue, Mar 20, 2018 at 04:23:36AM -0400, Zhu Yanjun wrote:
In mcast recv process, the function skb_clone is used. In fact,
the refcount can be increased to replace cloning a new skb since
the original skb will not be modified before it is freed.

This can make the performance better and save the memory.
"can"??
If you have numbers suggesting to add them to commit message.

Also, suggesting to use this header instead:

IB/rxe: optimize mcast recv process

OK. I will use this "IB/rxe: optimize mcast recv process".:-)
I will make a new patch right now.

CC: Srinivas Eeda <srinivas.eeda@xxxxxxxxxx>
CC: Junxiao Bi <junxiao.bi@xxxxxxxxxx>
Signed-off-by: Zhu Yanjun <yanjun.zhu@xxxxxxxxxx>
---
V2->v3: Following Yuval's advice, the temp variable is removed.
V1->V2: Following Yuval's advice, skb test is removed.
---
  drivers/infiniband/sw/rxe/rxe_recv.c | 14 ++++----------
  1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c
index 4c3f899..04beecd 100644
--- a/drivers/infiniband/sw/rxe/rxe_recv.c
+++ b/drivers/infiniband/sw/rxe/rxe_recv.c
@@ -276,7 +276,6 @@ static void rxe_rcv_mcast_pkt(struct rxe_dev *rxe, struct sk_buff *skb)
  {
  	struct rxe_pkt_info *pkt = SKB_TO_PKT(skb);
  	struct rxe_mc_grp *mcg;
-	struct sk_buff *skb_copy;
  	struct rxe_mc_elem *mce;
  	struct rxe_qp *qp;
  	union ib_gid dgid;
@@ -309,18 +308,14 @@ static void rxe_rcv_mcast_pkt(struct rxe_dev *rxe, struct sk_buff *skb)
  			continue;
/* if *not* the last qp in the list
-		 * make a copy of the skb to post to the next qp
+		 * increase the users of the skb then post to the next qp
  		 */
-		skb_copy = (mce->qp_list.next != &mcg->qp_list) ?
-				skb_clone(skb, GFP_ATOMIC) : NULL;
+		if (mce->qp_list.next != &mcg->qp_list)
+			refcount_inc(&skb->users);
Do we need somewhere to take care for refcount_dec? I assume not as
expecting kfree_skb to do so but just wanted to make sure.
Yeah, I agree with you. kfree_skb do the work.

pkt->qp = qp;
  		rxe_add_ref(qp);
  		rxe_rcv_pkt(rxe, pkt, skb);
-
-		skb = skb_copy;
-		if (!skb)
-			break;
  	}
spin_unlock_bh(&mcg->mcg_lock);
@@ -328,8 +323,7 @@ static void rxe_rcv_mcast_pkt(struct rxe_dev *rxe, struct sk_buff *skb)
  	rxe_drop_ref(mcg);	/* drop ref from rxe_pool_get_key. */
err1:
-	if (skb)
-		kfree_skb(skb);
+	kfree_skb(skb);
With or without accepting the comments/suggestions - lgtm.

Reviewed-by: Yuval Shaia <yuval.shaia@xxxxxxxxxx>
Cool! Thanks you very much.:-D

Zhu Yanjun

  }
static int rxe_match_dgid(struct rxe_dev *rxe, struct sk_buff *skb)
--
2.7.4


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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