Linux Virtualization - Mar 2014 - [PATCH] virtio-blk: Initialize blkqueue depth from virtqueue size

virtio-blk set the default queue depth to 64 requests, which was
insufficient for high-IOPS devices. Instead set the blk-queue depth to
the device's virtqueue depth divided by two (each I/O requires at least
two VQ entries).

Signed-off-by: Venkatesh Srinivas <venkateshs at google.com>
---
 drivers/block/virtio_blk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index b1cb3f4..863ab02 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -485,7 +485,6 @@ static struct blk_mq_ops virtio_mq_ops = {
 static struct blk_mq_reg virtio_mq_reg = {
 	.ops		= &virtio_mq_ops,
 	.nr_hw_queues	= 1,
-	.queue_depth	= 64,
 	.numa_node	= NUMA_NO_NODE,
 	.flags		= BLK_MQ_F_SHOULD_MERGE,
 };
@@ -555,6 +554,7 @@ static int virtblk_probe(struct virtio_device *vdev)
 	virtio_mq_reg.cmd_size  		sizeof(struct virtblk_req) +
 		sizeof(struct scatterlist) * sg_elems;
+	virtio_mq_reg.queue_depth = vblk->vq->num_free / 2;
 
 	q = vblk->disk->queue = blk_mq_init_queue(&virtio_mq_reg, vblk);
 	if (!q) {
-- 
1.9.0.279.gdc9e3eb

The current virtio block sets a queue depth of 64, which is
insufficient for very fast devices.  It has been demonstrated that
with a high IOPS device, using a queue depth of 256 can double the
IOPS which can be sustained.

As suggested by Venkatash Srinivas, set the queue depth by default to
be one half the the device's virtqueue, which is the maximum queue
depth that can be supported by the channel to the host OS (each I/O
request requires at least two VQ entries).

Also allow the queue depth to be something which can be set at module
load time or via a kernel boot-time parameter, for
testing/benchmarking purposes.

Signed-off-by: "Theodore Ts'o" <tytso at mit.edu>
Signed-off-by: Venkatesh Srinivas <venkateshs at google.com>
Cc: Rusty Russell <rusty at rustcorp.com.au>
Cc: "Michael S. Tsirkin" <mst at redhat.com>
Cc: virtio-dev at lists.oasis-open.org
Cc: virtualization at lists.linux-foundation.org
Cc: Frank Swiderski <fes at google.com>
---

This is a combination of my patch and Vekatash's patch.  I agree that
setting the default automatically is better than requiring the user to
set the value by hand.

 drivers/block/virtio_blk.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 6a680d4..0f70c01 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -481,6 +481,9 @@ static struct blk_mq_ops virtio_mq_ops = {
 	.free_hctx	= blk_mq_free_single_hw_queue,
 };
 
+static int queue_depth = -1;
+module_param(queue_depth, int, 0444);
+
 static struct blk_mq_reg virtio_mq_reg = {
 	.ops		= &virtio_mq_ops,
 	.nr_hw_queues	= 1,
@@ -551,9 +554,14 @@ static int virtblk_probe(struct virtio_device *vdev)
 		goto out_free_vq;
 	}
 
+	virtio_mq_reg.queue_depth = queue_depth > 0 ? queue_depth :
+		(vblk->vq->num_free / 2);
 	virtio_mq_reg.cmd_size  		sizeof(struct virtblk_req) +
 		sizeof(struct scatterlist) * sg_elems;
+	virtblk_name_format("vd", index, vblk->disk->disk_name,
DISK_NAME_LEN);
+	pr_info("%s: using queue depth %d\n", vblk->disk->disk_name,
+		virtio_mq_reg.queue_depth);
 
 	q = vblk->disk->queue = blk_mq_init_queue(&virtio_mq_reg, vblk);
 	if (!q) {
@@ -565,8 +573,6 @@ static int virtblk_probe(struct virtio_device *vdev)
 
 	q->queuedata = vblk;
 
-	virtblk_name_format("vd", index, vblk->disk->disk_name,
DISK_NAME_LEN);
-
 	vblk->disk->major = major;
 	vblk->disk->first_minor = index_to_minor(index);
 	vblk->disk->private_data = vblk;
-- 
1.9.0

On March 14, 2014 11:34:31 PM EDT, Theodore Ts'o <tytso at mit.edu>
wrote:
>The current virtio block sets a queue depth of 64, which is
>insufficient for very fast devices.  It has been demonstrated that
>with a high IOPS device, using a queue depth of 256 can double the
>IOPS which can be sustained.
>
>As suggested by Venkatash Srinivas, set the queue depth by default to
>be one half the the device's virtqueue, which is the maximum queue
>depth that can be supported by the channel to the host OS (each I/O
>request requires at least two VQ entries).
>
>Also allow the queue depth to be something which can be set at module
>load time or via a kernel boot-time parameter, for
>testing/benchmarking purposes.
>
>Signed-off-by: "Theodore Ts'o" <tytso at mit.edu>
>Signed-off-by: Venkatesh Srinivas <venkateshs at google.com>
>Cc: Rusty Russell <rusty at rustcorp.com.au>
>Cc: "Michael S. Tsirkin" <mst at redhat.com>
>Cc: virtio-dev at lists.oasis-open.org
>Cc: virtualization at lists.linux-foundation.org
>Cc: Frank Swiderski <fes at google.com>
>---
>
>This is a combination of my patch and Vekatash's patch.  I agree that
>setting the default automatically is better than requiring the user to
>set the value by hand.
>
> drivers/block/virtio_blk.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
>index 6a680d4..0f70c01 100644
>--- a/drivers/block/virtio_blk.c
>+++ b/drivers/block/virtio_blk.c
>@@ -481,6 +481,9 @@ static struct blk_mq_ops virtio_mq_ops = {
> 	.free_hctx	= blk_mq_free_single_hw_queue,
> };
> 
>+static int queue_depth = -1;
>+module_param(queue_depth, int, 0444);
?
>+
> static struct blk_mq_reg virtio_mq_reg = {
> 	.ops		= &virtio_mq_ops,
> 	.nr_hw_queues	= 1,
>@@ -551,9 +554,14 @@ static int virtblk_probe(struct virtio_device
>*vdev)
> 		goto out_free_vq;
> 	}
> 
>+	virtio_mq_reg.queue_depth = queue_depth > 0 ? queue_depth :
>+		(vblk->vq->num_free / 2);
> 	virtio_mq_reg.cmd_size > 		sizeof(struct virtblk_req) +
> 		sizeof(struct scatterlist) * sg_elems;
>+	virtblk_name_format("vd", index, vblk->disk->disk_name,
>DISK_NAME_LEN);
>+	pr_info("%s: using queue depth %d\n",
vblk->disk->disk_name,
>+		virtio_mq_reg.queue_depth);
Isn't that visible from sysfs?
> 
> 	q = vblk->disk->queue = blk_mq_init_queue(&virtio_mq_reg, vblk);
> 	if (!q) {
>@@ -565,8 +573,6 @@ static int virtblk_probe(struct virtio_device
>*vdev)
> 
> 	q->queuedata = vblk;
> 
>-	virtblk_name_format("vd", index, vblk->disk->disk_name,
>DISK_NAME_LEN);
>-
> 	vblk->disk->major = major;
> 	vblk->disk->first_minor = index_to_minor(index);
> 	vblk->disk->private_data = vblk;

On Fri, Mar 14, 2014 at 11:34:31PM -0400, Theodore Ts'o
wrote:
> The current virtio block sets a queue depth of 64, which is
> insufficient for very fast devices.  It has been demonstrated that
> with a high IOPS device, using a queue depth of 256 can double the
> IOPS which can be sustained.
> 
> As suggested by Venkatash Srinivas, set the queue depth by default to
> be one half the the device's virtqueue, which is the maximum queue
> depth that can be supported by the channel to the host OS (each I/O
> request requires at least two VQ entries).
I don't think this should be a module parameter.  The default sizing
should be based of the parameters of the actual virtqueue, and if we
want to allow tuning it it should be by a sysfs attribute, preferable
using the same semantics as SCSI.

Theodore Ts'o <tytso at mit.edu> writes:
> The current virtio block sets a queue depth of 64, which is
> insufficient for very fast devices.  It has been demonstrated that
> with a high IOPS device, using a queue depth of 256 can double the
> IOPS which can be sustained.
>
> As suggested by Venkatash Srinivas, set the queue depth by default to
> be one half the the device's virtqueue, which is the maximum queue
> depth that can be supported by the channel to the host OS (each I/O
> request requires at least two VQ entries).
>
> Also allow the queue depth to be something which can be set at module
> load time or via a kernel boot-time parameter, for
> testing/benchmarking purposes.
Note that with indirect descriptors (which is supported by Almost
Everyone), we can actually use the full index, so this value is a bit
pessimistic.  But it's OK as a starting point.

Cheers,
Rusty.