Re: [PATCH net-next V2 0/8] Packed virtqueue support for vhost

On Mon, Jul 16, 2018 at 05:46:33PM +0800, Jason Wang wrote:

On 2018年07月16日 16:39, Michael S. Tsirkin wrote:

quoted

On Mon, Jul 16, 2018 at 11:28:03AM +0800, Jason Wang wrote:

quoted

Hi all:

This series implements packed virtqueues. The code were tested with
Tiwei's guest driver series at https://patchwork.ozlabs.org/cover/942297/


Pktgen test for both RX and TX does not show obvious difference with
split virtqueues. The main bottleneck is the guest Linux driver, since
it can not stress vhost for a 100% CPU utilization. A full TCP
benchmark is ongoing. Will test virtio-net pmd as well when it was
ready.

Well the question then is why we should bother merging this
if this doesn't give a performance gain.

We meet bottlenecks at other places. I can only test Linux driver which has
lots of overheads e.g interrupts. And perf show only a small fraction of
time were spent on e.g virtqueue manipulation. I hope virtio-net pmd can
give us different result but we don't have one ready for testing now. (Jen's
V4 have bugs thus can not work with this series).

Can't linux busy poll? And how about testing loopback with XDP?

quoted

  Do you see
a gain in CPU utilization maybe?

Unfortunately not.

quoted

If not - let's wait for that TCP benchmark result?

We can, but you know TCP_STREAM result is sometime misleading.

A brunch of other patches of mine were rebased on this and then blocked on
this series. Consider we don't meet regression, maybe we can merge this
first and try optimizations or fixups on top?

Thanks

I'm not sure I understand this approach. Packed ring is just an optimization.
What value is there in merging it if it does not help speed?

quoted

quoted

Notes:
- This version depends on Tiwei's series at https://patchwork.ozlabs.org/cover/942297/

This version were tested with:

- Zerocopy (Out of Order) support
- vIOMMU support
- mergeable buffer on/off
- busy polling on/off
- vsock (nc-vsock)

Changes from V1:
- drop uapi patch and use Tiwei's
- split the enablement of packed virtqueue into a separate patch

Changes from RFC V5:

- save unnecessary barriers during vhost_add_used_packed_n()
- more compact math for event idx
- fix failure of SET_VRING_BASE when avail_wrap_counter is true
- fix not copy avail_wrap_counter during GET_VRING_BASE
- introduce SET_VRING_USED_BASE/GET_VRING_USED_BASE for syncing last_used_idx
- rename used_wrap_counter to last_used_wrap_counter
- rebase to net-next

Changes from RFC V4:

- fix signalled_used index recording
- track avail index correctly
- various minor fixes

Changes from RFC V3:

- Fix math on event idx checking
- Sync last avail wrap counter through GET/SET_VRING_BASE
- remove desc_event prefix in the driver/device structure

Changes from RFC V2:

- do not use & in checking desc_event_flags
- off should be most significant bit
- remove the workaround of mergeable buffer for dpdk prototype
- id should be in the last descriptor in the chain
- keep _F_WRITE for write descriptor when adding used
- device flags updating should use ADDR_USED type
- return error on unexpected unavail descriptor in a chain
- return false in vhost_ve_avail_empty is descriptor is available
- track last seen avail_wrap_counter
- correctly examine available descriptor in get_indirect_packed()
- vhost_idx_diff should return u16 instead of bool

Changes from RFC V1:

- Refactor vhost used elem code to avoid open coding on used elem
- Event suppression support (compile test only).
- Indirect descriptor support (compile test only).
- Zerocopy support.
- vIOMMU support.
- SCSI/VSOCK support (compile test only).
- Fix several bugs

Jason Wang (8):
   vhost: move get_rx_bufs to vhost.c
   vhost: hide used ring layout from device
   vhost: do not use vring_used_elem
   vhost_net: do not explicitly manipulate vhost_used_elem
   vhost: vhost_put_user() can accept metadata type
   vhost: packed ring support
   vhost: event suppression for packed ring
   vhost: enable packed virtqueues

  drivers/vhost/net.c        | 143 ++-----
  drivers/vhost/scsi.c       |  62 +--
  drivers/vhost/vhost.c      | 994 ++++++++++++++++++++++++++++++++++++++++-----
  drivers/vhost/vhost.h      |  55 ++-
  drivers/vhost/vsock.c      |  42 +-
  include/uapi/linux/vhost.h |   7 +
  6 files changed, 1035 insertions(+), 268 deletions(-)

-- 
2.7.4