On 2014/2/25 16:13, Jason Wang wrote:
On 02/25/2014 03:53 PM, Qin Chuanyu wrote:
On 2014/2/25 15:38, Jason Wang wrote:
On 02/25/2014 02:55 PM, Qin Chuanyu wrote:
guest kick vhost base on vring flag status and get perfermance
improved,
vhost_zerocopy_callback could do this in the same way, as
virtqueue_enable_cb need one more check after change the status of
avail_ring flags, vhost also do the same thing after
vhost_enable_notify
test result list as below:
guest and host: suse11sp3, netperf, intel 2.4GHz
+-------+----------+---------+----------+---------+
| | old | new |
+-------+----------+---------+----------+---------+
| UDP | Gbit/s | PPS | Gbit/s | PPS |
| 256 | 0.74805 | 321309 | 0.77933 | 334743 |
| 512 | 1.42 | 328475 | 1.44 | 333550 |
| 1024 | 2.79 | 334426 | 2.81 | 336986 |
| 1460 | 3.71 | 316215 | 4.02 | 342325 |
+-------+----------+---------+----------+---------+
Looks good, do you have cpu utilization number?
+------+----------+--------+----------+--------+--------+---------+
| | old | new |
+------+----------+--------+----------+--------+--------+---------+
| UDP | Gbit/s | PPS |CPU idle% | Gbit/s | PPS |CPU idle%|
| 256 | 0.74805 | 321309 | 87.16 | 0.77933| 334743 | 90.71 |
| 512 | 1.42 | 328475 | 87.03 | 1.44 | 333550 | 90.43 |
| 1024 | 2.79 | 334426 | 89.09 | 2.81 | 336986 | 89.55 |
| 1460 | 3.71 | 316215 | 87.53 | 4.02 | 342325 | 89.58 |
+------+----------+--------+----------+--------+--------+---------+
after change, less cpu has been used.
Thanks
Signed-off-by: Chuanyu Qin <qinchuanyu@xxxxxxxxxx>
---
drivers/vhost/net.c | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index a0fa5de..9bc0a15 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -322,7 +322,9 @@ static void vhost_zerocopy_callback(struct
ubuf_info *ubuf, bool success)
* (the value 16 here is more or less arbitrary, it's tuned to
trigger
* less than 10% of times).
*/
- if (cnt <= 1 || !(cnt % 16))
+ smp_rmb();
Better add a comment to explain why this is needed.
Looks like what you need is a smp_mb() here to make sure the len is
updated before testing vq->used_flags?
I wanner make sure the used_flags is updated, is smp_rmb() enough?
or a smp_mb() is needed?
used_flags was guaranteed to be updated after smp_mb() in
vhost_enable_notify(). And vhost_net_ubuf_put() does a
atomic_sub_return() which implements memory barrier. So looks like
there's no need for an extra one.
atomic_sub_return only do as below:
asm volatile(LOCK_PREFIX "xaddl %0, %1"
: "+r" (i), "+m" (v->counter)
: : "memory");
return i + __i;
google as below:
The "memory" clobber makes GCC assume that any memory may be arbitrarily
read or written by the asm block, so will prevent the compiler from
reordering loads or stores across it:
This will cause GCC to not keep memory values cached in registers
across the assembler instruction and not optimize stores or loads to
that memory.
(That does not prevent a CPU from reordering loads and stores with
respect to another CPU, though; you need real memory barrier
instructions for that.)
-----------------------------------------------------------------
vhost_zerocopy_callback might be involved in softirq context in another
cpu ,so I think smp_rmb() is still needed, is it right ?
+ if ((!(vq->used_flags & VRING_USED_F_NO_NOTIFY))
+ && (cnt <= 1 || !(cnt % 16)))
vhost_poll_queue(&vq->poll);
rcu_read_unlock_bh();
@@ -386,6 +388,12 @@ static void handle_tx(struct vhost_net *net)
vhost_disable_notify(&net->dev, vq);
continue;
}
+ /* there might skb been freed between last
+ * vhost_zerocopy_signal_used and vhost_enable_notify,
+ * so one more check is needed.
+ */
+ if (zcopy)
+ vhost_zerocopy_signal_used(net, vq);
break;
}
if (in) {
.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
