Toshiaki Makita
2018-Jul-04 07:59 UTC
[PATCH net-next v5 3/4] net: vhost: factor out busy polling logic to vhost_net_busy_poll()
On 2018/07/04 13:31, xiangxia.m.yue at gmail.com wrote: ...> +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); > + unsigned long busyloop_timeout; > + struct socket *sock; > + struct vhost_virtqueue *vq = rx ? tvq : rvq; > + > + mutex_lock_nested(&vq->mutex, rx ? VHOST_NET_VQ_TX: VHOST_NET_VQ_RX); > + > + vhost_disable_notify(&net->dev, vq); > + sock = rvq->private_data; > + busyloop_timeout = rx ? rvq->busyloop_timeout : tvq->busyloop_timeout; > + > + 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 (vhost_enable_notify(&net->dev, vq) && rx) {Hmm... on tx here sock has no rx data, so you are waiting for sock wakeup for rx and vhost_enable_notify() seems not needed. Do you want this actually? } else if (rx && vhost_enable_notify(&net->dev, vq)) {> + vhost_disable_notify(&net->dev, vq); > + vhost_poll_queue(&vq->poll); > + }-- Toshiaki Makita
Jason Wang
2018-Jul-04 09:18 UTC
[PATCH net-next v5 3/4] net: vhost: factor out busy polling logic to vhost_net_busy_poll()
On 2018?07?04? 15:59, Toshiaki Makita wrote:> On 2018/07/04 13:31, xiangxia.m.yue at gmail.com wrote: > ... >> +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); >> + unsigned long busyloop_timeout; >> + struct socket *sock; >> + struct vhost_virtqueue *vq = rx ? tvq : rvq; >> + >> + mutex_lock_nested(&vq->mutex, rx ? VHOST_NET_VQ_TX: VHOST_NET_VQ_RX); >> + >> + vhost_disable_notify(&net->dev, vq); >> + sock = rvq->private_data; >> + busyloop_timeout = rx ? rvq->busyloop_timeout : tvq->busyloop_timeout; >> + >> + 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 (vhost_enable_notify(&net->dev, vq) && rx) { > Hmm... on tx here sock has no rx data, so you are waiting for sock > wakeup for rx and vhost_enable_notify() seems not needed. Do you want > this actually? > > } else if (rx && vhost_enable_notify(&net->dev, vq)) {Right, rx need to be checked first here. Thanks>> + vhost_disable_notify(&net->dev, vq); >> + vhost_poll_queue(&vq->poll); >> + }
Tonghao Zhang
2018-Jul-04 09:46 UTC
[PATCH net-next v5 3/4] net: vhost: factor out busy polling logic to vhost_net_busy_poll()
On Wed, Jul 4, 2018 at 5:18 PM Jason Wang <jasowang at redhat.com> wrote:> > > > On 2018?07?04? 15:59, Toshiaki Makita wrote: > > On 2018/07/04 13:31, xiangxia.m.yue at gmail.com wrote: > > ... > >> +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); > >> + unsigned long busyloop_timeout; > >> + struct socket *sock; > >> + struct vhost_virtqueue *vq = rx ? tvq : rvq; > >> + > >> + mutex_lock_nested(&vq->mutex, rx ? VHOST_NET_VQ_TX: VHOST_NET_VQ_RX); > >> + > >> + vhost_disable_notify(&net->dev, vq); > >> + sock = rvq->private_data; > >> + busyloop_timeout = rx ? rvq->busyloop_timeout : tvq->busyloop_timeout; > >> + > >> + 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 (vhost_enable_notify(&net->dev, vq) && rx) { > > Hmm... on tx here sock has no rx data, so you are waiting for sock > > wakeup for rx and vhost_enable_notify() seems not needed. Do you want > > this actually? > > > > } else if (rx && vhost_enable_notify(&net->dev, vq)) { > > Right, rx need to be checked first here.thanks? if we dont call the vhost_enable_notify for tx. so we dont need to call vhost_disable_notify for tx? @@ -451,7 +451,9 @@ static void vhost_net_busy_poll(struct vhost_net *net, tvq->busyloop_timeout; mutex_lock_nested(&vq->mutex, rx ? VHOST_NET_VQ_TX: VHOST_NET_VQ_RX); - vhost_disable_notify(&net->dev, vq); + + if (rx) + vhost_disable_notify(&net->dev, vq); preempt_disable(); endtime = busy_clock() + busyloop_timeout;> Thanks > > >> + vhost_disable_notify(&net->dev, vq); > >> + vhost_poll_queue(&vq->poll); > >> + } >
Possibly Parallel Threads
- [PATCH net-next v5 3/4] net: vhost: factor out busy polling logic to vhost_net_busy_poll()
- [PATCH net-next v5 3/4] net: vhost: factor out busy polling logic to vhost_net_busy_poll()
- [PATCH net-next v4 3/4] net: vhost: factor out busy polling logic to vhost_net_busy_poll()
- [PATCH net-next v4 3/4] net: vhost: factor out busy polling logic to vhost_net_busy_poll()
- [PATCH net-next v7 3/4] net: vhost: factor out busy polling logic to vhost_net_busy_poll()