Patchwork virtio-blk performance and MSI

login
register
mail settings
Submitter Christoph Hellwig
Date Aug. 6, 2009, 4:35 p.m.
Message ID <20090806163549.GA25594@lst.de>
Download mbox | patch
Permalink /patch/30865/
State Superseded
Headers show

Comments

Christoph Hellwig - Aug. 6, 2009, 4:35 p.m.
Michael suggested to me a while ago to try MSI with virtio-blk and I
played with this small patch:


 
which gave about 5% speedups on 4k sized reads and writes, see the full
iozone output I attached.  Now getting the information about using
multiple MSI vectors from the command line to virtio-blk similar to how
virtio-net does seems extremly messy right now.  Waiting for Gerd's
additional qdev patches to make it easier as a qdev property.
File size set to 131072 KB
	Record Size 4 KB
	O_DIRECT feature enabled
	Command line used: iozone -s 128m -r 4k -I -f /dev/sdb
	Output is in Kbytes/sec
	Time Resolution = 0.000001 seconds.
	Processor cache size set to 1024 Kbytes.
	Processor cache line size set to 32 bytes.
	File stride size set to 17 * record size.
                                                            random  random    bkwd   record   stride                                   
              KB  reclen   write rewrite    read    reread    read   write    read  rewrite     read   fwrite frewrite   fread  freread
native    131072       4   11428   11847    25110    25302   24415   10904   25051    12146    24859   978572  1224433 1974096  2086567
          131072       4   11812   11784    24941    25209   24325   10776   24814    12271    24907   959819  1208770 1977191  2103138
          131072       4   11834   11892    25270    25347   24427   10839   24571    12161    24558   958934  1213707 1959754  2100647
          131072       4   11688   11914    25102    25100   24514   10855   24787    12237    24738   987739  1218774 1985245  2085435
          131072       4   11768   11910    24986    25087   24342   10819   24687    12304    24711   974889  1221511 2027124  2102430
qemu      131072       4    8752    9137    14020    14181   13924    8491   14158     8215    13816   378448  1498838 2117166  2341281
          131072       4    9113    9097    14019    14187   14024    8536   14153     8243    14132  1194485  1506540 2053520  2333202
          131072       4    9082    9128    14001    14232   13971    8541   14113     8216    14103  1260659  1464543 2101490  2335442
          131072       4    9103    9163    14373    14149   13983    8523   14171     8242    14026  1278104  1503047 2127449  2334738
          131072       4    9084    9128    14103    14212   13980    8519   14064     8260    13810  1204696  1497434 2053129  2334362
qemu+msi  131072       4    9466    9726    15339    15225   14845    8884   15159     8631    14460   375140  1488522 2066115  2337399
          131072       4    9541    9590    15025    15059   15010    8852   15007     8677    14736  1142718  1491640 2111847  2332153
          131072       4    9492    9621    14831    15093   14792    8895   14849     8452    14976  1163760  1461825 2118741  2337985
          131072       4    9519    9615    14954    14950   14713    8915   15229     8547    14854  1212529  1490471 2091894  2343676
          131072       4    9527    9576    14872    14828   14741    8880   14891     8769    14502  1253559  1436703 2127827  2344256
Avi Kivity - Aug. 9, 2009, 10:01 a.m.
On 08/06/2009 07:35 PM, Christoph Hellwig wrote:
> Michael suggested to me a while ago to try MSI with virtio-blk and I
> played with this small patch:
>
>
> Index: qemu-kvm/hw/virtio-blk.c
> ===================================================================
> --- qemu-kvm.orig/hw/virtio-blk.c
> +++ qemu-kvm/hw/virtio-blk.c
> @@ -416,6 +416,7 @@ VirtIODevice *virtio_blk_init(DeviceStat
>       s->vdev.get_config = virtio_blk_update_config;
>       s->vdev.get_features = virtio_blk_get_features;
>       s->vdev.reset = virtio_blk_reset;
> +    s->vdev.nvectors = 2;
>       s->bs = bs;
>       s->rq = NULL;
>       if (strlen(ps = (char *)drive_get_serial(bs)))
>
> which gave about 5% speedups on 4k sized reads and writes, see the full
> iozone output I attached.  Now getting the information about using
> multiple MSI vectors from the command line to virtio-blk similar to how
> virtio-net does seems extremly messy right now.  Waiting for Gerd's
> additional qdev patches to make it easier as a qdev property.
>
>    

Looks good.  Anthony, I think this applies upstream?
Christoph Hellwig - Aug. 9, 2009, 5:41 p.m.
On Sun, Aug 09, 2009 at 01:01:35PM +0300, Avi Kivity wrote:
> Looks good.  Anthony, I think this applies upstream?

This applies upstream, but as mentioned we can't just use it as-is.
We'll very recent kvm kernel support for multiple MSI vectors, and
when the host doesn't have it even 2.6.30 crashes badly in the guest.
Avi Kivity - Aug. 9, 2009, 5:49 p.m.
On 08/09/2009 08:41 PM, Christoph Hellwig wrote:
> On Sun, Aug 09, 2009 at 01:01:35PM +0300, Avi Kivity wrote:
>    
>> Looks good.  Anthony, I think this applies upstream?
>>      
>
> This applies upstream, but as mentioned we can't just use it as-is.
> We'll very recent kvm kernel support for multiple MSI vectors, and
> when the host doesn't have it even 2.6.30 crashes badly in the guest.
>    

Sorry, confused.  Upstream doesn't use kvm for msi (everything's done in 
userspace).  What exactly blocks us here?
Christoph Hellwig - Aug. 9, 2009, 7:13 p.m.
On Sun, Aug 09, 2009 at 08:49:23PM +0300, Avi Kivity wrote:
> Sorry, confused.  Upstream doesn't use kvm for msi (everything's done in 
> userspace).  What exactly blocks us here?

Hmm, good question.  Michael, do you know if we can just enabled
MSI unconditionally in upstream qemu, or are there guests that can't
cope with it?
Michael S. Tsirkin - Aug. 9, 2009, 7:49 p.m.
On Sun, Aug 09, 2009 at 07:41:00PM +0200, Christoph Hellwig wrote:
> On Sun, Aug 09, 2009 at 01:01:35PM +0300, Avi Kivity wrote:
> > Looks good.  Anthony, I think this applies upstream?
> 
> This applies upstream, but as mentioned we can't just use it as-is.
> We'll very recent kvm kernel support for multiple MSI vectors, and
> when the host doesn't have it even 2.6.30 crashes badly in the guest.

I think the problems were in qemu-kvm and that they are fixed now.
Could you verify please?

> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael S. Tsirkin - Aug. 9, 2009, 7:53 p.m.
On Sun, Aug 09, 2009 at 09:13:13PM +0200, Christoph Hellwig wrote:
> On Sun, Aug 09, 2009 at 08:49:23PM +0300, Avi Kivity wrote:
> > Sorry, confused.  Upstream doesn't use kvm for msi (everything's done in 
> > userspace).  What exactly blocks us here?
> 
> Hmm, good question.  Michael, do you know if we can just enabled
> MSI unconditionally in upstream qemu, or are there guests that can't
> cope with it?

Upstream or not, block should request MSI by default, the code in hw/msi
checks that host kernel can support it and only enables when it's safe.

And AFAIK all guests can cope with it but only if they are rebooted.
So we must have a flag to change the # of vectors, to have ability
to load images and migrate from state stored by old qemu.
And of course this option is also useful for troubleshooting.
Michael S. Tsirkin - Aug. 9, 2009, 7:56 p.m.
On Sun, Aug 09, 2009 at 01:01:35PM +0300, Avi Kivity wrote:
> On 08/06/2009 07:35 PM, Christoph Hellwig wrote:
>> Michael suggested to me a while ago to try MSI with virtio-blk and I
>> played with this small patch:
>>
>>
>> Index: qemu-kvm/hw/virtio-blk.c
>> ===================================================================
>> --- qemu-kvm.orig/hw/virtio-blk.c
>> +++ qemu-kvm/hw/virtio-blk.c
>> @@ -416,6 +416,7 @@ VirtIODevice *virtio_blk_init(DeviceStat
>>       s->vdev.get_config = virtio_blk_update_config;
>>       s->vdev.get_features = virtio_blk_get_features;
>>       s->vdev.reset = virtio_blk_reset;
>> +    s->vdev.nvectors = 2;

some whitespace damage btw

>>       s->bs = bs;
>>       s->rq = NULL;
>>       if (strlen(ps = (char *)drive_get_serial(bs)))
>>
>> which gave about 5% speedups on 4k sized reads and writes, see the full
>> iozone output I attached.  Now getting the information about using
>> multiple MSI vectors from the command line to virtio-blk similar to how
>> virtio-net does seems extremly messy right now.  Waiting for Gerd's
>> additional qdev patches to make it easier as a qdev property.
>>
>>    
>
> Looks good.  Anthony, I think this applies upstream?

This applies upstream, but we also need the flag to change # of vectors:
for loading old images, but also for troubleshooting.
Thus the qdev dependency.

Patch

Index: qemu-kvm/hw/virtio-blk.c
===================================================================
--- qemu-kvm.orig/hw/virtio-blk.c
+++ qemu-kvm/hw/virtio-blk.c
@@ -416,6 +416,7 @@  VirtIODevice *virtio_blk_init(DeviceStat
     s->vdev.get_config = virtio_blk_update_config;
     s->vdev.get_features = virtio_blk_get_features;
     s->vdev.reset = virtio_blk_reset;
+    s->vdev.nvectors = 2;
     s->bs = bs;
     s->rq = NULL;
     if (strlen(ps = (char *)drive_get_serial(bs)))