Jason Wang
2015-Oct-22 05:27 UTC
[PATCH net-next RFC 1/2] vhost: introduce vhost_has_work()
This path introduces a helper which can give a hint for whether or not there's a work queued in the work list. Signed-off-by: Jason Wang <jasowang at redhat.com> --- drivers/vhost/vhost.c | 6 ++++++ drivers/vhost/vhost.h | 1 + 2 files changed, 7 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 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -37,6 +37,7 @@ struct vhost_poll { void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn); void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work); +bool vhost_has_work(struct vhost_dev *dev); void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn, unsigned long mask, struct vhost_dev *dev); -- 1.8.3.1
Jason Wang
2015-Oct-22 05:27 UTC
[PATCH net-next RFC 2/2] vhost_net: basic polling support
This patch tries to poll for new added tx buffer for a while at the end of tx processing. The maximum time spent on polling were limited through a module parameter. To avoid block rx, the loop will end it there's new other works queued on vhost so in fact socket receive queue is also be polled. busyloop_timeout = 50 gives us following improvement on TCP_RR test: size/session/+thu%/+normalize% 1/ 1/ +5%/ -20% 1/ 50/ +17%/ +3% Signed-off-by: Jason Wang <jasowang at redhat.com> --- drivers/vhost/net.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c index 9eda69e..bbb522a 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c @@ -31,7 +31,9 @@ #include "vhost.h" static int experimental_zcopytx = 1; +static int busyloop_timeout = 50; module_param(experimental_zcopytx, int, 0444); +module_param(busyloop_timeout, int, 0444); MODULE_PARM_DESC(experimental_zcopytx, "Enable Zero Copy TX;" " 1 -Enable; 0 - Disable"); @@ -287,12 +289,23 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success) rcu_read_unlock_bh(); } +static bool tx_can_busy_poll(struct vhost_dev *dev, + unsigned long endtime) +{ + unsigned long now = local_clock() >> 10; + + return busyloop_timeout && !need_resched() && + !time_after(now, endtime) && !vhost_has_work(dev) && + single_task_running(); +} + /* Expects to be always run from workqueue - which acts as * read-size critical section for our kind of RCU. */ static void handle_tx(struct vhost_net *net) { struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX]; struct vhost_virtqueue *vq = &nvq->vq; + unsigned long endtime; unsigned out, in; int head; struct msghdr msg = { @@ -331,6 +344,8 @@ static void handle_tx(struct vhost_net *net) % UIO_MAXIOV == nvq->done_idx)) break; + endtime = (local_clock() >> 10) + busyloop_timeout; +again: head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov), &out, &in, @@ -340,6 +355,10 @@ static void handle_tx(struct vhost_net *net) break; /* Nothing new? Wait for eventfd to tell us they refilled. */ if (head == vq->num) { + if (tx_can_busy_poll(vq->dev, endtime)) { + cpu_relax(); + goto again; + } if (unlikely(vhost_enable_notify(&net->dev, vq))) { vhost_disable_notify(&net->dev, vq); continue; -- 1.8.3.1
Michael S. Tsirkin
2015-Oct-22 08:38 UTC
[PATCH net-next RFC 1/2] vhost: introduce vhost_has_work()
On Thu, Oct 22, 2015 at 01:27:28AM -0400, Jason Wang wrote:> This path introduces a helper which can give a hint for whether or not > there's a work queued in the work list. > > Signed-off-by: Jason Wang <jasowang at redhat.com> > --- > drivers/vhost/vhost.c | 6 ++++++ > drivers/vhost/vhost.h | 1 + > 2 files changed, 7 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);This doesn't take a lock so it's unreliable. I think it's ok in this case since it's just an optimization - but pls document this.> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h > index 4772862..ea0327d 100644 > --- a/drivers/vhost/vhost.h > +++ b/drivers/vhost/vhost.h > @@ -37,6 +37,7 @@ struct vhost_poll { > > void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn); > void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work); > +bool vhost_has_work(struct vhost_dev *dev); > > void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn, > unsigned long mask, struct vhost_dev *dev); > -- > 1.8.3.1
Michael S. Tsirkin
2015-Oct-22 09:33 UTC
[PATCH net-next RFC 2/2] vhost_net: basic polling support
On Thu, Oct 22, 2015 at 01:27:29AM -0400, Jason Wang wrote:> This patch tries to poll for new added tx buffer for a while at the > end of tx processing. The maximum time spent on polling were limited > through a module parameter. To avoid block rx, the loop will end it > there's new other works queued on vhost so in fact socket receive > queue is also be polled. > > busyloop_timeout = 50 gives us following improvement on TCP_RR test: > > size/session/+thu%/+normalize% > 1/ 1/ +5%/ -20% > 1/ 50/ +17%/ +3%Is there a measureable increase in cpu utilization with busyloop_timeout = 0?> Signed-off-by: Jason Wang <jasowang at redhat.com>We might be able to shave off the minor regression by careful use of likely/unlikely, or maybe deferring> --- > drivers/vhost/net.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c > index 9eda69e..bbb522a 100644 > --- a/drivers/vhost/net.c > +++ b/drivers/vhost/net.c > @@ -31,7 +31,9 @@ > #include "vhost.h" > > static int experimental_zcopytx = 1; > +static int busyloop_timeout = 50; > module_param(experimental_zcopytx, int, 0444); > +module_param(busyloop_timeout, int, 0444);Pls add a description, including the units and the special value 0.> MODULE_PARM_DESC(experimental_zcopytx, "Enable Zero Copy TX;" > " 1 -Enable; 0 - Disable"); > > @@ -287,12 +289,23 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success) > rcu_read_unlock_bh(); > } > > +static bool tx_can_busy_poll(struct vhost_dev *dev, > + unsigned long endtime) > +{ > + unsigned long now = local_clock() >> 10;local_clock might go backwards if we jump between CPUs. One way to fix would be to record the CPU id and break out of loop if that changes. Also - defer this until we actually know we need it?> + > + return busyloop_timeout && !need_resched() && > + !time_after(now, endtime) && !vhost_has_work(dev) && > + single_task_running();signal pending as well?> +} > + > /* Expects to be always run from workqueue - which acts as > * read-size critical section for our kind of RCU. */ > static void handle_tx(struct vhost_net *net) > { > struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX]; > struct vhost_virtqueue *vq = &nvq->vq; > + unsigned long endtime; > unsigned out, in; > int head; > struct msghdr msg = { > @@ -331,6 +344,8 @@ static void handle_tx(struct vhost_net *net) > % UIO_MAXIOV == nvq->done_idx)) > break; > > + endtime = (local_clock() >> 10) + busyloop_timeout; > +again: > head = vhost_get_vq_desc(vq, vq->iov, > ARRAY_SIZE(vq->iov), > &out, &in, > @@ -340,6 +355,10 @@ static void handle_tx(struct vhost_net *net) > break; > /* Nothing new? Wait for eventfd to tell us they refilled. */ > if (head == vq->num) { > + if (tx_can_busy_poll(vq->dev, endtime)) { > + cpu_relax(); > + goto again; > + } > if (unlikely(vhost_enable_notify(&net->dev, vq))) { > vhost_disable_notify(&net->dev, vq); > continue; > -- > 1.8.3.1
Jason Wang
2015-Oct-23 07:10 UTC
[PATCH net-next RFC 1/2] vhost: introduce vhost_has_work()
On 10/22/2015 04:38 PM, Michael S. Tsirkin wrote:> On Thu, Oct 22, 2015 at 01:27:28AM -0400, Jason Wang wrote: >> > This path introduces a helper which can give a hint for whether or not >> > there's a work queued in the work list. >> > >> > Signed-off-by: Jason Wang <jasowang at redhat.com> >> > --- >> > drivers/vhost/vhost.c | 6 ++++++ >> > drivers/vhost/vhost.h | 1 + >> > 2 files changed, 7 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); > This doesn't take a lock so it's unreliable. > I think it's ok in this case since it's just > an optimization - but pls document this. >Ok, will do.
Possibly Parallel Threads
- [PATCH net-next RFC 1/2] vhost: introduce vhost_has_work()
- [PATCH net-next RFC 2/2] vhost_net: basic polling support
- [PATCH net-next RFC 2/2] vhost_net: basic polling support
- [PATCH net-next rfc V2 0/2] basic busy polling support for vhost_net
- [PATCH net-next rfc V2 0/2] basic busy polling support for vhost_net