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