Re: QEMU/KVM: virtio-blk asynchronous IO

[Anthony Liguori <aliguori@us.ibm.com>]

Marcelo Tosatti wrote:
> Use asynchronous IO in the virtio block QEMU driver.
>
> virtio_blk_handle_output should not block for long periods, since it
> holds the mutex lock prohibiting other vcpu's from doing IO to QEMU
> devices. Without AIO write intensive benchmarks make guests hang for
> several seconds. Write performance also increases significantly.
>
> Also report errors properly.
>
> To take full advantage of parallel IO we need to allow for more than AIO
> thread per-fd, or use direct IO (-nocache) which uses kernel AIO.
>
> Separate patch allows virtio-block guest driver to queue more than one
> element in the virtio ring.
>
> Anthony, this patch abuses the virtqueue_push() interface by passing a
> VirtQueueElement with only "index" member valid, since we know this is
> all it uses. Doing so avoids allocation, zeroing and copy of an entire
> VirtQueueElement structure. What do you say?
>   

I've got a virtio patch series that changes the virtqueue_pop() 
interface to return a pointer to a VirtQueueElement which I believe 
addresses your use-case.

I can just fold your patch into my series (I'll be sending it out this 
afternoon).

Regards,

Anthony Liguori

> Index: kvm-userspace.io/qemu/hw/virtio-blk.c
> ===================================================================
> --- kvm-userspace.io.orig/qemu/hw/virtio-blk.c
> +++ kvm-userspace.io/qemu/hw/virtio-blk.c
> @@ -77,53 +77,100 @@ static VirtIOBlock *to_virtio_blk(VirtIO
>      return (VirtIOBlock *)vdev;
>  }
>
> +typedef struct VirtIOBlockReq
> +{
> +    VirtIODevice *vdev;
> +    VirtQueue *vq;
> +    struct iovec in_sg_status;
> +    unsigned int pending;
> +    unsigned int len;
> +    unsigned int elem_idx;
> +    int status;
> +} VirtIOBlockReq;
> +
> +static void virtio_blk_rw_complete(void *opaque, int ret)
> +{
> +    VirtIOBlockReq *req = opaque;
> +    struct virtio_blk_inhdr *in;
> +    VirtQueueElement elem;
> +
> +    req->status |= ret;
> +    if (--req->pending > 0)
> +        return;
> +
> +    elem.index = req->elem_idx;
> +    in = (void *)req->in_sg_status.iov_base;
> +
> +    in->status = req->status ? VIRTIO_BLK_S_IOERR : VIRTIO_BLK_S_OK;
> +    virtqueue_push(req->vq, &elem, req->len);
> +    virtio_notify(req->vdev, req->vq);
> +    qemu_free(req);
> +}
> +
>  static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq)
>  {
>      VirtIOBlock *s = to_virtio_blk(vdev);
>      VirtQueueElement elem;
> +    VirtIOBlockReq *req;
>      unsigned int count;
>
>      while ((count = virtqueue_pop(vq, &elem)) != 0) {
>  	struct virtio_blk_inhdr *in;
>  	struct virtio_blk_outhdr *out;
> -	unsigned int wlen;
>  	off_t off;
>  	int i;
>
> +	/*
> +	 * FIXME: limit the number of in-flight requests
> +	 */
> +	req = qemu_malloc(sizeof(VirtIOBlockReq));
> +	if (!req)
> +	return;
> +	memset(req, 0, sizeof(*req));
> +	memcpy(&req->in_sg_status, &elem.in_sg[elem.in_num - 1],
> +	       sizeof(req->in_sg_status));
> +	req->vdev = vdev;
> +	req->vq = vq;
> +	req->elem_idx = elem.index;
> +
>  	out = (void *)elem.out_sg[0].iov_base;
>  	in = (void *)elem.in_sg[elem.in_num - 1].iov_base;
>  	off = out->sector;
>
>  	if (out->type & VIRTIO_BLK_T_SCSI_CMD) {
> -	    wlen = sizeof(*in);
> +	    unsigned int len = sizeof(*in);
> +
>  	    in->status = VIRTIO_BLK_S_UNSUPP;
> +	    virtqueue_push(vq, &elem, len);
> +	    virtio_notify(vdev, vq);
> +	    qemu_free(req);
> +
>  	} else if (out->type & VIRTIO_BLK_T_OUT) {
> -	    wlen = sizeof(*in);
> +	    req->pending = elem.out_num - 1;
>
>  	    for (i = 1; i < elem.out_num; i++) {
> -		bdrv_write(s->bs, off,
> +		bdrv_aio_write(s->bs, off,
>  			   elem.out_sg[i].iov_base,
> -			   elem.out_sg[i].iov_len / 512);
> +			   elem.out_sg[i].iov_len / 512,
> +			   virtio_blk_rw_complete,
> +			   req);
>  		off += elem.out_sg[i].iov_len / 512;
> +		req->len += elem.out_sg[i].iov_len;
>  	    }
>
> -	    in->status = VIRTIO_BLK_S_OK;
>  	} else {
> -	    wlen = sizeof(*in);
> +	    req->pending = elem.in_num - 1;
>
>  	    for (i = 0; i < elem.in_num - 1; i++) {
> -		bdrv_read(s->bs, off,
> +		bdrv_aio_read(s->bs, off,
>  			  elem.in_sg[i].iov_base,
> -			  elem.in_sg[i].iov_len / 512);
> +			  elem.in_sg[i].iov_len / 512,
> +			  virtio_blk_rw_complete,
> +			  req);
>  		off += elem.in_sg[i].iov_len / 512;
> -		wlen += elem.in_sg[i].iov_len;
> +		req->len += elem.in_sg[i].iov_len;
>  	    }
> -
> -	    in->status = VIRTIO_BLK_S_OK;
>  	}
> -
> -	virtqueue_push(vq, &elem, wlen);
> -	virtio_notify(vdev, vq);
>      }
>  }
>
>   


-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace