| 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
| ||
|
Message-ID: <CAMDZJNUU+cHu0_pjCiFQY7UuXYULz=ciiFs2zwnarZDckzgajQ@mail.gmail.com>
Date: Mon, 2 Jul 2018 12:05:30 +0800
From: Tonghao Zhang <xiangxia.m.yue@...il.com>
To: jasowang@...hat.com
Cc: mst@...hat.com, makita.toshiaki@....ntt.co.jp,
virtualization@...ts.linux-foundation.org,
Linux Kernel Network Developers <netdev@...r.kernel.org>,
Tonghao Zhang <zhangtonghao@...ichuxing.com>
Subject: Re: [PATCH net-next v3 3/4] net: vhost: factor out busy polling logic
to vhost_net_busy_poll()
On Mon, Jul 2, 2018 at 10:29 AM Jason Wang <jasowang@...hat.com> wrote:
>
>
>
> On 2018年06月30日 14:33, 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 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 <zhangtonghao@...ichuxing.com>
> > ---
> > drivers/vhost/net.c | 92 ++++++++++++++++++++++++++++++-----------------------
> > 1 file changed, 53 insertions(+), 39 deletions(-)
> >
> > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > index 62bb8e8..458f81d 100644
> > --- a/drivers/vhost/net.c
> > +++ b/drivers/vhost/net.c
> > @@ -429,6 +429,50 @@ static int vhost_net_enable_vq(struct vhost_net *n,
> > return vhost_poll_start(poll, sock->file);
> > }
> >
> > +static int sk_has_rx_data(struct sock *sk)
> > +{
> > + struct socket *sock = sk->sk_socket;
> > +
> > + if (sock->ops->peek_len)
> > + return sock->ops->peek_len(sock);
> > +
> > + return skb_queue_empty(&sk->sk_receive_queue);
> > +}
> > +
> > +static void vhost_net_busy_poll(struct vhost_net *net,
> > + struct vhost_virtqueue *rvq,
> > + struct vhost_virtqueue *tvq,
> > + bool rx)
> > +{
> > + unsigned long uninitialized_var(endtime);
> > + struct socket *sock = rvq->private_data;
> > + struct vhost_virtqueue *vq = rx ? tvq : rvq;
> > + unsigned long busyloop_timeout = rx ? rvq->busyloop_timeout :
> > + tvq->busyloop_timeout;
>
> As simple as vq->busyloop_timeout?
maybe we should allow user set busyloop_timeout for rx or tx
differently. this code should be moved under mutex.
> > +
> > + mutex_lock_nested(&vq->mutex, rx ? VHOST_NET_VQ_TX: VHOST_NET_VQ_RX);
>
> We need move sock = rvq->private_data under the protection of vq mutex
> if rx is false.
yes, thanks for your review.
> > + vhost_disable_notify(&net->dev, vq);
> > +
> > + preempt_disable();
> > + endtime = busy_clock() + busyloop_timeout;
> > + while (vhost_can_busy_poll(tvq->dev, endtime) &&
> > + !(sock && sk_has_rx_data(sock->sk)) &&
> > + vhost_vq_avail_empty(tvq->dev, tvq))
> > + cpu_relax();
> > + preempt_enable();
> > +
> > + if ((rx && !vhost_vq_avail_empty(&net->dev, vq)) ||
> > + (!rx && (sock && sk_has_rx_data(sock->sk)))) {
> > + vhost_poll_queue(&vq->poll);
> > + } else if (unlikely(vhost_enable_notify(&net->dev, vq))) {
> > + vhost_disable_notify(&net->dev, vq);
> > + vhost_poll_queue(&vq->poll);
> > + }
> > +
> > + mutex_unlock(&vq->mutex);
> > +}
> > +
> > +
> > static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
> > struct vhost_virtqueue *vq,
> > struct iovec iov[], unsigned int iov_size,
> > @@ -621,16 +665,6 @@ static int peek_head_len(struct vhost_net_virtqueue *rvq, struct sock *sk)
> > return len;
> > }
> >
> > -static int sk_has_rx_data(struct sock *sk)
> > -{
> > - struct socket *sock = sk->sk_socket;
> > -
> > - if (sock->ops->peek_len)
> > - return sock->ops->peek_len(sock);
> > -
> > - return skb_queue_empty(&sk->sk_receive_queue);
> > -}
> > -
> > static void vhost_rx_signal_used(struct vhost_net_virtqueue *nvq)
> > {
> > struct vhost_virtqueue *vq = &nvq->vq;
> > @@ -645,39 +679,19 @@ static void vhost_rx_signal_used(struct vhost_net_virtqueue *nvq)
> >
> > static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk)
> > {
> > - struct vhost_net_virtqueue *rvq = &net->vqs[VHOST_NET_VQ_RX];
> > - struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX];
> > - struct vhost_virtqueue *vq = &nvq->vq;
> > - unsigned long uninitialized_var(endtime);
> > - int len = peek_head_len(rvq, sk);
> > + struct vhost_net_virtqueue *nvq_rx = &net->vqs[VHOST_NET_VQ_RX];
> > + struct vhost_net_virtqueue *nvq_tx = &net->vqs[VHOST_NET_VQ_TX];
>
> It looks to me rnvq and tnvq is slightly better.
yes. patch 4 will also update.
> Other looks good to me.
>
> Thanks
>
> >
> > - if (!len && vq->busyloop_timeout) {
> > - /* Flush batched heads first */
> > - vhost_rx_signal_used(rvq);
> > - /* Both tx vq and rx socket were polled here */
> > - mutex_lock_nested(&vq->mutex, VHOST_NET_VQ_TX);
> > - vhost_disable_notify(&net->dev, vq);
> > + int len = peek_head_len(nvq_rx, sk);
> >
> > - preempt_disable();
> > - endtime = busy_clock() + vq->busyloop_timeout;
> > -
> > - while (vhost_can_busy_poll(&net->dev, endtime) &&
> > - !sk_has_rx_data(sk) &&
> > - vhost_vq_avail_empty(&net->dev, vq))
> > - cpu_relax();
> > -
> > - preempt_enable();
> > -
> > - if (!vhost_vq_avail_empty(&net->dev, vq))
> > - vhost_poll_queue(&vq->poll);
> > - else if (unlikely(vhost_enable_notify(&net->dev, vq))) {
> > - vhost_disable_notify(&net->dev, vq);
> > - vhost_poll_queue(&vq->poll);
> > - }
> > + if (!len && nvq_rx->vq.busyloop_timeout) {
> > + /* Flush batched heads first */
> > + vhost_rx_signal_used(nvq_rx);
> >
> > - mutex_unlock(&vq->mutex);
> > + /* Both tx vq and rx socket were polled here */
> > + vhost_net_busy_poll(net, &nvq_rx->vq, &nvq_tx->vq, true);
> >
> > - len = peek_head_len(rvq, sk);
> > + len = peek_head_len(nvq_rx, sk);
> > }
> >
> > return len;
>
Powered by blists - more mailing lists