Message ID | 20090806163549.GA25594@lst.de |
---|---|
State | Superseded |
Headers | show |
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?
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.
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?
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?
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
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.
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.
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)))