netdev - Re: [PATCH net-next v6 3/4] net: vhost: factor out busy polling logic to vhost_net_busy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMDZJNVPi8DMScyF2KW0mLAYe0p8uUVVY55Oo9Hk6SJJTMZYKg@mail.gmail.com>
Date:   Mon, 23 Jul 2018 20:43:03 +0800
From:   Tonghao Zhang <xiangxia.m.yue@...il.com>
To:     makita.toshiaki@....ntt.co.jp
Cc:     jasowang@...hat.com, mst@...hat.com,
        virtualization@...ts.linux-foundation.org,
        Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next v6 3/4] net: vhost: factor out busy polling logic
 to vhost_net_busy_poll()

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