[RFC PATCH 0/2] block: virtio-blk: support multi vq per virtio-blk

[RFC PATCH 0/2] block: virtio-blk: support multi vq per virtio-blk

[Ming Lei]

Hi,

This patches try to support multi virtual queues(multi-vq) in one
virtio-blk device, and maps each virtual queue(vq) to blk-mq's
hardware queue.

With this approach, both scalability and performance problems on
virtio-blk device get improved.

For verifying the improvement, I implements virtio-blk multi-vq over
qemu's dataplane feature, and both handling host notification
from each vq and processing host I/O are still kept in the per-device
iothread context, the changes are based on qemu v2.0.0 release, and
can be accessed from below tree:

	git://kernel.ubuntu.com/ming/qemu.git #v2.0.0-virtblk-dataplane-mq

For enabling the multi-vq feature, 'num_queues=N' need to be added into
'-device virtio-blk-pci ...' of qemu command line, and suggest to pass
'vectors=N+1' to keep one MSI irq vector per each vq, and the feature
depends on x-data-plane.

Fio(libaio, randread, iodepth=64, bs=4K, jobs=N) is run inside VM to
verify the improvement.

I just create a small quadcore VM and run fio inside the VM, and
num_queues of the virtio-blk device is set as 2, but looks the
improvement is still obvious.

1), about scalability
- without mutli-vq feature
	-- jobs=2, thoughput: 145K iops
	-- jobs=4, thoughput: 100K iops
- without mutli-vq feature
	-- jobs=2, thoughput: 186K iops
	-- jobs=4, thoughput: 199K iops

2), about thoughput
- without mutli-vq feature
	-- top thoughput: 145K iops
- with mutli-vq feature
    -- top thoughput: 199K iops

So even for one quadcore VM, if the virtqueue number is increased
from 1 to 2, both scalability and performance can get improved a
lot.

Thanks,
--
Ming Lei
 

[RFC PATCH 0/2] block: virtio-blk: support multi vq per virtio-blk

[Ming Lei]

Hi,

This patches try to support multi virtual queues(multi-vq) in one
virtio-blk device, and maps each virtual queue(vq) to blk-mq's
hardware queue.

With this approach, both scalability and performance problems on
virtio-blk device get improved.

For verifying the improvement, I implements virtio-blk multi-vq over
qemu's dataplane feature, and both handling host notification
from each vq and processing host I/O are still kept in the per-device
iothread context, the changes are based on qemu v2.0.0 release, and
can be accessed from below tree:

	git://kernel.ubuntu.com/ming/qemu.git #v2.0.0-virtblk-dataplane-mq

For enabling the multi-vq feature, 'num_queues=N' need to be added into
'-device virtio-blk-pci ...' of qemu command line, and suggest to pass
'vectors=N+1' to keep one MSI irq vector per each vq, and the feature
depends on x-data-plane.

Fio(libaio, randread, iodepth=64, bs=4K, jobs=N) is run inside VM to
verify the improvement.

I just create a small quadcore VM and run fio inside the VM, and
num_queues of the virtio-blk device is set as 2, but looks the
improvement is still obvious.

1), about scalability
- without mutli-vq feature
	-- jobs=2, thoughput: 145K iops
	-- jobs=4, thoughput: 100K iops
- without mutli-vq feature
	-- jobs=2, thoughput: 186K iops
	-- jobs=4, thoughput: 199K iops

2), about thoughput
- without mutli-vq feature
	-- top thoughput: 145K iops
- with mutli-vq feature
    -- top thoughput: 199K iops

So even for one quadcore VM, if the virtqueue number is increased
from 1 to 2, both scalability and performance can get improved a
lot.

Thanks,
--
Ming Lei

[RFC PATCH 1/2] include/uapi/linux/virtio_blk.h: introduce feature of VIRTIO_BLK_F_MQ

[Ming Lei]

Current virtio-blk spec only supports one virtual queue for transfering
data between VM and host, and inside VM all kinds of operations on
the virtual queue needs to hold one lock, so cause below problems:

	- no scalability
    - bad throughput

So this patch requests to introduce feature of VIRTIO_BLK_F_MQ
so that more than one virtual queues can be used to virtio-blk
device, then above problems can be solved or eased.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 include/uapi/linux/virtio_blk.h |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/uapi/linux/virtio_blk.h b/include/uapi/linux/virtio_blk.h
index 6d8e61c..c5a2751 100644
--- a/include/uapi/linux/virtio_blk.h
+++ b/include/uapi/linux/virtio_blk.h
@@ -40,6 +40,7 @@
 #define VIRTIO_BLK_F_WCE	9	/* Writeback mode enabled after reset */
 #define VIRTIO_BLK_F_TOPOLOGY	10	/* Topology information is available */
 #define VIRTIO_BLK_F_CONFIG_WCE	11	/* Writeback mode available in config */
+#define VIRTIO_BLK_F_MQ		12	/* support more than one vq */
 
 #ifndef __KERNEL__
 /* Old (deprecated) name for VIRTIO_BLK_F_WCE. */
@@ -77,6 +78,9 @@ struct virtio_blk_config {
 
 	/* writeback mode (if VIRTIO_BLK_F_CONFIG_WCE) */
 	__u8 wce;
+
+	/* number of vqs, only available when VIRTIO_BLK_F_MQ is set */
+	__u16 num_queues;
 } __attribute__((packed));
 
 /*
-- 
1.7.9.5

[RFC PATCH 1/2] include/uapi/linux/virtio_blk.h: introduce feature of VIRTIO_BLK_F_MQ

[Ming Lei]

Current virtio-blk spec only supports one virtual queue for transfering
data between VM and host, and inside VM all kinds of operations on
the virtual queue needs to hold one lock, so cause below problems:

	- no scalability
    - bad throughput

So this patch requests to introduce feature of VIRTIO_BLK_F_MQ
so that more than one virtual queues can be used to virtio-blk
device, then above problems can be solved or eased.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 include/uapi/linux/virtio_blk.h |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/uapi/linux/virtio_blk.h b/include/uapi/linux/virtio_blk.h
index 6d8e61c..c5a2751 100644
--- a/include/uapi/linux/virtio_blk.h
+++ b/include/uapi/linux/virtio_blk.h
@@ -40,6 +40,7 @@
 #define VIRTIO_BLK_F_WCE	9	/* Writeback mode enabled after reset */
 #define VIRTIO_BLK_F_TOPOLOGY	10	/* Topology information is available */
 #define VIRTIO_BLK_F_CONFIG_WCE	11	/* Writeback mode available in config */
+#define VIRTIO_BLK_F_MQ		12	/* support more than one vq */
 
 #ifndef __KERNEL__
 /* Old (deprecated) name for VIRTIO_BLK_F_WCE. */
@@ -77,6 +78,9 @@ struct virtio_blk_config {
 
 	/* writeback mode (if VIRTIO_BLK_F_CONFIG_WCE) */
 	__u8 wce;
+
+	/* number of vqs, only available when VIRTIO_BLK_F_MQ is set */
+	__u16 num_queues;
 } __attribute__((packed));
 
 /*
-- 
1.7.9.5

Re: [RFC PATCH 1/2] include/uapi/linux/virtio_blk.h: introduce feature of VIRTIO_BLK_F_MQ

[Rusty Russell]

Ming Lei <ming.lei@canonical.com> writes:
> Current virtio-blk spec only supports one virtual queue for transfering
> data between VM and host, and inside VM all kinds of operations on
> the virtual queue needs to hold one lock, so cause below problems:
>
> 	- no scalability
>     - bad throughput
>
> So this patch requests to introduce feature of VIRTIO_BLK_F_MQ
> so that more than one virtual queues can be used to virtio-blk
> device, then above problems can be solved or eased.
>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
>  include/uapi/linux/virtio_blk.h |    4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/include/uapi/linux/virtio_blk.h b/include/uapi/linux/virtio_blk.h
> index 6d8e61c..c5a2751 100644
> --- a/include/uapi/linux/virtio_blk.h
> +++ b/include/uapi/linux/virtio_blk.h
> @@ -40,6 +40,7 @@
>  #define VIRTIO_BLK_F_WCE	9	/* Writeback mode enabled after reset */
>  #define VIRTIO_BLK_F_TOPOLOGY	10	/* Topology information is available */
>  #define VIRTIO_BLK_F_CONFIG_WCE	11	/* Writeback mode available in config */
> +#define VIRTIO_BLK_F_MQ		12	/* support more than one vq */
>  
>  #ifndef __KERNEL__
>  /* Old (deprecated) name for VIRTIO_BLK_F_WCE. */
> @@ -77,6 +78,9 @@ struct virtio_blk_config {
>  
>  	/* writeback mode (if VIRTIO_BLK_F_CONFIG_WCE) */
>  	__u8 wce;
> +
> +	/* number of vqs, only available when VIRTIO_BLK_F_MQ is set */
> +	__u16 num_queues;
>  } __attribute__((packed));

Hmm, please pad this like so:

        __u8 unused;

        __u16 num_queues;

That avoids weird alignment.

Thanks,
Rusty.

Re: [RFC PATCH 1/2] include/uapi/linux/virtio_blk.h: introduce feature of VIRTIO_BLK_F_MQ

[Rusty Russell]

Ming Lei <ming.lei@canonical.com> writes:
> Current virtio-blk spec only supports one virtual queue for transfering
> data between VM and host, and inside VM all kinds of operations on
> the virtual queue needs to hold one lock, so cause below problems:
>
> 	- no scalability
>     - bad throughput
>
> So this patch requests to introduce feature of VIRTIO_BLK_F_MQ
> so that more than one virtual queues can be used to virtio-blk
> device, then above problems can be solved or eased.
>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
>  include/uapi/linux/virtio_blk.h |    4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/include/uapi/linux/virtio_blk.h b/include/uapi/linux/virtio_blk.h
> index 6d8e61c..c5a2751 100644
> --- a/include/uapi/linux/virtio_blk.h
> +++ b/include/uapi/linux/virtio_blk.h
> @@ -40,6 +40,7 @@
>  #define VIRTIO_BLK_F_WCE	9	/* Writeback mode enabled after reset */
>  #define VIRTIO_BLK_F_TOPOLOGY	10	/* Topology information is available */
>  #define VIRTIO_BLK_F_CONFIG_WCE	11	/* Writeback mode available in config */
> +#define VIRTIO_BLK_F_MQ		12	/* support more than one vq */
>  
>  #ifndef __KERNEL__
>  /* Old (deprecated) name for VIRTIO_BLK_F_WCE. */
> @@ -77,6 +78,9 @@ struct virtio_blk_config {
>  
>  	/* writeback mode (if VIRTIO_BLK_F_CONFIG_WCE) */
>  	__u8 wce;
> +
> +	/* number of vqs, only available when VIRTIO_BLK_F_MQ is set */
> +	__u16 num_queues;
>  } __attribute__((packed));

Hmm, please pad this like so:

        __u8 unused;

        __u16 num_queues;

That avoids weird alignment.

Thanks,
Rusty.

Re: [RFC PATCH 1/2] include/uapi/linux/virtio_blk.h: introduce feature of VIRTIO_BLK_F_MQ

[Rusty Russell]

Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> writes:
> Current virtio-blk spec only supports one virtual queue for transfering
> data between VM and host, and inside VM all kinds of operations on
> the virtual queue needs to hold one lock, so cause below problems:
>
> 	- no scalability
>     - bad throughput
>
> So this patch requests to introduce feature of VIRTIO_BLK_F_MQ
> so that more than one virtual queues can be used to virtio-blk
> device, then above problems can be solved or eased.
>
> Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> ---
>  include/uapi/linux/virtio_blk.h |    4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/include/uapi/linux/virtio_blk.h b/include/uapi/linux/virtio_blk.h
> index 6d8e61c..c5a2751 100644
> --- a/include/uapi/linux/virtio_blk.h
> +++ b/include/uapi/linux/virtio_blk.h
> @@ -40,6 +40,7 @@
>  #define VIRTIO_BLK_F_WCE	9	/* Writeback mode enabled after reset */
>  #define VIRTIO_BLK_F_TOPOLOGY	10	/* Topology information is available */
>  #define VIRTIO_BLK_F_CONFIG_WCE	11	/* Writeback mode available in config */
> +#define VIRTIO_BLK_F_MQ		12	/* support more than one vq */
>  
>  #ifndef __KERNEL__
>  /* Old (deprecated) name for VIRTIO_BLK_F_WCE. */
> @@ -77,6 +78,9 @@ struct virtio_blk_config {
>  
>  	/* writeback mode (if VIRTIO_BLK_F_CONFIG_WCE) */
>  	__u8 wce;
> +
> +	/* number of vqs, only available when VIRTIO_BLK_F_MQ is set */
> +	__u16 num_queues;
>  } __attribute__((packed));

Hmm, please pad this like so:

        __u8 unused;

        __u16 num_queues;

That avoids weird alignment.

Thanks,
Rusty.

[RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Ming Lei]

Firstly this patch supports more than one virtual queues for virtio-blk
device.

Secondly this patch maps the virtual queue to blk-mq's hardware queue.

With this approach, both scalability and performance problem can be improved.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/block/virtio_blk.c |   75 ++++++++++++++++++++++++++++++++------------
 1 file changed, 55 insertions(+), 20 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index f63d358..e0d077d 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -16,6 +16,8 @@
 
 #define PART_BITS 4
 
+#define MAX_NUM_VQ 16
+
 static int major;
 static DEFINE_IDA(vd_index_ida);
 
@@ -24,8 +26,8 @@ static struct workqueue_struct *virtblk_wq;
 struct virtio_blk
 {
 	struct virtio_device *vdev;
-	struct virtqueue *vq;
-	spinlock_t vq_lock;
+	struct virtqueue *vq[MAX_NUM_VQ];
+	spinlock_t vq_lock[MAX_NUM_VQ];
 
 	/* The disk structure for the kernel. */
 	struct gendisk *disk;
@@ -47,6 +49,9 @@ struct virtio_blk
 
 	/* Ida index - used to track minor number allocations. */
 	int index;
+
+	/* num of vqs */
+	int num_vqs;
 };
 
 struct virtblk_req
@@ -133,14 +138,15 @@ static void virtblk_done(struct virtqueue *vq)
 {
 	struct virtio_blk *vblk = vq->vdev->priv;
 	bool req_done = false;
+	int qid = vq->index;
 	struct virtblk_req *vbr;
 	unsigned long flags;
 	unsigned int len;
 
-	spin_lock_irqsave(&vblk->vq_lock, flags);
+	spin_lock_irqsave(&vblk->vq_lock[qid], flags);
 	do {
 		virtqueue_disable_cb(vq);
-		while ((vbr = virtqueue_get_buf(vblk->vq, &len)) != NULL) {
+		while ((vbr = virtqueue_get_buf(vblk->vq[qid], &len)) != NULL) {
 			blk_mq_complete_request(vbr->req);
 			req_done = true;
 		}
@@ -151,7 +157,7 @@ static void virtblk_done(struct virtqueue *vq)
 	/* In case queue is stopped waiting for more buffers. */
 	if (req_done)
 		blk_mq_start_stopped_hw_queues(vblk->disk->queue, true);
-	spin_unlock_irqrestore(&vblk->vq_lock, flags);
+	spin_unlock_irqrestore(&vblk->vq_lock[qid], flags);
 }
 
 static int virtio_queue_rq(struct blk_mq_hw_ctx *hctx, struct request *req)
@@ -160,6 +166,7 @@ static int virtio_queue_rq(struct blk_mq_hw_ctx *hctx, struct request *req)
 	struct virtblk_req *vbr = blk_mq_rq_to_pdu(req);
 	unsigned long flags;
 	unsigned int num;
+	int qid = hctx->queue_num;
 	const bool last = (req->cmd_flags & REQ_END) != 0;
 	int err;
 	bool notify = false;
@@ -202,12 +209,12 @@ static int virtio_queue_rq(struct blk_mq_hw_ctx *hctx, struct request *req)
 			vbr->out_hdr.type |= VIRTIO_BLK_T_IN;
 	}
 
-	spin_lock_irqsave(&vblk->vq_lock, flags);
-	err = __virtblk_add_req(vblk->vq, vbr, vbr->sg, num);
+	spin_lock_irqsave(&vblk->vq_lock[qid], flags);
+	err = __virtblk_add_req(vblk->vq[qid], vbr, vbr->sg, num);
 	if (err) {
-		virtqueue_kick(vblk->vq);
+		virtqueue_kick(vblk->vq[qid]);
 		blk_mq_stop_hw_queue(hctx);
-		spin_unlock_irqrestore(&vblk->vq_lock, flags);
+		spin_unlock_irqrestore(&vblk->vq_lock[qid], flags);
 		/* Out of mem doesn't actually happen, since we fall back
 		 * to direct descriptors */
 		if (err == -ENOMEM || err == -ENOSPC)
@@ -215,12 +222,12 @@ static int virtio_queue_rq(struct blk_mq_hw_ctx *hctx, struct request *req)
 		return BLK_MQ_RQ_QUEUE_ERROR;
 	}
 
-	if (last && virtqueue_kick_prepare(vblk->vq))
+	if (last && virtqueue_kick_prepare(vblk->vq[qid]))
 		notify = true;
-	spin_unlock_irqrestore(&vblk->vq_lock, flags);
+	spin_unlock_irqrestore(&vblk->vq_lock[qid], flags);
 
 	if (notify)
-		virtqueue_notify(vblk->vq);
+		virtqueue_notify(vblk->vq[qid]);
 	return BLK_MQ_RQ_QUEUE_OK;
 }
 
@@ -377,12 +384,40 @@ static void virtblk_config_changed(struct virtio_device *vdev)
 static int init_vq(struct virtio_blk *vblk)
 {
 	int err = 0;
+	int i;
+	vq_callback_t *callbacks[MAX_NUM_VQ];
+	const char *names[MAX_NUM_VQ];
+	unsigned short num_vqs;
+	struct virtio_device *vdev = vblk->vdev;
 
-	/* We expect one virtqueue, for output. */
-	vblk->vq = virtio_find_single_vq(vblk->vdev, virtblk_done, "requests");
-	if (IS_ERR(vblk->vq))
-		err = PTR_ERR(vblk->vq);
+	if (virtio_has_feature(vdev, VIRTIO_BLK_F_MQ))
+		err = virtio_cread_feature(vdev, VIRTIO_BLK_F_MQ,
+				   struct virtio_blk_config, num_queues,
+				   &num_vqs);
+	else
+		num_vqs = 1;
+
+	if (err)
+		goto out;
 
+	if (num_vqs > MAX_NUM_VQ)
+		num_vqs = MAX_NUM_VQ;
+
+	for (i = 0; i < num_vqs; i++) {
+		callbacks[i] = virtblk_done;
+		names[i] = "requests";
+	}
+
+	/* Discover virtqueues and write information to configuration.  */
+	err = vdev->config->find_vqs(vdev, num_vqs, vblk->vq,
+			callbacks, names);
+	if (err)
+		goto out;
+
+	for (i = 0; i < num_vqs; i++)
+		spin_lock_init(&vblk->vq_lock[i]);
+	vblk->num_vqs = num_vqs;
+out:
 	return err;
 }
 
@@ -551,7 +586,6 @@ static int virtblk_probe(struct virtio_device *vdev)
 	err = init_vq(vblk);
 	if (err)
 		goto out_free_vblk;
-	spin_lock_init(&vblk->vq_lock);
 
 	/* FIXME: How many partitions?  How long is a piece of string? */
 	vblk->disk = alloc_disk(1 << PART_BITS);
@@ -562,7 +596,7 @@ static int virtblk_probe(struct virtio_device *vdev)
 
 	/* Default queue sizing is to fill the ring. */
 	if (!virtblk_queue_depth) {
-		virtblk_queue_depth = vblk->vq->num_free;
+		virtblk_queue_depth = vblk->vq[0]->num_free;
 		/* ... but without indirect descs, we use 2 descs per req */
 		if (!virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC))
 			virtblk_queue_depth /= 2;
@@ -570,7 +604,6 @@ static int virtblk_probe(struct virtio_device *vdev)
 
 	memset(&vblk->tag_set, 0, sizeof(vblk->tag_set));
 	vblk->tag_set.ops = &virtio_mq_ops;
-	vblk->tag_set.nr_hw_queues = 1;
 	vblk->tag_set.queue_depth = virtblk_queue_depth;
 	vblk->tag_set.numa_node = NUMA_NO_NODE;
 	vblk->tag_set.flags = BLK_MQ_F_SHOULD_MERGE;
@@ -578,6 +611,7 @@ static int virtblk_probe(struct virtio_device *vdev)
 		sizeof(struct virtblk_req) +
 		sizeof(struct scatterlist) * sg_elems;
 	vblk->tag_set.driver_data = vblk;
+	vblk->tag_set.nr_hw_queues = vblk->num_vqs;
 
 	err = blk_mq_alloc_tag_set(&vblk->tag_set);
 	if (err)
@@ -777,7 +811,8 @@ static const struct virtio_device_id id_table[] = {
 static unsigned int features[] = {
 	VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
 	VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE, VIRTIO_BLK_F_SCSI,
-	VIRTIO_BLK_F_WCE, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE
+	VIRTIO_BLK_F_WCE, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
+	VIRTIO_BLK_F_MQ,
 };
 
 static struct virtio_driver virtio_blk = {
-- 
1.7.9.5

[RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Ming Lei]

Firstly this patch supports more than one virtual queues for virtio-blk
device.

Secondly this patch maps the virtual queue to blk-mq's hardware queue.

With this approach, both scalability and performance problem can be improved.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/block/virtio_blk.c |   75 ++++++++++++++++++++++++++++++++------------
 1 file changed, 55 insertions(+), 20 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index f63d358..e0d077d 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -16,6 +16,8 @@
 
 #define PART_BITS 4
 
+#define MAX_NUM_VQ 16
+
 static int major;
 static DEFINE_IDA(vd_index_ida);
 
@@ -24,8 +26,8 @@ static struct workqueue_struct *virtblk_wq;
 struct virtio_blk
 {
 	struct virtio_device *vdev;
-	struct virtqueue *vq;
-	spinlock_t vq_lock;
+	struct virtqueue *vq[MAX_NUM_VQ];
+	spinlock_t vq_lock[MAX_NUM_VQ];
 
 	/* The disk structure for the kernel. */
 	struct gendisk *disk;
@@ -47,6 +49,9 @@ struct virtio_blk
 
 	/* Ida index - used to track minor number allocations. */
 	int index;
+
+	/* num of vqs */
+	int num_vqs;
 };
 
 struct virtblk_req
@@ -133,14 +138,15 @@ static void virtblk_done(struct virtqueue *vq)
 {
 	struct virtio_blk *vblk = vq->vdev->priv;
 	bool req_done = false;
+	int qid = vq->index;
 	struct virtblk_req *vbr;
 	unsigned long flags;
 	unsigned int len;
 
-	spin_lock_irqsave(&vblk->vq_lock, flags);
+	spin_lock_irqsave(&vblk->vq_lock[qid], flags);
 	do {
 		virtqueue_disable_cb(vq);
-		while ((vbr = virtqueue_get_buf(vblk->vq, &len)) != NULL) {
+		while ((vbr = virtqueue_get_buf(vblk->vq[qid], &len)) != NULL) {
 			blk_mq_complete_request(vbr->req);
 			req_done = true;
 		}
@@ -151,7 +157,7 @@ static void virtblk_done(struct virtqueue *vq)
 	/* In case queue is stopped waiting for more buffers. */
 	if (req_done)
 		blk_mq_start_stopped_hw_queues(vblk->disk->queue, true);
-	spin_unlock_irqrestore(&vblk->vq_lock, flags);
+	spin_unlock_irqrestore(&vblk->vq_lock[qid], flags);
 }
 
 static int virtio_queue_rq(struct blk_mq_hw_ctx *hctx, struct request *req)
@@ -160,6 +166,7 @@ static int virtio_queue_rq(struct blk_mq_hw_ctx *hctx, struct request *req)
 	struct virtblk_req *vbr = blk_mq_rq_to_pdu(req);
 	unsigned long flags;
 	unsigned int num;
+	int qid = hctx->queue_num;
 	const bool last = (req->cmd_flags & REQ_END) != 0;
 	int err;
 	bool notify = false;
@@ -202,12 +209,12 @@ static int virtio_queue_rq(struct blk_mq_hw_ctx *hctx, struct request *req)
 			vbr->out_hdr.type |= VIRTIO_BLK_T_IN;
 	}
 
-	spin_lock_irqsave(&vblk->vq_lock, flags);
-	err = __virtblk_add_req(vblk->vq, vbr, vbr->sg, num);
+	spin_lock_irqsave(&vblk->vq_lock[qid], flags);
+	err = __virtblk_add_req(vblk->vq[qid], vbr, vbr->sg, num);
 	if (err) {
-		virtqueue_kick(vblk->vq);
+		virtqueue_kick(vblk->vq[qid]);
 		blk_mq_stop_hw_queue(hctx);
-		spin_unlock_irqrestore(&vblk->vq_lock, flags);
+		spin_unlock_irqrestore(&vblk->vq_lock[qid], flags);
 		/* Out of mem doesn't actually happen, since we fall back
 		 * to direct descriptors */
 		if (err == -ENOMEM || err == -ENOSPC)
@@ -215,12 +222,12 @@ static int virtio_queue_rq(struct blk_mq_hw_ctx *hctx, struct request *req)
 		return BLK_MQ_RQ_QUEUE_ERROR;
 	}
 
-	if (last && virtqueue_kick_prepare(vblk->vq))
+	if (last && virtqueue_kick_prepare(vblk->vq[qid]))
 		notify = true;
-	spin_unlock_irqrestore(&vblk->vq_lock, flags);
+	spin_unlock_irqrestore(&vblk->vq_lock[qid], flags);
 
 	if (notify)
-		virtqueue_notify(vblk->vq);
+		virtqueue_notify(vblk->vq[qid]);
 	return BLK_MQ_RQ_QUEUE_OK;
 }
 
@@ -377,12 +384,40 @@ static void virtblk_config_changed(struct virtio_device *vdev)
 static int init_vq(struct virtio_blk *vblk)
 {
 	int err = 0;
+	int i;
+	vq_callback_t *callbacks[MAX_NUM_VQ];
+	const char *names[MAX_NUM_VQ];
+	unsigned short num_vqs;
+	struct virtio_device *vdev = vblk->vdev;
 
-	/* We expect one virtqueue, for output. */
-	vblk->vq = virtio_find_single_vq(vblk->vdev, virtblk_done, "requests");
-	if (IS_ERR(vblk->vq))
-		err = PTR_ERR(vblk->vq);
+	if (virtio_has_feature(vdev, VIRTIO_BLK_F_MQ))
+		err = virtio_cread_feature(vdev, VIRTIO_BLK_F_MQ,
+				   struct virtio_blk_config, num_queues,
+				   &num_vqs);
+	else
+		num_vqs = 1;
+
+	if (err)
+		goto out;
 
+	if (num_vqs > MAX_NUM_VQ)
+		num_vqs = MAX_NUM_VQ;
+
+	for (i = 0; i < num_vqs; i++) {
+		callbacks[i] = virtblk_done;
+		names[i] = "requests";
+	}
+
+	/* Discover virtqueues and write information to configuration.  */
+	err = vdev->config->find_vqs(vdev, num_vqs, vblk->vq,
+			callbacks, names);
+	if (err)
+		goto out;
+
+	for (i = 0; i < num_vqs; i++)
+		spin_lock_init(&vblk->vq_lock[i]);
+	vblk->num_vqs = num_vqs;
+out:
 	return err;
 }
 
@@ -551,7 +586,6 @@ static int virtblk_probe(struct virtio_device *vdev)
 	err = init_vq(vblk);
 	if (err)
 		goto out_free_vblk;
-	spin_lock_init(&vblk->vq_lock);
 
 	/* FIXME: How many partitions?  How long is a piece of string? */
 	vblk->disk = alloc_disk(1 << PART_BITS);
@@ -562,7 +596,7 @@ static int virtblk_probe(struct virtio_device *vdev)
 
 	/* Default queue sizing is to fill the ring. */
 	if (!virtblk_queue_depth) {
-		virtblk_queue_depth = vblk->vq->num_free;
+		virtblk_queue_depth = vblk->vq[0]->num_free;
 		/* ... but without indirect descs, we use 2 descs per req */
 		if (!virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC))
 			virtblk_queue_depth /= 2;
@@ -570,7 +604,6 @@ static int virtblk_probe(struct virtio_device *vdev)
 
 	memset(&vblk->tag_set, 0, sizeof(vblk->tag_set));
 	vblk->tag_set.ops = &virtio_mq_ops;
-	vblk->tag_set.nr_hw_queues = 1;
 	vblk->tag_set.queue_depth = virtblk_queue_depth;
 	vblk->tag_set.numa_node = NUMA_NO_NODE;
 	vblk->tag_set.flags = BLK_MQ_F_SHOULD_MERGE;
@@ -578,6 +611,7 @@ static int virtblk_probe(struct virtio_device *vdev)
 		sizeof(struct virtblk_req) +
 		sizeof(struct scatterlist) * sg_elems;
 	vblk->tag_set.driver_data = vblk;
+	vblk->tag_set.nr_hw_queues = vblk->num_vqs;
 
 	err = blk_mq_alloc_tag_set(&vblk->tag_set);
 	if (err)
@@ -777,7 +811,8 @@ static const struct virtio_device_id id_table[] = {
 static unsigned int features[] = {
 	VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
 	VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE, VIRTIO_BLK_F_SCSI,
-	VIRTIO_BLK_F_WCE, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE
+	VIRTIO_BLK_F_WCE, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
+	VIRTIO_BLK_F_MQ,
 };
 
 static struct virtio_driver virtio_blk = {
-- 
1.7.9.5

Re: [RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Rusty Russell]

Ming Lei <ming.lei@canonical.com> writes:
> +	if (virtio_has_feature(vdev, VIRTIO_BLK_F_MQ))
> +		err = virtio_cread_feature(vdev, VIRTIO_BLK_F_MQ,
> +				   struct virtio_blk_config, num_queues,
> +				   &num_vqs);
> +	else
> +		num_vqs = 1;

This is redundant: virtio_cread_feature() checks the feature.

So, either:
        if (virtio_has_feature(vdev, VIRTIO_BLK_F_MQ))
                virtio_cread(vdev, struct virtio_blk_config, num_queues,
          		     &num_vqs);
        else
                num_vqs = 1;

Or:
	err = virtio_cread_feature(vdev, VIRTIO_BLK_F_MQ,
			   struct virtio_blk_config, num_queues,
			   &num_vqs);
        if (err)
                num_vqs = 1;

Otherwise, the patch looks pretty straight-forward.

Cheers,
Rusty.

Re: [RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Rusty Russell]

Ming Lei <ming.lei@canonical.com> writes:
> +	if (virtio_has_feature(vdev, VIRTIO_BLK_F_MQ))
> +		err = virtio_cread_feature(vdev, VIRTIO_BLK_F_MQ,
> +				   struct virtio_blk_config, num_queues,
> +				   &num_vqs);
> +	else
> +		num_vqs = 1;

This is redundant: virtio_cread_feature() checks the feature.

So, either:
        if (virtio_has_feature(vdev, VIRTIO_BLK_F_MQ))
                virtio_cread(vdev, struct virtio_blk_config, num_queues,
          		     &num_vqs);
        else
                num_vqs = 1;

Or:
	err = virtio_cread_feature(vdev, VIRTIO_BLK_F_MQ,
			   struct virtio_blk_config, num_queues,
			   &num_vqs);
        if (err)
                num_vqs = 1;

Otherwise, the patch looks pretty straight-forward.

Cheers,
Rusty.

Re: [RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Rusty Russell]

Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> writes:
> +	if (virtio_has_feature(vdev, VIRTIO_BLK_F_MQ))
> +		err = virtio_cread_feature(vdev, VIRTIO_BLK_F_MQ,
> +				   struct virtio_blk_config, num_queues,
> +				   &num_vqs);
> +	else
> +		num_vqs = 1;

This is redundant: virtio_cread_feature() checks the feature.

So, either:
        if (virtio_has_feature(vdev, VIRTIO_BLK_F_MQ))
                virtio_cread(vdev, struct virtio_blk_config, num_queues,
          		     &num_vqs);
        else
                num_vqs = 1;

Or:
	err = virtio_cread_feature(vdev, VIRTIO_BLK_F_MQ,
			   struct virtio_blk_config, num_queues,
			   &num_vqs);
        if (err)
                num_vqs = 1;

Otherwise, the patch looks pretty straight-forward.

Cheers,
Rusty.

Re: [RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Stefan Hajnoczi]

On Sat, Jun 14, 2014 at 1:29 AM, Ming Lei <ming.lei@canonical.com> wrote:
> Firstly this patch supports more than one virtual queues for virtio-blk
> device.
>
> Secondly this patch maps the virtual queue to blk-mq's hardware queue.
>
> With this approach, both scalability and performance problem can be improved.
>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
>  drivers/block/virtio_blk.c |   75 ++++++++++++++++++++++++++++++++------------
>  1 file changed, 55 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index f63d358..e0d077d 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -16,6 +16,8 @@
>
>  #define PART_BITS 4
>
> +#define MAX_NUM_VQ 16

It would be nice to allocate virtqueues dynamically instead of
hardcoding the limit.  virtio-scsi also allocates virtqueues
dynamically.

Stefan

Re: [RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Stefan Hajnoczi]

On Sat, Jun 14, 2014 at 1:29 AM, Ming Lei <ming.lei@canonical.com> wrote:
> Firstly this patch supports more than one virtual queues for virtio-blk
> device.
>
> Secondly this patch maps the virtual queue to blk-mq's hardware queue.
>
> With this approach, both scalability and performance problem can be improved.
>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
>  drivers/block/virtio_blk.c |   75 ++++++++++++++++++++++++++++++++------------
>  1 file changed, 55 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index f63d358..e0d077d 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -16,6 +16,8 @@
>
>  #define PART_BITS 4
>
> +#define MAX_NUM_VQ 16

It would be nice to allocate virtqueues dynamically instead of
hardcoding the limit.  virtio-scsi also allocates virtqueues
dynamically.

Stefan

Re: [RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Ming Lei]

On Tue, Jun 17, 2014 at 10:40 AM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Sat, Jun 14, 2014 at 1:29 AM, Ming Lei <ming.lei@canonical.com> wrote:
>> Firstly this patch supports more than one virtual queues for virtio-blk
>> device.
>>
>> Secondly this patch maps the virtual queue to blk-mq's hardware queue.
>>
>> With this approach, both scalability and performance problem can be improved.
>>
>> Signed-off-by: Ming Lei <ming.lei@canonical.com>
>> ---
>>  drivers/block/virtio_blk.c |   75 ++++++++++++++++++++++++++++++++------------
>>  1 file changed, 55 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
>> index f63d358..e0d077d 100644
>> --- a/drivers/block/virtio_blk.c
>> +++ b/drivers/block/virtio_blk.c
>> @@ -16,6 +16,8 @@
>>
>>  #define PART_BITS 4
>>
>> +#define MAX_NUM_VQ 16
>
> It would be nice to allocate virtqueues dynamically instead of
> hardcoding the limit.  virtio-scsi also allocates virtqueues
> dynamically.

virtio-scsi may have lots of LUN, but virtio-blk only has one disk
which needn't lots of hardware queues.

Also it doesn't matter since it isn't part of ABI.

If change on virtio_blk_config is agreed, both host side and
guest side can choose to support dynamic length or pre-defined
length freely.

Thanks,
--
Ming Lei

Re: [RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Ming Lei]

On Tue, Jun 17, 2014 at 10:40 AM, Stefan Hajnoczi <stefanha-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Sat, Jun 14, 2014 at 1:29 AM, Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> wrote:
>> Firstly this patch supports more than one virtual queues for virtio-blk
>> device.
>>
>> Secondly this patch maps the virtual queue to blk-mq's hardware queue.
>>
>> With this approach, both scalability and performance problem can be improved.
>>
>> Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
>> ---
>>  drivers/block/virtio_blk.c |   75 ++++++++++++++++++++++++++++++++------------
>>  1 file changed, 55 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
>> index f63d358..e0d077d 100644
>> --- a/drivers/block/virtio_blk.c
>> +++ b/drivers/block/virtio_blk.c
>> @@ -16,6 +16,8 @@
>>
>>  #define PART_BITS 4
>>
>> +#define MAX_NUM_VQ 16
>
> It would be nice to allocate virtqueues dynamically instead of
> hardcoding the limit.  virtio-scsi also allocates virtqueues
> dynamically.

virtio-scsi may have lots of LUN, but virtio-blk only has one disk
which needn't lots of hardware queues.

Also it doesn't matter since it isn't part of ABI.

If change on virtio_blk_config is agreed, both host side and
guest side can choose to support dynamic length or pre-defined
length freely.

Thanks,
--
Ming Lei

Re: [RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Paolo Bonzini]

Il 17/06/2014 17:50, Ming Lei ha scritto:
>> > It would be nice to allocate virtqueues dynamically instead of
>> > hardcoding the limit.  virtio-scsi also allocates virtqueues
>> > dynamically.
> virtio-scsi may have lots of LUN, but virtio-blk only has one disk
> which needn't lots of hardware queues.

If you want to do queue steering based on the guest VCPU number, the 
number of queues must be = to the number of VCPUs shouldn't it?

I tried using a divisor of the number of VCPUs, but couldn't get the 
block layer to deliver interrupts to the right VCPU.

Paolo

Re: [RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Paolo Bonzini]

Il 17/06/2014 17:50, Ming Lei ha scritto:
>> > It would be nice to allocate virtqueues dynamically instead of
>> > hardcoding the limit.  virtio-scsi also allocates virtqueues
>> > dynamically.
> virtio-scsi may have lots of LUN, but virtio-blk only has one disk
> which needn't lots of hardware queues.

If you want to do queue steering based on the guest VCPU number, the 
number of queues must be = to the number of VCPUs shouldn't it?

I tried using a divisor of the number of VCPUs, but couldn't get the 
block layer to deliver interrupts to the right VCPU.

Paolo

Re: [RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Ming Lei]

On Tue, Jun 17, 2014 at 11:53 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 17/06/2014 17:50, Ming Lei ha scritto:
>
>>> > It would be nice to allocate virtqueues dynamically instead of
>>> > hardcoding the limit.  virtio-scsi also allocates virtqueues
>>> > dynamically.
>>
>> virtio-scsi may have lots of LUN, but virtio-blk only has one disk
>> which needn't lots of hardware queues.
>
>
> If you want to do queue steering based on the guest VCPU number, the number
> of queues must be = to the number of VCPUs shouldn't it?
>
> I tried using a divisor of the number of VCPUs, but couldn't get the block
> layer to deliver interrupts to the right VCPU.

For blk-mq's hardware queue, that won't be necessary to equal to
VCPUs number, and irq affinity per hw queue can be simply set as
blk_mq_hw_ctx->cpumask.

Thanks,
--
Ming Lei

Re: [RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Ming Lei]

On Tue, Jun 17, 2014 at 11:53 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 17/06/2014 17:50, Ming Lei ha scritto:
>
>>> > It would be nice to allocate virtqueues dynamically instead of
>>> > hardcoding the limit.  virtio-scsi also allocates virtqueues
>>> > dynamically.
>>
>> virtio-scsi may have lots of LUN, but virtio-blk only has one disk
>> which needn't lots of hardware queues.
>
>
> If you want to do queue steering based on the guest VCPU number, the number
> of queues must be = to the number of VCPUs shouldn't it?
>
> I tried using a divisor of the number of VCPUs, but couldn't get the block
> layer to deliver interrupts to the right VCPU.

For blk-mq's hardware queue, that won't be necessary to equal to
VCPUs number, and irq affinity per hw queue can be simply set as
blk_mq_hw_ctx->cpumask.

Thanks,
--
Ming Lei

Re: [RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Paolo Bonzini]

Il 17/06/2014 18:00, Ming Lei ha scritto:
>> > If you want to do queue steering based on the guest VCPU number, the number
>> > of queues must be = to the number of VCPUs shouldn't it?
>> >
>> > I tried using a divisor of the number of VCPUs, but couldn't get the block
>> > layer to deliver interrupts to the right VCPU.
> For blk-mq's hardware queue, that won't be necessary to equal to
> VCPUs number, and irq affinity per hw queue can be simply set as
> blk_mq_hw_ctx->cpumask.

Yes, but on top of that you want to have each request processed exactly 
by the CPU that sent it.  Unless the cpumasks are singletons, most of 
the benefit went away in my virtio-scsi tests.  Perhaps blk-mq is smarter.

Can you try benchmarking a 16 VCPU guest with 8 and 16 queues?

Paolo

Re: [RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Paolo Bonzini]

Il 17/06/2014 18:00, Ming Lei ha scritto:
>> > If you want to do queue steering based on the guest VCPU number, the number
>> > of queues must be = to the number of VCPUs shouldn't it?
>> >
>> > I tried using a divisor of the number of VCPUs, but couldn't get the block
>> > layer to deliver interrupts to the right VCPU.
> For blk-mq's hardware queue, that won't be necessary to equal to
> VCPUs number, and irq affinity per hw queue can be simply set as
> blk_mq_hw_ctx->cpumask.

Yes, but on top of that you want to have each request processed exactly 
by the CPU that sent it.  Unless the cpumasks are singletons, most of 
the benefit went away in my virtio-scsi tests.  Perhaps blk-mq is smarter.

Can you try benchmarking a 16 VCPU guest with 8 and 16 queues?

Paolo

Re: [RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Ming Lei]

On Wed, Jun 18, 2014 at 12:34 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 17/06/2014 18:00, Ming Lei ha scritto:
>
>>> > If you want to do queue steering based on the guest VCPU number, the
>>> > number
>>> > of queues must be = to the number of VCPUs shouldn't it?
>>> >
>>> > I tried using a divisor of the number of VCPUs, but couldn't get the
>>> > block
>>> > layer to deliver interrupts to the right VCPU.
>>
>> For blk-mq's hardware queue, that won't be necessary to equal to
>> VCPUs number, and irq affinity per hw queue can be simply set as
>> blk_mq_hw_ctx->cpumask.
>
>
> Yes, but on top of that you want to have each request processed exactly by
> the CPU that sent it.  Unless the cpumasks are singletons, most of the
> benefit went away in my virtio-scsi tests.  Perhaps blk-mq is smarter.
>
> Can you try benchmarking a 16 VCPU guest with 8 and 16 queues?

From VM side, it might be better to use one hardware queue per vCPU,
since in theory it can remove vq lock contention.

But from host side, there is still disadvantage with more queues, since
more queues means more notify times, in my virtio-blk test, even with
ioeventfd, one notification may take ~3us averagely on qemu-system-x86_64.

For virtio-blk, I don't think it is always better to take more queues, and
we need to leverage below things in host side:

- host storage top performance, generally it reaches that with more
than 1 jobs with libaio(suppose it is N, so basically we can use N
iothread per device in qemu to try to get top performance)

- iothreads' loading(if iothreads are at full loading, increasing
queues doesn't help at all)

In my test, I only use the current per-dev iothread(x-dataplane)
in qemu to handle 2 vqs' notification and precess all I/O from
the 2 vqs, and looks it can improve IOPS by ~30%.

For virtio-scsi, the current usage doesn't make full use of blk-mq's
advantage too because only one vq is active at the same time, so I
guess the multi vqs' benefit won't be very much and I'd like to post
patches to support that first, then provide test data with
more queues(8, 16).


Thanks,
--
Ming Lei

Re: [RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Ming Lei]

On Wed, Jun 18, 2014 at 12:34 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 17/06/2014 18:00, Ming Lei ha scritto:
>
>>> > If you want to do queue steering based on the guest VCPU number, the
>>> > number
>>> > of queues must be = to the number of VCPUs shouldn't it?
>>> >
>>> > I tried using a divisor of the number of VCPUs, but couldn't get the
>>> > block
>>> > layer to deliver interrupts to the right VCPU.
>>
>> For blk-mq's hardware queue, that won't be necessary to equal to
>> VCPUs number, and irq affinity per hw queue can be simply set as
>> blk_mq_hw_ctx->cpumask.
>
>
> Yes, but on top of that you want to have each request processed exactly by
> the CPU that sent it.  Unless the cpumasks are singletons, most of the
> benefit went away in my virtio-scsi tests.  Perhaps blk-mq is smarter.
>
> Can you try benchmarking a 16 VCPU guest with 8 and 16 queues?

>From VM side, it might be better to use one hardware queue per vCPU,
since in theory it can remove vq lock contention.

But from host side, there is still disadvantage with more queues, since
more queues means more notify times, in my virtio-blk test, even with
ioeventfd, one notification may take ~3us averagely on qemu-system-x86_64.

For virtio-blk, I don't think it is always better to take more queues, and
we need to leverage below things in host side:

- host storage top performance, generally it reaches that with more
than 1 jobs with libaio(suppose it is N, so basically we can use N
iothread per device in qemu to try to get top performance)

- iothreads' loading(if iothreads are at full loading, increasing
queues doesn't help at all)

In my test, I only use the current per-dev iothread(x-dataplane)
in qemu to handle 2 vqs' notification and precess all I/O from
the 2 vqs, and looks it can improve IOPS by ~30%.

For virtio-scsi, the current usage doesn't make full use of blk-mq's
advantage too because only one vq is active at the same time, so I
guess the multi vqs' benefit won't be very much and I'd like to post
patches to support that first, then provide test data with
more queues(8, 16).


Thanks,
--
Ming Lei

Re: [RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Ming Lei]

On Wed, Jun 18, 2014 at 12:34 AM, Paolo Bonzini <pbonzini-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Il 17/06/2014 18:00, Ming Lei ha scritto:
>
>>> > If you want to do queue steering based on the guest VCPU number, the
>>> > number
>>> > of queues must be = to the number of VCPUs shouldn't it?
>>> >
>>> > I tried using a divisor of the number of VCPUs, but couldn't get the
>>> > block
>>> > layer to deliver interrupts to the right VCPU.
>>
>> For blk-mq's hardware queue, that won't be necessary to equal to
>> VCPUs number, and irq affinity per hw queue can be simply set as
>> blk_mq_hw_ctx->cpumask.
>
>
> Yes, but on top of that you want to have each request processed exactly by
> the CPU that sent it.  Unless the cpumasks are singletons, most of the
> benefit went away in my virtio-scsi tests.  Perhaps blk-mq is smarter.
>
> Can you try benchmarking a 16 VCPU guest with 8 and 16 queues?

>From VM side, it might be better to use one hardware queue per vCPU,
since in theory it can remove vq lock contention.

But from host side, there is still disadvantage with more queues, since
more queues means more notify times, in my virtio-blk test, even with
ioeventfd, one notification may take ~3us averagely on qemu-system-x86_64.

For virtio-blk, I don't think it is always better to take more queues, and
we need to leverage below things in host side:

- host storage top performance, generally it reaches that with more
than 1 jobs with libaio(suppose it is N, so basically we can use N
iothread per device in qemu to try to get top performance)

- iothreads' loading(if iothreads are at full loading, increasing
queues doesn't help at all)

In my test, I only use the current per-dev iothread(x-dataplane)
in qemu to handle 2 vqs' notification and precess all I/O from
the 2 vqs, and looks it can improve IOPS by ~30%.

For virtio-scsi, the current usage doesn't make full use of blk-mq's
advantage too because only one vq is active at the same time, so I
guess the multi vqs' benefit won't be very much and I'd like to post
patches to support that first, then provide test data with
more queues(8, 16).


Thanks,
--
Ming Lei

Re: [RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Paolo Bonzini]

On 18/06/2014 06:04, Ming Lei wrote:
> For virtio-blk, I don't think it is always better to take more queues, and
> we need to leverage below things in host side:
> 
> - host storage top performance, generally it reaches that with more
> than 1 jobs with libaio(suppose it is N, so basically we can use N
> iothread per device in qemu to try to get top performance)
> 
> - iothreads' loading(if iothreads are at full loading, increasing
> queues doesn't help at all)
> 
> In my test, I only use the current per-dev iothread(x-dataplane)
> in qemu to handle 2 vqs' notification and precess all I/O from
> the 2 vqs, and looks it can improve IOPS by ~30%.
> 
> For virtio-scsi, the current usage doesn't make full use of blk-mq's
> advantage too because only one vq is active at the same time, so I
> guess the multi vqs' benefit won't be very much and I'd like to post
> patches to support that first, then provide test data with
> more queues(8, 16).

Hi Ming Lei,

would you like to repost these patches now that MQ support is in the kernel?

Also, I changed my mind about moving linux-aio to AioContext.  I now
think it's a good idea, because it limits the number of io_getevents
syscalls. O:-)  So I would be happy to review your patches for that as well.

Paolo

Re: [RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Paolo Bonzini]

On 18/06/2014 06:04, Ming Lei wrote:
> For virtio-blk, I don't think it is always better to take more queues, and
> we need to leverage below things in host side:
> 
> - host storage top performance, generally it reaches that with more
> than 1 jobs with libaio(suppose it is N, so basically we can use N
> iothread per device in qemu to try to get top performance)
> 
> - iothreads' loading(if iothreads are at full loading, increasing
> queues doesn't help at all)
> 
> In my test, I only use the current per-dev iothread(x-dataplane)
> in qemu to handle 2 vqs' notification and precess all I/O from
> the 2 vqs, and looks it can improve IOPS by ~30%.
> 
> For virtio-scsi, the current usage doesn't make full use of blk-mq's
> advantage too because only one vq is active at the same time, so I
> guess the multi vqs' benefit won't be very much and I'd like to post
> patches to support that first, then provide test data with
> more queues(8, 16).

Hi Ming Lei,

would you like to repost these patches now that MQ support is in the kernel?

Also, I changed my mind about moving linux-aio to AioContext.  I now
think it's a good idea, because it limits the number of io_getevents
syscalls. O:-)  So I would be happy to review your patches for that as well.

Paolo

Re: [RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Paolo Bonzini]

On 18/06/2014 06:04, Ming Lei wrote:
> For virtio-blk, I don't think it is always better to take more queues, and
> we need to leverage below things in host side:
> 
> - host storage top performance, generally it reaches that with more
> than 1 jobs with libaio(suppose it is N, so basically we can use N
> iothread per device in qemu to try to get top performance)
> 
> - iothreads' loading(if iothreads are at full loading, increasing
> queues doesn't help at all)
> 
> In my test, I only use the current per-dev iothread(x-dataplane)
> in qemu to handle 2 vqs' notification and precess all I/O from
> the 2 vqs, and looks it can improve IOPS by ~30%.
> 
> For virtio-scsi, the current usage doesn't make full use of blk-mq's
> advantage too because only one vq is active at the same time, so I
> guess the multi vqs' benefit won't be very much and I'd like to post
> patches to support that first, then provide test data with
> more queues(8, 16).

Hi Ming Lei,

would you like to repost these patches now that MQ support is in the kernel?

Also, I changed my mind about moving linux-aio to AioContext.  I now
think it's a good idea, because it limits the number of io_getevents
syscalls. O:-)  So I would be happy to review your patches for that as well.

Paolo

Re: [RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Ming Lei]

Hi Paolo,

On Mon, Dec 14, 2015 at 6:31 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 18/06/2014 06:04, Ming Lei wrote:
>> For virtio-blk, I don't think it is always better to take more queues, and
>> we need to leverage below things in host side:
>>
>> - host storage top performance, generally it reaches that with more
>> than 1 jobs with libaio(suppose it is N, so basically we can use N
>> iothread per device in qemu to try to get top performance)
>>
>> - iothreads' loading(if iothreads are at full loading, increasing
>> queues doesn't help at all)
>>
>> In my test, I only use the current per-dev iothread(x-dataplane)
>> in qemu to handle 2 vqs' notification and precess all I/O from
>> the 2 vqs, and looks it can improve IOPS by ~30%.
>>
>> For virtio-scsi, the current usage doesn't make full use of blk-mq's
>> advantage too because only one vq is active at the same time, so I
>> guess the multi vqs' benefit won't be very much and I'd like to post
>> patches to support that first, then provide test data with
>> more queues(8, 16).
>
> Hi Ming Lei,
>
> would you like to repost these patches now that MQ support is in the kernel?
>
> Also, I changed my mind about moving linux-aio to AioContext.  I now
> think it's a good idea, because it limits the number of io_getevents
> syscalls. O:-)  So I would be happy to review your patches for that as well.

OK, I try to figure out a new version, and it might take a while since it is
close to festival season, :-)

Thanks,

Re: [RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Ming Lei]

Hi Paolo,

On Mon, Dec 14, 2015 at 6:31 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 18/06/2014 06:04, Ming Lei wrote:
>> For virtio-blk, I don't think it is always better to take more queues, and
>> we need to leverage below things in host side:
>>
>> - host storage top performance, generally it reaches that with more
>> than 1 jobs with libaio(suppose it is N, so basically we can use N
>> iothread per device in qemu to try to get top performance)
>>
>> - iothreads' loading(if iothreads are at full loading, increasing
>> queues doesn't help at all)
>>
>> In my test, I only use the current per-dev iothread(x-dataplane)
>> in qemu to handle 2 vqs' notification and precess all I/O from
>> the 2 vqs, and looks it can improve IOPS by ~30%.
>>
>> For virtio-scsi, the current usage doesn't make full use of blk-mq's
>> advantage too because only one vq is active at the same time, so I
>> guess the multi vqs' benefit won't be very much and I'd like to post
>> patches to support that first, then provide test data with
>> more queues(8, 16).
>
> Hi Ming Lei,
>
> would you like to repost these patches now that MQ support is in the kernel?
>
> Also, I changed my mind about moving linux-aio to AioContext.  I now
> think it's a good idea, because it limits the number of io_getevents
> syscalls. O:-)  So I would be happy to review your patches for that as well.

OK, I try to figure out a new version, and it might take a while since it is
close to festival season, :-)

Thanks,

Re: [RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Ming Lei]

Hi Paolo,

On Mon, Dec 14, 2015 at 6:31 PM, Paolo Bonzini <pbonzini-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>
>
> On 18/06/2014 06:04, Ming Lei wrote:
>> For virtio-blk, I don't think it is always better to take more queues, and
>> we need to leverage below things in host side:
>>
>> - host storage top performance, generally it reaches that with more
>> than 1 jobs with libaio(suppose it is N, so basically we can use N
>> iothread per device in qemu to try to get top performance)
>>
>> - iothreads' loading(if iothreads are at full loading, increasing
>> queues doesn't help at all)
>>
>> In my test, I only use the current per-dev iothread(x-dataplane)
>> in qemu to handle 2 vqs' notification and precess all I/O from
>> the 2 vqs, and looks it can improve IOPS by ~30%.
>>
>> For virtio-scsi, the current usage doesn't make full use of blk-mq's
>> advantage too because only one vq is active at the same time, so I
>> guess the multi vqs' benefit won't be very much and I'd like to post
>> patches to support that first, then provide test data with
>> more queues(8, 16).
>
> Hi Ming Lei,
>
> would you like to repost these patches now that MQ support is in the kernel?
>
> Also, I changed my mind about moving linux-aio to AioContext.  I now
> think it's a good idea, because it limits the number of io_getevents
> syscalls. O:-)  So I would be happy to review your patches for that as well.

OK, I try to figure out a new version, and it might take a while since it is
close to festival season, :-)

Thanks,

Re: [RFC PATCH 2/2] block: virtio-blk: support multi virt queues per virtio-blk device

[Ming Lei]

On Tue, Jun 17, 2014 at 10:40 AM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Sat, Jun 14, 2014 at 1:29 AM, Ming Lei <ming.lei@canonical.com> wrote:
>> Firstly this patch supports more than one virtual queues for virtio-blk
>> device.
>>
>> Secondly this patch maps the virtual queue to blk-mq's hardware queue.
>>
>> With this approach, both scalability and performance problem can be improved.
>>
>> Signed-off-by: Ming Lei <ming.lei@canonical.com>
>> ---
>>  drivers/block/virtio_blk.c |   75 ++++++++++++++++++++++++++++++++------------
>>  1 file changed, 55 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
>> index f63d358..e0d077d 100644
>> --- a/drivers/block/virtio_blk.c
>> +++ b/drivers/block/virtio_blk.c
>> @@ -16,6 +16,8 @@
>>
>>  #define PART_BITS 4
>>
>> +#define MAX_NUM_VQ 16
>
> It would be nice to allocate virtqueues dynamically instead of
> hardcoding the limit.  virtio-scsi also allocates virtqueues
> dynamically.

virtio-scsi may have lots of LUN, but virtio-blk only has one disk
which needn't lots of hardware queues.

Also it doesn't matter since it isn't part of ABI.

If change on virtio_blk_config is agreed, both host side and
guest side can choose to support dynamic length or pre-defined
length freely.

Thanks,
--
Ming Lei

Re: [RFC PATCH 0/2] block: virtio-blk: support multi vq per virtio-blk

[Jens Axboe]

On 06/13/2014 11:29 AM, Ming Lei wrote:
> Hi,
> 
> This patches try to support multi virtual queues(multi-vq) in one
> virtio-blk device, and maps each virtual queue(vq) to blk-mq's
> hardware queue.
> 
> With this approach, both scalability and performance problems on
> virtio-blk device get improved.
> 
> For verifying the improvement, I implements virtio-blk multi-vq over
> qemu's dataplane feature, and both handling host notification
> from each vq and processing host I/O are still kept in the per-device
> iothread context, the changes are based on qemu v2.0.0 release, and
> can be accessed from below tree:
> 
> 	git://kernel.ubuntu.com/ming/qemu.git #v2.0.0-virtblk-dataplane-mq
> 
> For enabling the multi-vq feature, 'num_queues=N' need to be added into
> '-device virtio-blk-pci ...' of qemu command line, and suggest to pass
> 'vectors=N+1' to keep one MSI irq vector per each vq, and the feature
> depends on x-data-plane.
> 
> Fio(libaio, randread, iodepth=64, bs=4K, jobs=N) is run inside VM to
> verify the improvement.
> 
> I just create a small quadcore VM and run fio inside the VM, and
> num_queues of the virtio-blk device is set as 2, but looks the
> improvement is still obvious.
> 
> 1), about scalability
> - without mutli-vq feature
> 	-- jobs=2, thoughput: 145K iops
> 	-- jobs=4, thoughput: 100K iops
> - without mutli-vq feature
> 	-- jobs=2, thoughput: 186K iops
> 	-- jobs=4, thoughput: 199K iops

Awesome! I was hoping someone would do that, and make virtio-blk take
full advantage of blk-mq.

-- 
Jens Axboe

Re: [RFC PATCH 0/2] block: virtio-blk: support multi vq per virtio-blk

[Jens Axboe]

On 06/13/2014 11:29 AM, Ming Lei wrote:
> Hi,
> 
> This patches try to support multi virtual queues(multi-vq) in one
> virtio-blk device, and maps each virtual queue(vq) to blk-mq's
> hardware queue.
> 
> With this approach, both scalability and performance problems on
> virtio-blk device get improved.
> 
> For verifying the improvement, I implements virtio-blk multi-vq over
> qemu's dataplane feature, and both handling host notification
> from each vq and processing host I/O are still kept in the per-device
> iothread context, the changes are based on qemu v2.0.0 release, and
> can be accessed from below tree:
> 
> 	git://kernel.ubuntu.com/ming/qemu.git #v2.0.0-virtblk-dataplane-mq
> 
> For enabling the multi-vq feature, 'num_queues=N' need to be added into
> '-device virtio-blk-pci ...' of qemu command line, and suggest to pass
> 'vectors=N+1' to keep one MSI irq vector per each vq, and the feature
> depends on x-data-plane.
> 
> Fio(libaio, randread, iodepth=64, bs=4K, jobs=N) is run inside VM to
> verify the improvement.
> 
> I just create a small quadcore VM and run fio inside the VM, and
> num_queues of the virtio-blk device is set as 2, but looks the
> improvement is still obvious.
> 
> 1), about scalability
> - without mutli-vq feature
> 	-- jobs=2, thoughput: 145K iops
> 	-- jobs=4, thoughput: 100K iops
> - without mutli-vq feature
> 	-- jobs=2, thoughput: 186K iops
> 	-- jobs=4, thoughput: 199K iops

Awesome! I was hoping someone would do that, and make virtio-blk take
full advantage of blk-mq.

-- 
Jens Axboe