search for: vhost_has_work

...insertions(+) diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index eec2f11..d42d11e 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -245,6 +245,12 @@ void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work) } EXPORT_SYMBOL_GPL(vhost_work_queue); +bool vhost_has_work(struct vhost_dev *dev) +{ + return !list_empty(&dev->work_list); +} +EXPORT_SYMBOL_GPL(vhost_has_work); + void vhost_poll_queue(struct vhost_poll *poll) { vhost_work_queue(poll->dev, &poll->work); diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index 4772862..ea0327d...

...insertions(+) diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index eec2f11..d42d11e 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -245,6 +245,12 @@ void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work) } EXPORT_SYMBOL_GPL(vhost_work_queue); +bool vhost_has_work(struct vhost_dev *dev) +{ + return !list_empty(&dev->work_list); +} +EXPORT_SYMBOL_GPL(vhost_has_work); + void vhost_poll_queue(struct vhost_poll *poll) { vhost_work_queue(poll->dev, &poll->work); diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index 4772862..ea0327d...

...t/vhost.c b/drivers/vhost/vhost.c > index eec2f11..d42d11e 100644 > --- a/drivers/vhost/vhost.c > +++ b/drivers/vhost/vhost.c > @@ -245,6 +245,12 @@ void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work) > } > EXPORT_SYMBOL_GPL(vhost_work_queue); > > +bool vhost_has_work(struct vhost_dev *dev) > +{ > + return !list_empty(&dev->work_list); > +} > +EXPORT_SYMBOL_GPL(vhost_has_work); > + > void vhost_poll_queue(struct vhost_poll *poll) > { > vhost_work_queue(poll->dev, &poll->work); This doesn't take a lock so it'...

...ael S. Tsirkin wrote: > On Fri, Jun 29, 2018 at 05:09:50PM +0900, Toshiaki Makita wrote: >> Under heavy load vhost busypoll may run without suppressing >> notification. For example tx zerocopy callback can push tx work while >> handle_tx() is running, then busyloop exits due to vhost_has_work() >> condition and enables notification but immediately reenters handle_tx() >> because the pushed work was tx. In this case handle_tx() tries to >> disable notification again, but when using event_idx it by design >> cannot. Then busyloop will run without suppressing notifi...

...ael S. Tsirkin wrote: > On Fri, Jun 29, 2018 at 05:09:50PM +0900, Toshiaki Makita wrote: >> Under heavy load vhost busypoll may run without suppressing >> notification. For example tx zerocopy callback can push tx work while >> handle_tx() is running, then busyloop exits due to vhost_has_work() >> condition and enables notification but immediately reenters handle_tx() >> because the pushed work was tx. In this case handle_tx() tries to >> disable notification again, but when using event_idx it by design >> cannot. Then busyloop will run without suppressing notifi...

...ers/vhost/net.c b/drivers/vhost/net.c index 07181cd8d52e..8ed63651b9eb 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c @@ -546,7 +546,7 @@ static void vhost_net_busy_poll(struct vhost_net *net, endtime = busy_clock() + busyloop_timeout; while (vhost_can_busy_poll(endtime)) { - if (vhost_has_work(&net->dev)) { + if (vhost_vq_has_work(vq)) { *busyloop_intr = true; break; } diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index e041e116afee..6567aed69ebb 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -262,11 +262,11 @@ void vhost_work_queue(stru...

...>>> Looks like in the last "else" block in vhost_net_busy_poll_check() we >>>>> need to enable vq since in that case we have no rx data and handle_rx() >>>>> is not scheduled. >>>>> >>>> Yes. >>> So we will use the vhost_has_work() to check whether or not the >>> handle_rx is scheduled ? >>> If we use the vhost_has_work(), the work in the dev work_list may be >>> rx work, or tx work, right ? >> >> Yes. We can add a boolean to record whether or not we've called >> vhost_poll_q...

...>>> Looks like in the last "else" block in vhost_net_busy_poll_check() we >>>>> need to enable vq since in that case we have no rx data and handle_rx() >>>>> is not scheduled. >>>>> >>>> Yes. >>> So we will use the vhost_has_work() to check whether or not the >>> handle_rx is scheduled ? >>> If we use the vhost_has_work(), the work in the dev work_list may be >>> rx work, or tx work, right ? >> >> Yes. We can add a boolean to record whether or not we've called >> vhost_poll_q...

Under heavy load vhost busypoll may run without suppressing notification. For example tx zerocopy callback can push tx work while handle_tx() is running, then busyloop exits due to vhost_has_work() condition and enables notification but immediately reenters handle_tx() because the pushed work was tx. In this case handle_tx() tries to disable notification again, but when using event_idx it by design cannot. Then busyloop will run without suppressing notification. Another example is the case...

Under heavy load vhost tx busypoll tend not to suppress vq kicks, which causes poor guest tx performance. The detailed scenario is described in commitlog of patch 2. Rx seems not to have that serious problem, but for consistency I made a similar change on rx to avoid rx wakeups (patch 3). Additionary patch 4 is to avoid rx kicks under heavy load during busypoll. Tx performance is greatly improved

...since handle_rx() will do this for us. >>> Looks like in the last "else" block in vhost_net_busy_poll_check() we >>> need to enable vq since in that case we have no rx data and handle_rx() >>> is not scheduled. >>> >> Yes. > So we will use the vhost_has_work() to check whether or not the > handle_rx is scheduled ? > If we use the vhost_has_work(), the work in the dev work_list may be > rx work, or tx work, right ? Yes. We can add a boolean to record whether or not we've called vhost_poll_queue() for rvq. And avoid calling vhost_net_enabl...

...since handle_rx() will do this for us. >>> Looks like in the last "else" block in vhost_net_busy_poll_check() we >>> need to enable vq since in that case we have no rx data and handle_rx() >>> is not scheduled. >>> >> Yes. > So we will use the vhost_has_work() to check whether or not the > handle_rx is scheduled ? > If we use the vhost_has_work(), the work in the dev work_list may be > rx work, or tx work, right ? Yes. We can add a boolean to record whether or not we've called vhost_poll_queue() for rvq. And avoid calling vhost_net_enabl...

Under heavy load vhost busypoll may run without suppressing notification. For example tx zerocopy callback can push tx work while handle_tx() is running, then busyloop exits due to vhost_has_work() condition and enables notification but immediately reenters handle_tx() because the pushed work was tx. In this case handle_tx() tries to disable notification again, but when using event_idx it by design cannot. Then busyloop will run without suppressing notification. Another example is the case...

Under heavy load vhost busypoll may run without suppressing notification. For example tx zerocopy callback can push tx work while handle_tx() is running, then busyloop exits due to vhost_has_work() condition and enables notification but immediately reenters handle_tx() because the pushed work was tx. In this case handle_tx() tries to disable notification again, but when using event_idx it by design cannot. Then busyloop will run without suppressing notification. Another example is the case...

...08:47:35AM +0800, Jason Wang wrote: > > > On 2018?08?21? 08:33, Jason Wang wrote: > > > > > > On 2018?08?19? 20:11, xiangxia.m.yue at gmail.com wrote: > > > From: Tonghao Zhang <xiangxia.m.yue at gmail.com> > > > > > > The patch uses vhost_has_work_pending() to check if > > > the specified handler is scheduled, because in the most case, > > > vhost_has_work() return true when other side handler is added > > > to worker list. Use the vhost_has_work_pending() insead of > > > vhost_has_work(). > > >...

...n the last "else" block in vhost_net_busy_poll_check() we > >>>>> need to enable vq since in that case we have no rx data and handle_rx() > >>>>> is not scheduled. > >>>>> > >>>> Yes. > >>> So we will use the vhost_has_work() to check whether or not the > >>> handle_rx is scheduled ? > >>> If we use the vhost_has_work(), the work in the dev work_list may be > >>> rx work, or tx work, right ? > >> > >> Yes. We can add a boolean to record whether or not we've cal...

...anges from V2: - poll also at the end of rx handling - factor out the polling logic and optimize the code a little bit - add two ioctls to get and set the busy poll timeout - test on ixgbe (which can give more stable and reproducable numbers) instead of mlx4. Changes from V1: - Add a comment for vhost_has_work() to explain why it could be lockless - Add param description for busyloop_timeout - Split out the busy polling logic into a new helper - Check and exit the loop when there's a pending signal - Disable preemption during busy looping to make sure lock_clock() was correctly used. Jason Wang...

...anges from V2: - poll also at the end of rx handling - factor out the polling logic and optimize the code a little bit - add two ioctls to get and set the busy poll timeout - test on ixgbe (which can give more stable and reproducable numbers) instead of mlx4. Changes from V1: - Add a comment for vhost_has_work() to explain why it could be lockless - Add param description for busyloop_timeout - Split out the busy polling logic into a new helper - Check and exit the loop when there's a pending signal - Disable preemption during busy looping to make sure lock_clock() was correctly used. Jason Wang...

...from RFC V2: - poll also at the end of rx handling - factor out the polling logic and optimize the code a little bit - add two ioctls to get and set the busy poll timeout - test on ixgbe (which can give more stable and reproducable numbers) instead of mlx4. Changes from RFC V1: - add a comment for vhost_has_work() to explain why it could be lockless - add param description for busyloop_timeout - split out the busy polling logic into a new helper - check and exit the loop when there's a pending signal - disable preemption during busy looping to make sure lock_clock() was correctly used. Jason Wang (3):...

...from RFC V2: - poll also at the end of rx handling - factor out the polling logic and optimize the code a little bit - add two ioctls to get and set the busy poll timeout - test on ixgbe (which can give more stable and reproducable numbers) instead of mlx4. Changes from RFC V1: - add a comment for vhost_has_work() to explain why it could be lockless - add param description for busyloop_timeout - split out the busy polling logic into a new helper - check and exit the loop when there's a pending signal - disable preemption during busy looping to make sure lock_clock() was correctly used. Jason Wang (3):...