Re: [RFC PATCH net-next 0/6] Always use tx interrupt for virtio-net
On Wed, Oct 15, 2014 at 07:14:14PM +0800, Jason Wang wrote: > On 10/15/2014 06:25 PM, Michael S. Tsirkin wrote: > > 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 > > > > Right. > >> 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. > > Yes. > >> 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? > > This means the tx interrupt are coalesced in a somewhat adaptive way. > Need benchmark to see its effect. I think it's a minimal change, and does not need new APIs. If that's not optimal, we can do smarter things on top. > > > >> - 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? > > Probably just one: > > - Move the virtqueue_disable_cb() from skb_xmit_done() into > virtnet_poll_tx() under tx lock. I think I did this already, I'll recheck. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH net-next 0/6] Always use tx interrupt for virtio-net
On 10/15/2014 06:25 PM, Michael S. Tsirkin wrote: > 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 > Right. >> 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. Yes. >> 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? This means the tx interrupt are coalesced in a somewhat adaptive way. Need benchmark to see its effect. > >> - 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? Probably just one: - Move the virtqueue_disable_cb() from skb_xmit_done() into virtnet_poll_tx() under tx lock. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH net-next 0/6] Always use tx interrupt for virtio-net
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(-) ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[RFC PATCH net-next 0/6] Always use tx interrupt for virtio-net
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. 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. 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. - 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. 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(-) ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
