Re: [PATCH RFC v4 net-next 1/5] virtio_net: enable tx interrupt

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

 





On Fri, Dec 19, 2014 at 3:32 PM, Qin Chuanyu <qinchuanyu@xxxxxxxxxx> wrote:
On 2014/12/1 18:17, Jason Wang wrote:
On newer hosts that support delayed tx interrupts,
we probably don't have much to gain from orphaning
packets early.

Note: this might degrade performance for
hosts without event idx support.
Should be addressed by the next patch.

Cc: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
Cc: Michael S. Tsirkin <mst@xxxxxxxxxx>
Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
Signed-off-by: Jason Wang <jasowang@xxxxxxxxxx>
---
drivers/net/virtio_net.c | 132 +++++++++++++++++++++++++++++++----------------
  1 file changed, 88 insertions(+), 44 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index ec2a8b4..f68114e 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
  static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
  {
  	struct skb_vnet_hdr *hdr;
@@ -912,7 +951,9 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
  		sg_set_buf(sq->sg, hdr, hdr_len);
  		num_sg = skb_to_sgvec(skb, sq->sg + 1, 0, skb->len) + 1;
  	}
- return virtqueue_add_outbuf(sq->vq, sq->sg, num_sg, skb, GFP_ATOMIC);
+
+	return virtqueue_add_outbuf(sq->vq, sq->sg, num_sg, skb,
+				    GFP_ATOMIC);
  }

static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev) @@ -924,8 +965,7 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
  	struct netdev_queue *txq = netdev_get_tx_queue(dev, qnum);
  	bool kick = !skb->xmit_more;

-	/* Free up any pending old buffers before queueing new ones. */
-	free_old_xmit_skbs(sq);

I think there is no need to remove free_old_xmit_skbs here.
you could add free_old_xmit_skbs in tx_irq's napi func.
also could do this in start_xmit if you handle the race well.

Note, free_old_xmit_skbs() has already called in tx napi.
It was a must after tx interrupt was enabled.


I have done the same thing in ixgbe driver(free skb in ndo_start_xmit and both in tx_irq's poll func), and it seems work well:)

Any performance numbers on this change?
I suspect it reduce the effects of interrupt coalescing.

I think there would be no so much interrupts in this way, also tx interrupt coalesce is not needed.

Tests (multiple sessions of TCP_RR) does not support this.
Calling free_old_xmit_skbs() in fact damage the performance.
Any justification that you think it may reduce the interrupts?

Thanks


+	virtqueue_disable_cb(sq->vq);

  	/* Try to transmit */
  	err = xmit_skb(sq, skb);
@@ -941,27 +981,19 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
  		return NETDEV_TX_OK;
  	}

-	/* Don't wait up for transmitted skbs to be freed. */
-	skb_orphan(skb);
-	nf_reset(skb);
-
  	/* Apparently nice girls don't return TX_BUSY; stop the queue
  	 * before it gets out of hand.  Naturally, this wastes entries. */
-	if (sq->vq->num_free < 2+MAX_SKB_FRAGS) {
+	if (sq->vq->num_free < 2+MAX_SKB_FRAGS)
  		netif_stop_subqueue(dev, qnum);
-		if (unlikely(!virtqueue_enable_cb_delayed(sq->vq))) {
-			/* More just got used, free them then recheck. */
-			free_old_xmit_skbs(sq);
-			if (sq->vq->num_free >= 2+MAX_SKB_FRAGS) {
-				netif_start_subqueue(dev, qnum);
-				virtqueue_disable_cb(sq->vq);
-			}
-		}
-	}

  	if (kick || netif_xmit_stopped(txq))
  		virtqueue_kick(sq->vq);

+	if (unlikely(!virtqueue_enable_cb_delayed(sq->vq))) {
+		virtqueue_disable_cb(sq->vq);
+		napi_schedule(&sq->napi);
+	}
+
  	return NETDEV_TX_OK;
  }

@@ -1138,8 +1170,10 @@ static int virtnet_close(struct net_device *dev)
  	/* Make sure refill_work doesn't re-enable napi! */
  	cancel_delayed_work_sync(&vi->refill);

-	for (i = 0; i < vi->max_queue_pairs; i++)
+	for (i = 0; i < vi->max_queue_pairs; i++) {
  		napi_disable(&vi->rq[i].napi);
+		napi_disable(&vi->sq[i].napi);
+	}

  	return 0;
  }
@@ -1452,8 +1486,10 @@ static void virtnet_free_queues(struct virtnet_info *vi)
  {
  	int i;

-	for (i = 0; i < vi->max_queue_pairs; i++)
+	for (i = 0; i < vi->max_queue_pairs; i++) {
  		netif_napi_del(&vi->rq[i].napi);
+		netif_napi_del(&vi->sq[i].napi);
+	}

  	kfree(vi->rq);
  	kfree(vi->sq);


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

_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization




[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux