From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tonghao Zhang Subject: Re: [PATCH net-next v6 3/4] net: vhost: factor out busy polling logic to vhost_net_busy_poll() Date: Tue, 24 Jul 2018 01:31:10 +0800 Message-ID: References: <1532196242-2998-1-git-send-email-xiangxia.m.yue@gmail.com> <1532196242-2998-4-git-send-email-xiangxia.m.yue@gmail.com> <2b0efbf4-09e2-0ee9-091f-e2d9e10483a1@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Cc: makita.toshiaki@lab.ntt.co.jp, jasowang@redhat.com, mst@redhat.com, virtualization@lists.linux-foundation.org, Linux Kernel Network Developers To: toshiaki.makita1@gmail.com Return-path: Received: from mail-oi0-f68.google.com ([209.85.218.68]:39980 "EHLO mail-oi0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2388251AbeGWSdh (ORCPT ); Mon, 23 Jul 2018 14:33:37 -0400 Received: by mail-oi0-f68.google.com with SMTP id w126-v6so2568061oie.7 for ; Mon, 23 Jul 2018 10:31:21 -0700 (PDT) In-Reply-To: <2b0efbf4-09e2-0ee9-091f-e2d9e10483a1@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: On Mon, Jul 23, 2018 at 10:20 PM Toshiaki Makita wrote: > > On 18/07/23 (=E6=9C=88) 21:43, Tonghao Zhang wrote: > > On Mon, Jul 23, 2018 at 5:58 PM Toshiaki Makita > > wrote: > >> > >> On 2018/07/22 3:04, xiangxia.m.yue@gmail.com wrote: > >>> From: Tonghao Zhang > >>> > >>> 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 > >>> --- > >> ... > >>> +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 =3D 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 ? > -- > Toshiaki Makita