diff mbox

meaningless to compare irqfd's msi message with new msi message in virtio_pci_vq_vector_unmask

Message ID D3E216785288A145B7BC975F83A2ED103FEE587B@szxeml556-mbx.china.huawei.com
State New
Headers show

Commit Message

Zhanghaoyu (A) June 26, 2013, 5:05 a.m. UTC
I searched "vector_irqfd" globally,  no place found to set/change irqfd's msi message, only irqfd's virq or users member may be changed in kvm_virtio_pci_vq_vector_use, 
kvm_virtio_pci_vq_vector_release, etc.
So I think it's meaningless to do below check in virtio_pci_vq_vector_unmask,
if (irqfd->msg.data != msg.data || irqfd->msg.address != msg.address)

And, I think the comparison between old msi message and new msi messge should be performed in kvm_update_routing_entry, the raw patch shown as below,
Signed-off-by: Zhang Haoyu <haoyu.zhang@huawei.com>
Signed-off-by: Zhang Huanzhong <zhanghuanzhong@huawei.com>
---
 hw/virtio/virtio-pci.c |    8 +++-----
 kvm-all.c              |    5 +++++
 2 files changed, 8 insertions(+), 5 deletions(-)

Comments

Zhanghaoyu (A) July 4, 2013, 6:52 a.m. UTC | #1
> I searched "vector_irqfd" globally,  no place found to set/change irqfd's msi message, only irqfd's virq or users member may be changed in kvm_virtio_pci_vq_vector_use, kvm_virtio_pci_vq_vector_release, etc.
> So I think it's meaningless to do below check in virtio_pci_vq_vector_unmask, if (irqfd->msg.data != msg.data || irqfd->msg.address != msg.address)
> 
> And, I think the comparison between old msi message and new msi messge should be performed in kvm_update_routing_entry, the raw patch shown as below,
> Signed-off-by: Zhang Haoyu <haoyu.zhang@huawei.com>
> Signed-off-by: Zhang Huanzhong <zhanghuanzhong@huawei.com>
> ---
>  hw/virtio/virtio-pci.c |    8 +++-----
>  kvm-all.c              |    5 +++++
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index b070b64..e4829a3 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -613,11 +613,9 @@ static int virtio_pci_vq_vector_unmask(VirtIOPCIProxy *proxy,
>  
>      if (proxy->vector_irqfd) {
>          irqfd = &proxy->vector_irqfd[vector];
> -        if (irqfd->msg.data != msg.data || irqfd->msg.address != msg.address) {
> -            ret = kvm_irqchip_update_msi_route(kvm_state, irqfd->virq, msg);
> -            if (ret < 0) {
> -                return ret;
> -            }
> +        ret = kvm_irqchip_update_msi_route(kvm_state, irqfd->virq, msg);
> +        if (ret < 0) {
> +            return ret;
>          }
>      }
>  
> diff --git a/kvm-all.c b/kvm-all.c
> index e6b262f..63a33b4 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1034,6 +1034,11 @@ static int kvm_update_routing_entry(KVMState *s,
>              continue;
>          }
>  
> +        if (entry->type == new_entry->type &&
> +            entry->flags == new_entry->flags &&
> +            !memcmp(&entry->u, &new_entry->u, sizeof(entry->u))) {
> +            return 0;
> +        }
>          entry->type = new_entry->type;
>          entry->flags = new_entry->flags;
>          entry->u = new_entry->u;
> --
> 1.7.3.1.msysgit.0
> 
> 
> This patch works for both virtio-pci device and pci-passthrough device.
> MST and I had been discussed this patch before, this patch can avoid meaninglessly updating the routing entry in kvm hypervisor when new msi message is identical with old msi message, 
> especially in some cases, for example, frequently mask/unmask per-vector masking control bit in ISR on some old linux guest(e.g., rhel-5.5), which gains much.
> At MST's request, the number will be provided later.
>
I started a VM(rhel-5.5) with direct-assigned intel 82599 VF. And, ran iperf-client on the VM, iperf-server on the host where the VM resides, 
so communication between VM and host was switched in the 82599 NIC. The throughput comparison between above patch applied and not shown as below,
before this patch applied:
[ID]   Interval                Transfer              Bandwidth
[SUM]  0.0-10.1 sec            96.5Mbytes            80.1Mbits/sec
after this patch applied:
[ID]   Interval                Transfer              Bandwidth
[SUM]  0.0-10.0 sec            10.9GBytes            9.37Gbits/sec

Then, I ran netperf-client on the VM, netperf-server on the host where the VM resides, the command shown as below
netperf-client: netperf -H [host ip] -l 120 -t TCP_RR -- -m 1024 -r 32,1024
netperf-server: netserver
The transaction rate comparison between above patch applied and not shown as below,
before this patch applied:
Socket    Size       Request     Resp.     Elapsed     Trans.
Send      Recv       Size        Size      Time        Rate
Bytes     Bytes      bytes       bytes     secs.       Per sec
16384     87380      32          1024      120.01      36.61
65536     87380
after this patch applied:
Socket    Size       Request     Resp.     Elapsed     Trans.
Send      Recv       Size        Size      Time        Rate
Bytes     Bytes      bytes       bytes     secs.       Per sec
16384     87380      32          1024      120.01      7464.89
65536     87380

> Thanks,
> Zhang Haoyu
diff mbox

Patch

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index b070b64..e4829a3 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -613,11 +613,9 @@  static int virtio_pci_vq_vector_unmask(VirtIOPCIProxy *proxy,
 
     if (proxy->vector_irqfd) {
         irqfd = &proxy->vector_irqfd[vector];
-        if (irqfd->msg.data != msg.data || irqfd->msg.address != msg.address) {
-            ret = kvm_irqchip_update_msi_route(kvm_state, irqfd->virq, msg);
-            if (ret < 0) {
-                return ret;
-            }
+        ret = kvm_irqchip_update_msi_route(kvm_state, irqfd->virq, msg);
+        if (ret < 0) {
+            return ret;
         }
     }
 
diff --git a/kvm-all.c b/kvm-all.c
index e6b262f..63a33b4 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1034,6 +1034,11 @@  static int kvm_update_routing_entry(KVMState *s,
             continue;
         }
 
+        if (entry->type == new_entry->type &&
+            entry->flags == new_entry->flags &&
+            !memcmp(&entry->u, &new_entry->u, sizeof(entry->u))) {
+            return 0;
+        }
         entry->type = new_entry->type;
         entry->flags = new_entry->flags;
         entry->u = new_entry->u;