xiangxia.m.yue at gmail.com
2018-Jul-21 18:03 UTC
[PATCH net-next v6 0/4] net: vhost: improve performance when enable busyloop
From: Tonghao Zhang <xiangxia.m.yue at gmail.com> This patches improve the guest receive performance. On the handle_tx side, we poll the sock receive queue at the same time. handle_rx do that in the same way. For more performance report, see patch 4. v5->v6: rebase the codes. Tonghao Zhang (4): net: vhost: lock the vqs one by one net: vhost: replace magic number of lock annotation net: vhost: factor out busy polling logic to vhost_net_busy_poll() net: vhost: add rx busy polling in tx path drivers/vhost/net.c | 148 +++++++++++++++++++++++++++++--------------------- drivers/vhost/vhost.c | 24 +++----- 2 files changed, 94 insertions(+), 78 deletions(-) -- 1.8.3.1
xiangxia.m.yue at gmail.com
2018-Jul-21 18:03 UTC
[PATCH net-next v6 1/4] net: vhost: lock the vqs one by one
From: Tonghao Zhang <xiangxia.m.yue at gmail.com> This patch changes the way that lock all vqs at the same, to lock them one by one. It will be used for next patch to avoid the deadlock. Signed-off-by: Tonghao Zhang <xiangxia.m.yue at gmail.com> Acked-by: Jason Wang <jasowang at redhat.com> Signed-off-by: Jason Wang <jasowang at redhat.com> --- drivers/vhost/vhost.c | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index a502f1a..a1c06e7 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -294,8 +294,11 @@ static void vhost_vq_meta_reset(struct vhost_dev *d) { int i; - for (i = 0; i < d->nvqs; ++i) + for (i = 0; i < d->nvqs; ++i) { + mutex_lock(&d->vqs[i]->mutex); __vhost_vq_meta_reset(d->vqs[i]); + mutex_unlock(&d->vqs[i]->mutex); + } } static void vhost_vq_reset(struct vhost_dev *dev, @@ -890,20 +893,6 @@ static inline void __user *__vhost_get_user(struct vhost_virtqueue *vq, #define vhost_get_used(vq, x, ptr) \ vhost_get_user(vq, x, ptr, VHOST_ADDR_USED) -static void vhost_dev_lock_vqs(struct vhost_dev *d) -{ - int i = 0; - for (i = 0; i < d->nvqs; ++i) - mutex_lock_nested(&d->vqs[i]->mutex, i); -} - -static void vhost_dev_unlock_vqs(struct vhost_dev *d) -{ - int i = 0; - for (i = 0; i < d->nvqs; ++i) - mutex_unlock(&d->vqs[i]->mutex); -} - static int vhost_new_umem_range(struct vhost_umem *umem, u64 start, u64 size, u64 end, u64 userspace_addr, int perm) @@ -953,7 +942,10 @@ static void vhost_iotlb_notify_vq(struct vhost_dev *d, if (msg->iova <= vq_msg->iova && msg->iova + msg->size - 1 > vq_msg->iova && vq_msg->type == VHOST_IOTLB_MISS) { + mutex_lock(&node->vq->mutex); vhost_poll_queue(&node->vq->poll); + mutex_unlock(&node->vq->mutex); + list_del(&node->node); kfree(node); } @@ -985,7 +977,6 @@ static int vhost_process_iotlb_msg(struct vhost_dev *dev, int ret = 0; mutex_lock(&dev->mutex); - vhost_dev_lock_vqs(dev); switch (msg->type) { case VHOST_IOTLB_UPDATE: if (!dev->iotlb) { @@ -1019,7 +1010,6 @@ static int vhost_process_iotlb_msg(struct vhost_dev *dev, break; } - vhost_dev_unlock_vqs(dev); mutex_unlock(&dev->mutex); return ret; -- 1.8.3.1
xiangxia.m.yue at gmail.com
2018-Jul-21 18:04 UTC
[PATCH net-next v6 2/4] net: vhost: replace magic number of lock annotation
From: Tonghao Zhang <xiangxia.m.yue at gmail.com> Use the VHOST_NET_VQ_XXX as a subclass for mutex_lock_nested. Signed-off-by: Tonghao Zhang <xiangxia.m.yue at gmail.com> Acked-by: Jason Wang <jasowang at redhat.com> --- drivers/vhost/net.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c index b224036..321264c 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c @@ -490,7 +490,7 @@ static void handle_tx(struct vhost_net *net) bool zcopy, zcopy_used; int sent_pkts = 0; - mutex_lock(&vq->mutex); + mutex_lock_nested(&vq->mutex, VHOST_NET_VQ_TX); sock = vq->private_data; if (!sock) goto out; @@ -667,7 +667,7 @@ static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk, /* Flush batched heads first */ vhost_rx_signal_used(rnvq); /* Both tx vq and rx socket were polled here */ - mutex_lock_nested(&tvq->mutex, 1); + mutex_lock_nested(&tvq->mutex, VHOST_NET_VQ_TX); vhost_disable_notify(&net->dev, tvq); preempt_disable(); @@ -809,7 +809,7 @@ static void handle_rx(struct vhost_net *net) __virtio16 num_buffers; int recv_pkts = 0; - mutex_lock_nested(&vq->mutex, 0); + mutex_lock_nested(&vq->mutex, VHOST_NET_VQ_RX); sock = vq->private_data; if (!sock) goto out; -- 1.8.3.1
xiangxia.m.yue at gmail.com
2018-Jul-21 18:04 UTC
[PATCH net-next v6 3/4] net: vhost: factor out busy polling logic to vhost_net_busy_poll()
From: Tonghao Zhang <xiangxia.m.yue at gmail.com> Factor out generic busy polling logic and will be used for in tx path in the next patch. And with the patch, qemu can set differently the busyloop_timeout for rx queue. Signed-off-by: Tonghao Zhang <xiangxia.m.yue at gmail.com> --- drivers/vhost/net.c | 114 ++++++++++++++++++++++++++++++++++------------------ 1 file changed, 74 insertions(+), 40 deletions(-) diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c index 321264c..2dc937e 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c @@ -428,6 +428,78 @@ static int vhost_net_enable_vq(struct vhost_net *n, return vhost_poll_start(poll, sock->file); } +static int sk_has_rx_data(struct sock *sk) +{ + struct socket *sock = sk->sk_socket; + + if (sock->ops->peek_len) + return sock->ops->peek_len(sock); + + return skb_queue_empty(&sk->sk_receive_queue); +} + +static void vhost_net_busy_poll_vq_check(struct vhost_net *net, + struct vhost_virtqueue *rvq, + struct vhost_virtqueue *tvq, + bool rx) +{ + struct socket *sock = rvq->private_data; + + if (rx) { + if (!vhost_vq_avail_empty(&net->dev, tvq)) { + vhost_poll_queue(&tvq->poll); + } else if (unlikely(vhost_enable_notify(&net->dev, tvq))) { + vhost_disable_notify(&net->dev, tvq); + vhost_poll_queue(&tvq->poll); + } + } else if ((sock && sk_has_rx_data(sock->sk)) && + !vhost_vq_avail_empty(&net->dev, rvq)) { + vhost_poll_queue(&rvq->poll); + } +} + +static void vhost_net_busy_poll(struct vhost_net *net, + struct vhost_virtqueue *rvq, + struct vhost_virtqueue *tvq, + bool *busyloop_intr, + bool rx) +{ + unsigned long busyloop_timeout; + unsigned long endtime; + struct socket *sock; + struct vhost_virtqueue *vq = rx ? tvq : rvq; + + mutex_lock_nested(&vq->mutex, rx ? VHOST_NET_VQ_TX: VHOST_NET_VQ_RX); + vhost_disable_notify(&net->dev, vq); + sock = rvq->private_data; + + busyloop_timeout = rx ? rvq->busyloop_timeout: + tvq->busyloop_timeout; + + preempt_disable(); + endtime = busy_clock() + busyloop_timeout; + + while (vhost_can_busy_poll(endtime)) { + if (vhost_has_work(&net->dev)) { + *busyloop_intr = true; + break; + } + + if ((sock && sk_has_rx_data(sock->sk) && + !vhost_vq_avail_empty(&net->dev, rvq)) || + !vhost_vq_avail_empty(&net->dev, tvq)) + break; + + cpu_relax(); + } + + preempt_enable(); + + vhost_net_busy_poll_vq_check(net, rvq, tvq, rx); + + mutex_unlock(&vq->mutex); +} + static int vhost_net_tx_get_vq_desc(struct vhost_net *net, struct vhost_virtqueue *vq, struct iovec iov[], unsigned int iov_size, @@ -631,16 +703,6 @@ static int peek_head_len(struct vhost_net_virtqueue *rvq, struct sock *sk) return len; } -static int sk_has_rx_data(struct sock *sk) -{ - struct socket *sock = sk->sk_socket; - - if (sock->ops->peek_len) - return sock->ops->peek_len(sock); - - return skb_queue_empty(&sk->sk_receive_queue); -} - static void vhost_rx_signal_used(struct vhost_net_virtqueue *nvq) { struct vhost_virtqueue *vq = &nvq->vq; @@ -660,41 +722,13 @@ static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk, struct vhost_net_virtqueue *tnvq = &net->vqs[VHOST_NET_VQ_TX]; struct vhost_virtqueue *rvq = &rnvq->vq; struct vhost_virtqueue *tvq = &tnvq->vq; - unsigned long uninitialized_var(endtime); int len = peek_head_len(rnvq, sk); - if (!len && tvq->busyloop_timeout) { + if (!len && rvq->busyloop_timeout) { /* Flush batched heads first */ vhost_rx_signal_used(rnvq); /* Both tx vq and rx socket were polled here */ - mutex_lock_nested(&tvq->mutex, VHOST_NET_VQ_TX); - vhost_disable_notify(&net->dev, tvq); - - preempt_disable(); - endtime = busy_clock() + tvq->busyloop_timeout; - - while (vhost_can_busy_poll(endtime)) { - if (vhost_has_work(&net->dev)) { - *busyloop_intr = true; - break; - } - if ((sk_has_rx_data(sk) && - !vhost_vq_avail_empty(&net->dev, rvq)) || - !vhost_vq_avail_empty(&net->dev, tvq)) - break; - cpu_relax(); - } - - preempt_enable(); - - if (!vhost_vq_avail_empty(&net->dev, tvq)) { - vhost_poll_queue(&tvq->poll); - } else if (unlikely(vhost_enable_notify(&net->dev, tvq))) { - vhost_disable_notify(&net->dev, tvq); - vhost_poll_queue(&tvq->poll); - } - - mutex_unlock(&tvq->mutex); + vhost_net_busy_poll(net, rvq, tvq, busyloop_intr, true); len = peek_head_len(rnvq, sk); } -- 1.8.3.1
xiangxia.m.yue at gmail.com
2018-Jul-21 18:04 UTC
[PATCH net-next v6 4/4] net: vhost: add rx busy polling in tx path
From: Tonghao Zhang <xiangxia.m.yue at gmail.com> This patch improves the guest receive performance. On the handle_tx side, we poll the sock receive queue at the same time. handle_rx do that in the same way. We set the poll-us=100us and use the netperf to test throughput and mean latency. When running the tests, the vhost-net kthread of that VM, is alway 100% CPU. The commands are shown as below. Rx performance is greatly improved by this patch. There is not notable performance change on tx with this series though. This patch is useful for bi-directional traffic. netperf -H IP -t TCP_STREAM -l 20 -- -O "THROUGHPUT, THROUGHPUT_UNITS, MEAN_LATENCY" Topology: [Host] ->linux bridge -> tap vhost-net ->[Guest] TCP_STREAM: * Without the patch: 19842.95 Mbps, 6.50 us mean latency * With the patch: 37373.10 Mbps, 3.43 us mean latency Signed-off-by: Tonghao Zhang <xiangxia.m.yue at gmail.com> --- drivers/vhost/net.c | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c index 2dc937e..a562828 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c @@ -501,29 +501,21 @@ static void vhost_net_busy_poll(struct vhost_net *net, } static int vhost_net_tx_get_vq_desc(struct vhost_net *net, - struct vhost_virtqueue *vq, + struct vhost_virtqueue *tvq, struct iovec iov[], unsigned int iov_size, unsigned int *out_num, unsigned int *in_num, bool *busyloop_intr) { - unsigned long uninitialized_var(endtime); - int r = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov), - out_num, in_num, NULL, NULL); - - if (r == vq->num && vq->busyloop_timeout) { - preempt_disable(); - endtime = busy_clock() + vq->busyloop_timeout; - while (vhost_can_busy_poll(endtime)) { - if (vhost_has_work(vq->dev)) { - *busyloop_intr = true; - break; - } - if (!vhost_vq_avail_empty(vq->dev, vq)) - break; - cpu_relax(); - } - preempt_enable(); - r = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov), + struct vhost_net_virtqueue *rnvq = &net->vqs[VHOST_NET_VQ_RX]; + struct vhost_virtqueue *rvq = &rnvq->vq; + + int r = vhost_get_vq_desc(tvq, tvq->iov, ARRAY_SIZE(tvq->iov), + out_num, in_num, NULL, NULL); + + if (r == tvq->num && tvq->busyloop_timeout) { + vhost_net_busy_poll(net, rvq, tvq, busyloop_intr, false); + + r = vhost_get_vq_desc(tvq, tvq->iov, ARRAY_SIZE(tvq->iov), out_num, in_num, NULL, NULL); } -- 1.8.3.1
Michael S. Tsirkin
2018-Jul-22 15:26 UTC
[PATCH net-next v6 1/4] net: vhost: lock the vqs one by one
On Sat, Jul 21, 2018 at 11:03:59AM -0700, xiangxia.m.yue at gmail.com wrote:> From: Tonghao Zhang <xiangxia.m.yue at gmail.com> > > This patch changes the way that lock all vqs > at the same, to lock them one by one. It will > be used for next patch to avoid the deadlock. > > Signed-off-by: Tonghao Zhang <xiangxia.m.yue at gmail.com> > Acked-by: Jason Wang <jasowang at redhat.com> > Signed-off-by: Jason Wang <jasowang at redhat.com> > --- > drivers/vhost/vhost.c | 24 +++++++----------------- > 1 file changed, 7 insertions(+), 17 deletions(-) > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > index a502f1a..a1c06e7 100644 > --- a/drivers/vhost/vhost.c > +++ b/drivers/vhost/vhost.c > @@ -294,8 +294,11 @@ static void vhost_vq_meta_reset(struct vhost_dev *d) > { > int i; > > - for (i = 0; i < d->nvqs; ++i) > + for (i = 0; i < d->nvqs; ++i) { > + mutex_lock(&d->vqs[i]->mutex); > __vhost_vq_meta_reset(d->vqs[i]); > + mutex_unlock(&d->vqs[i]->mutex); > + } > } > > static void vhost_vq_reset(struct vhost_dev *dev, > @@ -890,20 +893,6 @@ static inline void __user *__vhost_get_user(struct vhost_virtqueue *vq, > #define vhost_get_used(vq, x, ptr) \ > vhost_get_user(vq, x, ptr, VHOST_ADDR_USED) > > -static void vhost_dev_lock_vqs(struct vhost_dev *d) > -{ > - int i = 0; > - for (i = 0; i < d->nvqs; ++i) > - mutex_lock_nested(&d->vqs[i]->mutex, i); > -} > - > -static void vhost_dev_unlock_vqs(struct vhost_dev *d) > -{ > - int i = 0; > - for (i = 0; i < d->nvqs; ++i) > - mutex_unlock(&d->vqs[i]->mutex); > -} > - > static int vhost_new_umem_range(struct vhost_umem *umem, > u64 start, u64 size, u64 end, > u64 userspace_addr, int perm) > @@ -953,7 +942,10 @@ static void vhost_iotlb_notify_vq(struct vhost_dev *d, > if (msg->iova <= vq_msg->iova && > msg->iova + msg->size - 1 > vq_msg->iova && > vq_msg->type == VHOST_IOTLB_MISS) { > + mutex_lock(&node->vq->mutex); > vhost_poll_queue(&node->vq->poll); > + mutex_unlock(&node->vq->mutex); > + > list_del(&node->node); > kfree(node); > } > @@ -985,7 +977,6 @@ static int vhost_process_iotlb_msg(struct vhost_dev *dev, > int ret = 0; > > mutex_lock(&dev->mutex); > - vhost_dev_lock_vqs(dev); > switch (msg->type) { > case VHOST_IOTLB_UPDATE: > if (!dev->iotlb) { > @@ -1019,7 +1010,6 @@ static int vhost_process_iotlb_msg(struct vhost_dev *dev, > break; > } > > - vhost_dev_unlock_vqs(dev); > mutex_unlock(&dev->mutex); > > return ret;I do prefer the finer-grained locking but I remember we discussed something like this in the past and Jason saw issues with such a locking. Jason?> -- > 1.8.3.1
Toshiaki Makita
2018-Jul-23 09:57 UTC
[PATCH net-next v6 3/4] net: vhost: factor out busy polling logic to vhost_net_busy_poll()
On 2018/07/22 3:04, xiangxia.m.yue at gmail.com wrote:> From: Tonghao Zhang <xiangxia.m.yue at gmail.com> > > Factor out generic busy polling logic and will be > used for in tx path in the next patch. And with the patch, > qemu can set differently the busyloop_timeout for rx queue. > > Signed-off-by: Tonghao Zhang <xiangxia.m.yue at gmail.com> > ---...> +static void vhost_net_busy_poll_vq_check(struct vhost_net *net, > + struct vhost_virtqueue *rvq, > + struct vhost_virtqueue *tvq, > + bool rx) > +{ > + struct socket *sock = rvq->private_data; > + > + if (rx) { > + if (!vhost_vq_avail_empty(&net->dev, tvq)) { > + vhost_poll_queue(&tvq->poll); > + } else if (unlikely(vhost_enable_notify(&net->dev, tvq))) { > + vhost_disable_notify(&net->dev, tvq); > + vhost_poll_queue(&tvq->poll); > + } > + } else if ((sock && sk_has_rx_data(sock->sk)) && > + !vhost_vq_avail_empty(&net->dev, rvq)) { > + vhost_poll_queue(&rvq->poll);Now we wait for vq_avail for rx as well, I think you cannot skip vhost_enable_notify() on tx. Probably you might want to do: } else if (sock && sk_has_rx_data(sock->sk)) { if (!vhost_vq_avail_empty(&net->dev, rvq)) { vhost_poll_queue(&rvq->poll); } else if (unlikely(vhost_enable_notify(&net->dev, rvq))) { vhost_disable_notify(&net->dev, rvq); vhost_poll_queue(&rvq->poll); } } Also it's better to care vhost_net_disable_vq()/vhost_net_enable_vq() on tx? -- Toshiaki Makita
David Miller
2018-Jul-25 20:01 UTC
[PATCH net-next v6 0/4] net: vhost: improve performance when enable busyloop
From: xiangxia.m.yue at gmail.com Date: Sat, 21 Jul 2018 11:03:58 -0700> From: Tonghao Zhang <xiangxia.m.yue at gmail.com> > > This patches improve the guest receive performance. > On the handle_tx side, we poll the sock receive queue > at the same time. handle_rx do that in the same way. > > For more performance report, see patch 4. > > v5->v6: > rebase the codes.It looks like there is still some dangling discussions about this patch set. Please repost this series when those discussions have completed. Thank you.
Seemingly Similar Threads
- [PATCH net-next v6 1/4] net: vhost: lock the vqs one by one
- [PATCH net-next v6 0/4] net: vhost: improve performance when enable busyloop
- [REBASE PATCH net-next v9 1/4] net: vhost: lock the vqs one by one
- [REBASE PATCH net-next v9 1/4] net: vhost: lock the vqs one by one
- [PATCH net-next v9 0/6] net: vhost: improve performance when enable busyloop