From mboxrd@z Thu Jan  1 00:00:00 1970
From: Toshiaki Makita <toshiaki.makita1@gmail.com>
Subject: Re: [PATCH net-next v6 3/4] net: vhost: factor out busy polling logic
 to vhost_net_busy_poll()
Date: Mon, 23 Jul 2018 23:20:54 +0900
Message-ID: <2b0efbf4-09e2-0ee9-091f-e2d9e10483a1@gmail.com>
References: <1532196242-2998-1-git-send-email-xiangxia.m.yue@gmail.com>
 <1532196242-2998-4-git-send-email-xiangxia.m.yue@gmail.com>
 <c520c142-ef36-7918-7135-51258c17bd83@lab.ntt.co.jp>
 <CAMDZJNVPi8DMScyF2KW0mLAYe0p8uUVVY55Oo9Hk6SJJTMZYKg@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Cc: jasowang@redhat.com, mst@redhat.com,
        virtualization@lists.linux-foundation.org,
        Linux Kernel Network Developers <netdev@vger.kernel.org>
To: Tonghao Zhang <xiangxia.m.yue@gmail.com>,
        makita.toshiaki@lab.ntt.co.jp
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-pl0-f66.google.com ([209.85.160.66]:44781 "EHLO
        mail-pl0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S2388029AbeGWPW1 (ORCPT
        <rfc822;netdev@vger.kernel.org>); Mon, 23 Jul 2018 11:22:27 -0400
Received: by mail-pl0-f66.google.com with SMTP id m16-v6so287913pls.11
        for <netdev@vger.kernel.org>; Mon, 23 Jul 2018 07:20:58 -0700 (PDT)
In-Reply-To: <CAMDZJNVPi8DMScyF2KW0mLAYe0p8uUVVY55Oo9Hk6SJJTMZYKg@mail.gmail.com>
Content-Language: en-US
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

On 18/07/23 (月) 21:43, Tonghao Zhang wrote:
> On Mon, Jul 23, 2018 at 5:58 PM Toshiaki Makita
> <makita.toshiaki@lab.ntt.co.jp> wrote:
>>
>> On 2018/07/22 3:04, xiangxia.m.yue@gmail.com wrote:
>>> From: Tonghao Zhang <xiangxia.m.yue@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@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:
> 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().

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