On 2018/07/24 2:31, Tonghao Zhang wrote:

On Mon, Jul 23, 2018 at 10:20 PM Toshiaki Makita
[off-list ref] wrote:

quoted

On 18/07/23 (月) 21:43, Tonghao Zhang wrote:

quoted

On Mon, Jul 23, 2018 at 5:58 PM Toshiaki Makita
[off-list ref] wrote:

quoted

On 2018/07/22 3:04, xiangxia.m.yue@gmail.com wrote:

quoted

From: Tonghao Zhang <redacted>

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 <redacted>
---

...

quoted

+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:

I think vhost_enable_notify is needed.

quoted

} 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);
         }
}

As Jason review as before, we only want rx kick when packet is pending at
socket but we're out of available buffers. So we just enable notify,
but not poll it ?

         } else if ((sock && sk_has_rx_data(sock->sk)) &&
                     !vhost_vq_avail_empty(&net->dev, rvq)) {
                 vhost_poll_queue(&rvq->poll);
         else {
                 vhost_enable_notify(&net->dev, rvq);
         }

When vhost_enable_notify() returns true the avail becomes non-empty
while we are enabling notify. We may delay the rx process if we don't
check the return value of vhost_enable_notify().

I got it thanks.

quoted

quoted

quoted

Also it's better to care vhost_net_disable_vq()/vhost_net_enable_vq() on tx?

I cant find why it is better, if necessary, we can do it.

The reason is pretty simple... we are busypolling the socket so we don't
need rx wakeups during it?

OK, but one question, how about rx? do we use the
vhost_net_disable_vq/vhost_net_ensable_vq on rx ?

If we are busypolling the sock tx buf? I'm not sure if polling it
improves the performance.

-- 
Toshiaki Makita

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization