Skip to contentSkip to conversation

rbd: set max_sectors explicitly

PATCH In review Ilya Dryomov 2 participants 3 messages Oct 7, 2015 – Oct 16, 2015 1 message not in the archive

Subsystems (2)
Block layer RADOS block device (RBD)
lore
Share & tools
lore

Ilya Dryomov

author maintainer
#0 lore raw

PATCH 1/1 rbd: set max_sectors explicitly

Commit 30e2bc08b2bb ("Revert "block: remove artifical max_hw_sectors cap"") restored a clamp on max_sectors. It's now 2560 sectors instead of 1024, but it's not good enough: we set max_hw_sectors to rbd object size because we don't want object sized I/Os to be split, and the default object size is 4M.

So, set max_sectors to max_hw_sectors in rbd at queue init time.

Signed-off-by Ilya Dryomov <[email protected]>
---
 drivers/block/rbd.c | 1 +
 1 file changed, 1 insertion(+)
drivers/block/rbd.c +1 -0
index 05072464d25e..04e69b4df664 100644
@@ -3760,6 +3760,7 @@ static int rbd_init_disk(struct rbd_device *rbd_dev)
 	/* set io sizes to object size */
 	segment_size = rbd_obj_bytes(&rbd_dev->header);
 	blk_queue_max_hw_sectors(q, segment_size / SECTOR_SIZE);
+	q->limits.max_sectors = queue_max_hw_sectors(q);
 	blk_queue_max_segments(q, segment_size / SECTOR_SIZE);
 	blk_queue_max_segment_size(q, segment_size);
 	blk_queue_io_min(q, segment_size);
Signature · 1 line
2.4.3

A message that is not in the archive (sent off-list or not yet received): [email protected]

Reviewed-by
#2 lore raw

Forgot to "Reply-all". -Alex

-------- Forwarded Message --------
Subject: Re: [PATCH] rbd: set max_sectors explicitly
Date: Fri, 16 Oct 2015 07:22:55 -0500
From: Alex Elder <[email protected]>
To: Ilya Dryomov <[email protected]>

Quoted 18 lines · Ilya Dryomov

Commit 30e2bc08b2bb ("Revert "block: remove artifical max_hw_sectors cap"") restored a clamp on max_sectors. It's now 2560 sectors instead of 1024, but it's not good enough: we set max_hw_sectors to rbd object size because we don't want object sized I/Os to be split, and the default object size is 4M.

So, set max_sectors to max_hw_sectors in rbd at queue init time.

Signed-off-by Ilya Dryomov <[email protected]>
---
 drivers/block/rbd.c | 1 +
 1 file changed, 1 insertion(+)
drivers/block/rbd.c +0 -0
index 05072464d25e..04e69b4df664 100644

drivers/block/rbd.c static int rbd_init_disk(struct rbd_device *rbd_dev) · line 3763

Quoted 4 lines
drivers/block/rbd.c +1 -0
@@ -3760,6 +3760,7 @@ static int rbd_init_disk(struct rbd_device *rbd_dev)
 	/* set io sizes to object size */
 	segment_size = rbd_obj_bytes(&rbd_dev->header);
 	blk_queue_max_hw_sectors(q, segment_size / SECTOR_SIZE);
+	q->limits.max_sectors = queue_max_hw_sectors(q);

This currently is done by default by blk_queue_max_hw_sectors(). Do you see any different behavior with this patch in place?

This change seems at least harmless so if it's not too late:

Reviewed-by Alex Elder <[email protected]>
Quoted 4 lines
drivers/block/rbd.c +0 -0
 	blk_queue_max_segments(q, segment_size / SECTOR_SIZE);
 	blk_queue_max_segment_size(q, segment_size);
 	blk_queue_io_min(q, segment_size);
author maintainer
#3 lore raw
In reply to a message not in the archive
Quoted 27 lines · Alex Elder
Quoted 18 lines · Ilya Dryomov

Commit 30e2bc08b2bb ("Revert "block: remove artifical max_hw_sectors cap"") restored a clamp on max_sectors. It's now 2560 sectors instead of 1024, but it's not good enough: we set max_hw_sectors to rbd object size because we don't want object sized I/Os to be split, and the default object size is 4M.

So, set max_sectors to max_hw_sectors in rbd at queue init time.

Signed-off-by Ilya Dryomov <[email protected]>
---
 drivers/block/rbd.c | 1 +
 1 file changed, 1 insertion(+)
drivers/block/rbd.c +0 -0
index 05072464d25e..04e69b4df664 100644

drivers/block/rbd.c static int rbd_init_disk(struct rbd_device *rbd_dev) · line 3763

Quoted 4 lines
drivers/block/rbd.c +1 -0
@@ -3760,6 +3760,7 @@ static int rbd_init_disk(struct rbd_device *rbd_dev)
      /* set io sizes to object size */
      segment_size = rbd_obj_bytes(&rbd_dev->header);
      blk_queue_max_hw_sectors(q, segment_size / SECTOR_SIZE);
+     q->limits.max_sectors = queue_max_hw_sectors(q);

This currently is done by default by blk_queue_max_hw_sectors(). Do you see any different behavior with this patch in place?

Quoted 1 line

This change seems at least harmless so if it's not too late:

Not quite. The commit mentioned above reintroduced this bit into blk_limits_max_hw_sectors() in 4.3-rc1:

241 limits->max_sectors = min_t(unsigned int, max_hw_sectors,
242 BLK_DEF_MAX_SECTORS);

So this patch fixes a performance regression. If you spin up 4.3-rc1+ and map an rbd image without changing any settings, the best you'll see is 1280k I/Os despite a 4096k (typically) max_hw_sectors.

Thanks,

Ilya