All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Jason Wang <jasowang@redhat.com>
Cc: eric.dumazet@gmail.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org, davem@davemloft.net
Subject: Re: [RFC PATCH net-next 0/6] Always use tx interrupt for virtio-net
Date: Wed, 15 Oct 2014 13:25:53 +0300	[thread overview]
Message-ID: <20141015102553.GF25776@redhat.com> (raw)
In-Reply-To: <1413357930-45302-1-git-send-email-jasowang@redhat.com>

On Wed, Oct 15, 2014 at 03:25:24PM +0800, Jason Wang wrote:
> According to David, proper accounting and queueing (at all levels, not
> just TCP sockets) is more important than trying to skim a bunch of
> cycles by avoiding TX interrupts.

He also mentioned we should find other ways to batch

> Having an event to free the SKB is
> absolutely essential for the stack to operate correctly.
> 
> This series tries to enable tx interrupt for virtio-net. The idea is
> simple: enable tx interrupt and schedule a tx napi to free old xmit
> skbs.
> 
> Several notes:
> - Tx interrupt storm avoidance when queue is about to be full is
>   kept.

But queue is typically *not* full. More important to avoid interrupt
storms in that case IMO.

> Since we may enable callbacks in both ndo_start_xmit() and tx
>   napi, patch 1 adds a check to make sure used event never go
>   back. This will let the napi not enable the callbacks wrongly after
>   delayed callbacks was used.

So why not just use delayed callbacks?


> - For bulk dequeuing, there's no need to enable tx interrupt for each
>   packet. The last patch only enable tx interrupt for the final skb in
>   the chain through xmit_more and a new helper to publish current avail
>   idx as used event.
> 
> This series fixes several issues of original rfc pointed out by Michael.

Could you list the issues, for ease of review?


> 
> Smoking test is done, and will do complete netperf test. Send the
> seires for early review.
> 
> Thanks
> 
> Jason Wang (6):
>   virtio: make sure used event never go backwards
>   virtio: introduce virtio_enable_cb_avail()
>   virtio-net: small optimization on free_old_xmit_skbs()
>   virtio-net: return the number of packets sent in free_old_xmit_skbs()
>   virtio-net: enable tx interrupt
>   virtio-net: enable tx interrupt only for the final skb in the chain
> 
>  drivers/net/virtio_net.c     |  125 ++++++++++++++++++++++++++++++------------
>  drivers/virtio/virtio_ring.c |   25 ++++++++-
>  include/linux/virtio.h       |    2 +
>  3 files changed, 115 insertions(+), 37 deletions(-)

WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Jason Wang <jasowang@redhat.com>
Cc: rusty@rustcorp.com.au, virtualization@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	davem@davemloft.net, eric.dumazet@gmail.com
Subject: Re: [RFC PATCH net-next 0/6] Always use tx interrupt for virtio-net
Date: Wed, 15 Oct 2014 13:25:53 +0300	[thread overview]
Message-ID: <20141015102553.GF25776@redhat.com> (raw)
In-Reply-To: <1413357930-45302-1-git-send-email-jasowang@redhat.com>

On Wed, Oct 15, 2014 at 03:25:24PM +0800, Jason Wang wrote:
> According to David, proper accounting and queueing (at all levels, not
> just TCP sockets) is more important than trying to skim a bunch of
> cycles by avoiding TX interrupts.

He also mentioned we should find other ways to batch

> Having an event to free the SKB is
> absolutely essential for the stack to operate correctly.
> 
> This series tries to enable tx interrupt for virtio-net. The idea is
> simple: enable tx interrupt and schedule a tx napi to free old xmit
> skbs.
> 
> Several notes:
> - Tx interrupt storm avoidance when queue is about to be full is
>   kept.

But queue is typically *not* full. More important to avoid interrupt
storms in that case IMO.

> Since we may enable callbacks in both ndo_start_xmit() and tx
>   napi, patch 1 adds a check to make sure used event never go
>   back. This will let the napi not enable the callbacks wrongly after
>   delayed callbacks was used.

So why not just use delayed callbacks?


> - For bulk dequeuing, there's no need to enable tx interrupt for each
>   packet. The last patch only enable tx interrupt for the final skb in
>   the chain through xmit_more and a new helper to publish current avail
>   idx as used event.
> 
> This series fixes several issues of original rfc pointed out by Michael.

Could you list the issues, for ease of review?


> 
> Smoking test is done, and will do complete netperf test. Send the
> seires for early review.
> 
> Thanks
> 
> Jason Wang (6):
>   virtio: make sure used event never go backwards
>   virtio: introduce virtio_enable_cb_avail()
>   virtio-net: small optimization on free_old_xmit_skbs()
>   virtio-net: return the number of packets sent in free_old_xmit_skbs()
>   virtio-net: enable tx interrupt
>   virtio-net: enable tx interrupt only for the final skb in the chain
> 
>  drivers/net/virtio_net.c     |  125 ++++++++++++++++++++++++++++++------------
>  drivers/virtio/virtio_ring.c |   25 ++++++++-
>  include/linux/virtio.h       |    2 +
>  3 files changed, 115 insertions(+), 37 deletions(-)

  parent reply	other threads:[~2014-10-15 10:25 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-15  7:25 [RFC PATCH net-next 0/6] Always use tx interrupt for virtio-net Jason Wang
2014-10-15  7:25 ` Jason Wang
2014-10-15  7:25 ` [RFC PATCH net-next 1/6] virtio: make sure used event never go backwards Jason Wang
2014-10-15  7:25   ` Jason Wang
2014-10-15  9:34   ` Michael S. Tsirkin
2014-10-15  9:34     ` Michael S. Tsirkin
2014-10-15 10:13     ` Jason Wang
2014-10-15 10:13       ` Jason Wang
2014-10-15 10:32       ` Michael S. Tsirkin
2014-10-15 10:32         ` Michael S. Tsirkin
2014-10-15 10:44         ` Jason Wang
2014-10-15 10:44           ` Jason Wang
2014-10-15 11:38           ` Michael S. Tsirkin
2014-10-15 11:38             ` Michael S. Tsirkin
2014-10-17  5:04             ` Jason Wang
2014-10-17  5:04               ` Jason Wang
2014-10-15  7:25 ` [RFC PATCH net-next 2/6] virtio: introduce virtio_enable_cb_avail() Jason Wang
2014-10-15  7:25   ` Jason Wang
2014-10-15  9:28   ` Michael S. Tsirkin
2014-10-15  9:28     ` Michael S. Tsirkin
2014-10-15 10:19     ` Jason Wang
2014-10-15 10:19       ` Jason Wang
2014-10-15 10:41       ` Michael S. Tsirkin
2014-10-15 10:41         ` Michael S. Tsirkin
2014-10-15 10:58         ` Jason Wang
2014-10-15 10:58           ` Jason Wang
2014-10-15 11:43           ` Michael S. Tsirkin
2014-10-15 11:43             ` Michael S. Tsirkin
2014-10-15  7:25 ` [RFC PATCH net-next 3/6] virtio-net: small optimization on free_old_xmit_skbs() Jason Wang
2014-10-15  7:25   ` Jason Wang
2014-10-15  9:36   ` Eric Dumazet
2014-10-15  9:36     ` Eric Dumazet
2014-10-15  9:37   ` Michael S. Tsirkin
2014-10-15  9:37     ` Michael S. Tsirkin
2014-10-15  9:49     ` David Laight
2014-10-15  9:49       ` David Laight
2014-10-15 10:48       ` Michael S. Tsirkin
2014-10-15 10:48         ` Michael S. Tsirkin
2014-10-15 10:51         ` David Laight
2014-10-15 10:51           ` David Laight
2014-10-15 12:00           ` Michael S. Tsirkin
2014-10-15 12:00             ` Michael S. Tsirkin
2014-10-15  7:25 ` [RFC PATCH net-next 4/6] virtio-net: return the number of packets sent in free_old_xmit_skbs() Jason Wang
2014-10-15  7:25 ` Jason Wang
2014-10-15  7:25 ` [RFC PATCH net-next 5/6] virtio-net: enable tx interrupt Jason Wang
2014-10-15  7:25 ` Jason Wang
2014-10-15  9:37   ` Eric Dumazet
2014-10-15  9:37     ` Eric Dumazet
2014-10-15 10:21     ` Jason Wang
2014-10-15 10:21       ` Jason Wang
2014-10-15 10:18   ` Michael S. Tsirkin
2014-10-15 10:18     ` Michael S. Tsirkin
2014-10-15 10:25     ` Jason Wang
2014-10-15 10:25       ` Jason Wang
2014-10-15 10:43       ` Michael S. Tsirkin
2014-10-15 10:43         ` Michael S. Tsirkin
2014-10-15 11:00         ` Jason Wang
2014-10-15 11:00           ` Jason Wang
2014-10-15  7:25 ` [RFC PATCH net-next 6/6] virtio-net: enable tx interrupt only for the final skb in the chain Jason Wang
2014-10-15  7:25 ` Jason Wang
2014-10-15 10:22   ` Michael S. Tsirkin
2014-10-15 10:22     ` Michael S. Tsirkin
2014-10-15 10:31     ` Jason Wang
2014-10-15 10:31       ` Jason Wang
2014-10-15 10:46       ` Michael S. Tsirkin
2014-10-15 10:46         ` Michael S. Tsirkin
2014-10-15 10:25 ` Michael S. Tsirkin [this message]
2014-10-15 10:25   ` [RFC PATCH net-next 0/6] Always use tx interrupt for virtio-net Michael S. Tsirkin
2014-10-15 11:14   ` Jason Wang
2014-10-15 11:14     ` Jason Wang
2014-10-15 11:58     ` Michael S. Tsirkin
2014-10-15 11:58       ` Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141015102553.GF25776@redhat.com \
    --to=mst@redhat.com \
    --cc=davem@davemloft.net \
    --cc=eric.dumazet@gmail.com \
    --cc=jasowang@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=virtualization@lists.linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.