Re: [PATCHv2 1/1] IB/rxe: remove unnecessary skb_clone

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

 





On 2018/2/18 16:12, Yonatan Cohen wrote:
On 2/14/2018 12:36 PM, Yuval Shaia wrote:
       }
-    skb_copy = skb_clone(skb, GFP_ATOMIC);
-    if (skb_copy)
-        rxe_add_ref(qp); /* for the new SKB */
Are you sure we don't need this?
 From my stress tests and performance tests, it will get better performance
to remove skb_clone.

My concern is only with the above ref count.
I agree with yuval in case xmit fails.
    send_atomic_ack()
    {
       rxe_xmit_packet()
       {
          rxe_send()
          {
         rxe_add_ref(pkt->qp)  <--- add qp ref
         err = ip_local_out() <---- fail here
          }

       if (rxe_xmit_packet() fails) {
        rxe_drop_ref(qp) <--- you must deref here
       }
    }
Hi,

Thanks for your review.
I checked the source code. When error occurs in the function ip_local_out/ip6_local_out,
the network stack will call skb's destructor, rxe_skb_tx_dtor.
In this function, rxe_drop_ref is called. So it is not necessary to call  rxe_drop_ref again.

static void rxe_skb_tx_dtor(struct sk_buff *skb)
{
        struct sock *sk = skb->sk;
        struct rxe_qp *qp = sk->sk_user_data;
        int skb_out = atomic_dec_return(&qp->skb_out);

        if (unlikely(qp->need_req_skb &&
                     skb_out < RXE_INFLIGHT_SKBS_PER_QP_LOW))
                rxe_run_task(&qp->req.task, 1);

        rxe_drop_ref(qp);  <----here
}

Best Regards,
Zhu Yanjun

And there is no memory leak.  The whole soft RoCE can work well.
So I think removing this function is a good choice.

Zhu Yanjun

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




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