[PATCH] rbd: set max_sectors explicitly

[PATCH] rbd: set max_sectors explicitly

[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 <idryomov@gmail.com>
---
 drivers/block/rbd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 05072464d25e..04e69b4df664 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -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);
-- 
2.4.3

Fwd: Re: [PATCH] rbd: set max_sectors explicitly

[Alex Elder]

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 <elder@ieee.org>
To: Ilya Dryomov <idryomov@gmail.com>

On 10/07/2015 12:00 PM, Ilya Dryomov wrote:
> 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 <idryomov@gmail.com>
> ---
>  drivers/block/rbd.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 05072464d25e..04e69b4df664 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -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 <elder@linaro.org>

>  	blk_queue_max_segments(q, segment_size / SECTOR_SIZE);
>  	blk_queue_max_segment_size(q, segment_size);
>  	blk_queue_io_min(q, segment_size);
> 

Re: [PATCH] rbd: set max_sectors explicitly

[Ilya Dryomov]

On Fri, Oct 16, 2015 at 2:22 PM, Alex Elder <elder@ieee.org> wrote:
> On 10/07/2015 12:00 PM, Ilya Dryomov wrote:
>> 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 <idryomov@gmail.com>
>> ---
>>  drivers/block/rbd.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
>> index 05072464d25e..04e69b4df664 100644
>> --- a/drivers/block/rbd.c
>> +++ b/drivers/block/rbd.c
>> @@ -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:

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