use larger max_request_size for virtio_blk

RFC PATCH In review Weiping Zhang 4 participants 6 messages Apr 5, 2018 – Apr 7, 2018

Subsystems (3)

Block layer virtio core virtio block and SCSI drivers

lore

Share & tools

lore

Weiping Zhang

author

2018-04-05 10:09 UTC #0 lore raw

cover use larger max_request_size for virtio_blk

Hi,

For virtio block device, actually there is no a hard limit for max request size, and virtio_blk driver set -1 to blk_queue_max_hw_sectors(q, -1U);. But it doesn't work, because there is a default upper limitation BLK_DEF_MAX_SECTORS (1280 sectors). So this series want to add a new helper blk_queue_max_hw_sectors_no_limit to set a proper max reqeust size.

Weiping Zhang (2):
blk-setting: add new helper blk_queue_max_hw_sectors_no_limit
virtio_blk: add new module parameter to set max request size

 block/blk-settings.c       | 20 ++++++++++++++++++++
 drivers/block/virtio_blk.c | 32 ++++++++++++++++++++++++++++++--
 include/linux/blkdev.h     |  2 ++
 3 files changed, 52 insertions(+), 2 deletions(-)

Signature · 1 line

2.9.4

Weiping Zhang

author

2018-04-05 10:10 UTC #1 lore raw

RFC PATCH 1/2 blk-setting: add new helper blk_queue_max_hw_sectors_no_limit

There is a default upper limitation BLK_DEF_MAX_SECTORS, but for some virtual block device driver there is no such limitation. So add a new help to set max request size.

Signed-off-by Weiping Zhang <[email protected]>

---
 block/blk-settings.c   | 20 ++++++++++++++++++++
 include/linux/blkdev.h |  2 ++
 2 files changed, 22 insertions(+)

block/blk-settings.c +20 -0

index 48ebe6b..685c30c 100644
@@ -253,6 +253,26 @@ void blk_queue_max_hw_sectors(struct request_queue *q, unsigned int max_hw_secto
 }
 EXPORT_SYMBOL(blk_queue_max_hw_sectors);
 
+/* same as blk_queue_max_hw_sectors but without default upper limitation */
+void blk_queue_max_hw_sectors_no_limit(struct request_queue *q,
+		unsigned int max_hw_sectors)
+{
+	struct queue_limits *limits = &q->limits;
+	unsigned int max_sectors;
+
+	if ((max_hw_sectors << 9) < PAGE_SIZE) {
+		max_hw_sectors = 1 << (PAGE_SHIFT - 9);
+		printk(KERN_INFO "%s: set to minimum %d\n",
+		       __func__, max_hw_sectors);
+	}
+
+	limits->max_hw_sectors = max_hw_sectors;
+	max_sectors = min_not_zero(max_hw_sectors, limits->max_dev_sectors);
+	limits->max_sectors = max_sectors;
+	q->backing_dev_info->io_pages = max_sectors >> (PAGE_SHIFT - 9);
+}
+EXPORT_SYMBOL(blk_queue_max_hw_sectors_no_limit);
+
 /**
  * blk_queue_chunk_sectors - set size of the chunk for this queue
  * @q:  the request queue for the device

include/linux/blkdev.h +2 -0

index ed63f3b..2250709 100644
@@ -1243,6 +1243,8 @@ extern void blk_cleanup_queue(struct request_queue *);
 extern void blk_queue_make_request(struct request_queue *, make_request_fn *);
 extern void blk_queue_bounce_limit(struct request_queue *, u64);
 extern void blk_queue_max_hw_sectors(struct request_queue *, unsigned int);
+extern void blk_queue_max_hw_sectors_no_limit(struct request_queue *,
+						unsigned int);
 extern void blk_queue_chunk_sectors(struct request_queue *, unsigned int);
 extern void blk_queue_max_segments(struct request_queue *, unsigned short);
 extern void blk_queue_max_discard_segments(struct request_queue *,

Signature · 1 line

2.9.4

Weiping Zhang

author

2018-04-05 10:10 UTC #2 lore raw

RFC PATCH 2/2 virtio_blk: add new module parameter to set max request size

In reply to Weiping Zhang

Actually there is no upper limitation, so add new module parameter to provide a way to set a proper max request size for virtio block. Using a larger request size can improve sequence performance in theory, and reduce the interaction between guest and hypervisor.

Signed-off-by Weiping Zhang <[email protected]>

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

drivers/block/virtio_blk.c +30 -2

index 4a07593c..5ac6d59 100644
@@ -64,6 +64,34 @@ struct virtblk_req {
 	struct scatterlist sg[];
 };
 
+
+static int max_request_size_set(const char *val, const struct kernel_param *kp);
+
+static const struct kernel_param_ops max_request_size_ops = {
+	.set = max_request_size_set,
+	.get = param_get_uint,
+};
+
+static unsigned long max_request_size = 4096; /* in unit of KiB */
+module_param_cb(max_request_size, &max_request_size_ops, &max_request_size,
+		0444);
+MODULE_PARM_DESC(max_request_size, "set max request size, in unit of KiB");
+
+static int max_request_size_set(const char *val, const struct kernel_param *kp)
+{
+	int ret;
+	unsigned int size_kb, page_kb = 1 << (PAGE_SHIFT - 10);
+
+	ret = kstrtouint(val, 10, &size_kb);
+	if (ret != 0)
+		return -EINVAL;
+
+	if (size_kb < page_kb)
+		return -EINVAL;
+
+	return param_set_uint(val, kp);
+}
+
 static inline blk_status_t virtblk_result(struct virtblk_req *vbr)
 {
 	switch (vbr->status) {

@@ -730,8 +758,8 @@ static int virtblk_probe(struct virtio_device *vdev)
 	/* We can handle whatever the host told us to handle. */
 	blk_queue_max_segments(q, vblk->sg_elems-2);
 
-	/* No real sector limit. */
-	blk_queue_max_hw_sectors(q, -1U);
+	/* No real sector limit, use 512b (max_request_size << 10) >> 9 */
+	blk_queue_max_hw_sectors_no_limit(q, max_request_size << 1);
 
 	/* Host can optionally specify maximum segment size and number of
 	 * segments. */

Signature · 1 line

2.9.4

Martin K. Petersen

2018-04-05 12:24 UTC #3 lore raw

In reply to Weiping Zhang

Weiping,

Quoted 4 lines

For virtio block device, actually there is no a hard limit for max request size, and virtio_blk driver set -1 to blk_queue_max_hw_sectors(q, -1U);. But it doesn't work, because there is a default upper limitation BLK_DEF_MAX_SECTORS (1280 sectors).

That's intentional (although it's an ongoing debate what the actual value should be).

Quoted 2 lines

So this series want to add a new helper blk_queue_max_hw_sectors_no_limit to set a proper max reqeust size.

BLK_DEF_MAX_SECTORS is a kernel default empirically chosen to strike a decent balance between I/O latency and bandwidth. It sets an upper bound for filesystem requests only. Regardless of the capabilities of the block device driver and underlying hardware.

You can override the limit on a per-device basis via max_sectors_kb in sysfs. People generally do it via a udev rule.

Signature · 1 line

Martin K. Petersen	Oracle Linux Engineering

Jens Axboe

maintainer

2018-04-05 14:29 UTC #4 lore raw

In reply to Weiping Zhang

Quoted 8 lines · Weiping Zhang

Hi,
For virtio block device, actually there is no a hard limit for max request size, and virtio_blk driver set -1 to blk_queue_max_hw_sectors(q, -1U);. But it doesn't work, because there is a default upper limitation BLK_DEF_MAX_SECTORS (1280 sectors). So this series want to add a new helper blk_queue_max_hw_sectors_no_limit to set a proper max reqeust size.

Quoted 8 lines

Weiping Zhang (2):
blk-setting: add new helper blk_queue_max_hw_sectors_no_limit
virtio_blk: add new module parameter to set max request size
 block/blk-settings.c       | 20 ++++++++++++++++++++
 drivers/block/virtio_blk.c | 32 ++++++++++++++++++++++++++++++--
 include/linux/blkdev.h     |  2 ++
 3 files changed, 52 insertions(+), 2 deletions(-)

The driver should just use blk_queue_max_hw_sectors() to set the limit, and then the soft limit can be modified by a udev rule. Technically the driver doesn't own the software limit, it's imposed to ensure that we don't introduce too much latency per request.

Your situation is no different from many other setups, where the hw limit is much higher than the default 1280k.

Signature · 1 line

Jens Axboe

Weiping Zhang

2018-04-07 08:30 UTC #5 lore raw

2018-04-05 22:29 GMT+08:00 Jens Axboe <[email protected]>:

Quoted 18 lines · Weiping Zhang

Quoted 8 lines
Hi,
For virtio block device, actually there is no a hard limit for max request size, and virtio_blk driver set -1 to blk_queue_max_hw_sectors(q, -1U);. But it doesn't work, because there is a default upper limitation BLK_DEF_MAX_SECTORS (1280 sectors). So this series want to add a new helper blk_queue_max_hw_sectors_no_limit to set a proper max reqeust size.
Quoted 8 lines
Weiping Zhang (2):
blk-setting: add new helper blk_queue_max_hw_sectors_no_limit
virtio_blk: add new module parameter to set max request size
 block/blk-settings.c       | 20 ++++++++++++++++++++
 drivers/block/virtio_blk.c | 32 ++++++++++++++++++++++++++++++--
 include/linux/blkdev.h     |  2 ++
 3 files changed, 52 insertions(+), 2 deletions(-)

Quoted 8 lines

The driver should just use blk_queue_max_hw_sectors() to set the limit, and then the soft limit can be modified by a udev rule. Technically the driver doesn't own the software limit, it's imposed to ensure that we don't introduce too much latency per request.
Your situation is no different from many other setups, where the hw limit is much higher than the default 1280k.

Hi Martin, Jens,

It seems more reasonable to change software limitation by udev rule, thanks you.

Quoted 5 lines

_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization