Re: [PATCH V2 2/2] vhost_net: conditionally enable tx polling
On 2016年05月30日 23:55, Michael S. Tsirkin wrote:
On Mon, May 30, 2016 at 02:47:54AM -0400, Jason Wang wrote:
We always poll tx for socket, this is sub optimal since:
- it will be only used when we exceed the sndbuf of the socket.
- since we use two independent polls for tx and vq, this will slightly
increase the waitqueue traversing time and more important, vhost
could not benefit from commit
9e641bdcfa4ef4d6e2fbaa59c1be0ad5d1551fd5 ("net-tun: restructure
tun_do_read for better sleep/wakeup efficiency") even if we've
stopped rx polling during handle_rx since tx poll were still left in
the waitqueue.
Why is this an issue?
sock_def_write_space only wakes up when queue is half empty,
not on each packet.
if ((atomic_read(>sk_wmem_alloc) << 1) <= sk->sk_sndbuf)
I suspect the issue is with your previous patch,
it now pokes at the spinlock on data path
where it used not to.
Is that right?
The problem is not tx wake up but still rx wake up. Patch 1 removes rx
poll, but still left tx poll. So in sock_def_readable(),
skwq_has_sleeper() returns true, we still need to traverse waitqueue and
touch spinlocks. With this patch, unless a heavy tx load, tx poll were
disabled, sock_def_readable() can return finish very soon.
Fix this by conditionally enable tx polling only when -EAGAIN were
met.
Test shows about 8% improvement on guest rx pps.
Before: ~135
After: ~146
Signed-off-by: Jason Wang
---
drivers/vhost/net.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index e91603b..5a05fa0 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -378,6 +378,7 @@ static void handle_tx(struct vhost_net *net)
goto out;
vhost_disable_notify(>dev, vq);
+ vhost_net_disable_vq(net, vq);
hdr_size = nvq->vhost_hlen;
zcopy = nvq->ubufs;
@@ -459,6 +460,8 @@ static void handle_tx(struct vhost_net *net)
% UIO_MAXIOV;
}
vhost_discard_vq_desc(vq, 1);
+ if (err == -EAGAIN)
+ vhost_net_enable_vq(net, vq);
break;
}
if (err != len)
--
1.8.3.1
Re: [PATCH V2 2/2] vhost_net: conditionally enable tx polling
On Mon, May 30, 2016 at 02:47:54AM -0400, Jason Wang wrote:
> We always poll tx for socket, this is sub optimal since:
>
> - it will be only used when we exceed the sndbuf of the socket.
> - since we use two independent polls for tx and vq, this will slightly
> increase the waitqueue traversing time and more important, vhost
> could not benefit from commit
> 9e641bdcfa4ef4d6e2fbaa59c1be0ad5d1551fd5 ("net-tun: restructure
> tun_do_read for better sleep/wakeup efficiency") even if we've
> stopped rx polling during handle_rx since tx poll were still left in
> the waitqueue.
Why is this an issue?
sock_def_write_space only wakes up when queue is half empty,
not on each packet.
if ((atomic_read(>sk_wmem_alloc) << 1) <= sk->sk_sndbuf)
I suspect the issue is with your previous patch,
it now pokes at the spinlock on data path
where it used not to.
Is that right?
>
> Fix this by conditionally enable tx polling only when -EAGAIN were
> met.
>
> Test shows about 8% improvement on guest rx pps.
>
> Before: ~135
> After: ~146
>
> Signed-off-by: Jason Wang
> ---
> drivers/vhost/net.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index e91603b..5a05fa0 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -378,6 +378,7 @@ static void handle_tx(struct vhost_net *net)
> goto out;
>
> vhost_disable_notify(>dev, vq);
> + vhost_net_disable_vq(net, vq);
>
> hdr_size = nvq->vhost_hlen;
> zcopy = nvq->ubufs;
> @@ -459,6 +460,8 @@ static void handle_tx(struct vhost_net *net)
> % UIO_MAXIOV;
> }
> vhost_discard_vq_desc(vq, 1);
> + if (err == -EAGAIN)
> + vhost_net_enable_vq(net, vq);
> break;
> }
> if (err != len)
> --
> 1.8.3.1
[PATCH V2 2/2] vhost_net: conditionally enable tx polling
We always poll tx for socket, this is sub optimal since:
- it will be only used when we exceed the sndbuf of the socket.
- since we use two independent polls for tx and vq, this will slightly
increase the waitqueue traversing time and more important, vhost
could not benefit from commit
9e641bdcfa4ef4d6e2fbaa59c1be0ad5d1551fd5 ("net-tun: restructure
tun_do_read for better sleep/wakeup efficiency") even if we've
stopped rx polling during handle_rx since tx poll were still left in
the waitqueue.
Fix this by conditionally enable tx polling only when -EAGAIN were
met.
Test shows about 8% improvement on guest rx pps.
Before: ~135
After: ~146
Signed-off-by: Jason Wang
---
drivers/vhost/net.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index e91603b..5a05fa0 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -378,6 +378,7 @@ static void handle_tx(struct vhost_net *net)
goto out;
vhost_disable_notify(>dev, vq);
+ vhost_net_disable_vq(net, vq);
hdr_size = nvq->vhost_hlen;
zcopy = nvq->ubufs;
@@ -459,6 +460,8 @@ static void handle_tx(struct vhost_net *net)
% UIO_MAXIOV;
}
vhost_discard_vq_desc(vq, 1);
+ if (err == -EAGAIN)
+ vhost_net_enable_vq(net, vq);
break;
}
if (err != len)
--
1.8.3.1
