On 2018/07/24 2:31, Tonghao Zhang wrote: > On Mon, Jul 23, 2018 at 10:20 PM Toshiaki Makita > <[email protected]> wrote: >> >> On 18/07/23 (月) 21:43, Tonghao Zhang wrote: >>> On Mon, Jul 23, 2018 at 5:58 PM Toshiaki Makita >>> <[email protected]> wrote: >>>> >>>> On 2018/07/22 3:04, [email protected] wrote: >>>>> From: Tonghao Zhang <[email protected]> >>>>> >>>>> 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 <[email protected]> >>>>> --- >>>> ... >>>>> +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. >>> >>>> } 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. >>>> 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 [email protected] https://lists.linuxfoundation.org/mailman/listinfo/virtualization
