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

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().

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?