bcache queue limit cleanups

bcache queue limit cleanups

[Christoph Hellwig]

Hi all,

this patch against Jens' for-6.9/block tree gets rid of the last
queue limit update in bcache by calculation the io_opt ahead of
time.

[PATCH] bcache: move calculation of stripe_size and io_opt into bcache_device_init

[Christoph Hellwig]

bcache currently calculates the stripe size for the non-cached_dev
case directly in bcache_device_init, but for the cached_dev case it does
it in the caller.  Consolidate it in one places, which also enables
setting the io_opt queue_limit before allocating the gendisk so that it
can be passed in instead of changing the limit just after the allocation.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/bcache/super.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index d06a9649d30269..f716c3265f5cf0 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -913,6 +913,10 @@ static int bcache_device_init(struct bcache_device *d, unsigned int block_size,
 	uint64_t n;
 	int idx;
 
+	if (cached_bdev) {
+		d->stripe_size = bdev_io_opt(cached_bdev) >> SECTOR_SHIFT;
+		lim.io_opt = umax(block_size, bdev_io_opt(cached_bdev));
+	}
 	if (!d->stripe_size)
 		d->stripe_size = 1 << 31;
 	else if (d->stripe_size < BCH_MIN_STRIPE_SZ)
@@ -1418,9 +1422,7 @@ static int cached_dev_init(struct cached_dev *dc, unsigned int block_size)
 		hlist_add_head(&io->hash, dc->io_hash + RECENT_IO);
 	}
 
-	dc->disk.stripe_size = q->limits.io_opt >> 9;
-
-	if (dc->disk.stripe_size)
+	if (bdev_io_opt(dc->bdev))
 		dc->partial_stripes_expensive =
 			q->limits.raid_partial_stripes_expensive;
 
@@ -1430,9 +1432,6 @@ static int cached_dev_init(struct cached_dev *dc, unsigned int block_size)
 	if (ret)
 		return ret;
 
-	blk_queue_io_opt(dc->disk.disk->queue,
-		max(queue_io_opt(dc->disk.disk->queue), queue_io_opt(q)));
-
 	atomic_set(&dc->io_errors, 0);
 	dc->io_disable = false;
 	dc->error_limit = DEFAULT_CACHED_DEV_ERROR_LIMIT;
-- 
2.39.2

Re: bcache queue limit cleanups

[Christoph Hellwig]

On Mon, Feb 26, 2024 at 11:48:25AM +0100, Christoph Hellwig wrote:
> this patch against Jens' for-6.9/block tree gets rid of the last
> queue limit update in bcache by calculation the io_opt ahead of
> time.

Any chance to get this patch reviewed?  It is one of just two
queue limits API parts that still isn't reviewed.

Re: [PATCH] bcache: move calculation of stripe_size and io_opt into bcache_device_init

[Coly Li]

On Mon, Feb 26, 2024 at 11:48:26AM +0100, Christoph Hellwig wrote:
> bcache currently calculates the stripe size for the non-cached_dev
> case directly in bcache_device_init, but for the cached_dev case it does
> it in the caller.  Consolidate it in one places, which also enables
> setting the io_opt queue_limit before allocating the gendisk so that it
> can be passed in instead of changing the limit just after the allocation.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good to me.

Reviewed-by: Coly Li <colyli@suse.de>

Thanks.


Coly Li


> ---
>  drivers/md/bcache/super.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index d06a9649d30269..f716c3265f5cf0 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -913,6 +913,10 @@ static int bcache_device_init(struct bcache_device *d, unsigned int block_size,
>  	uint64_t n;
>  	int idx;
>  
> +	if (cached_bdev) {
> +		d->stripe_size = bdev_io_opt(cached_bdev) >> SECTOR_SHIFT;
> +		lim.io_opt = umax(block_size, bdev_io_opt(cached_bdev));
> +	}
>  	if (!d->stripe_size)
>  		d->stripe_size = 1 << 31;
>  	else if (d->stripe_size < BCH_MIN_STRIPE_SZ)
> @@ -1418,9 +1422,7 @@ static int cached_dev_init(struct cached_dev *dc, unsigned int block_size)
>  		hlist_add_head(&io->hash, dc->io_hash + RECENT_IO);
>  	}
>  
> -	dc->disk.stripe_size = q->limits.io_opt >> 9;
> -
> -	if (dc->disk.stripe_size)
> +	if (bdev_io_opt(dc->bdev))
>  		dc->partial_stripes_expensive =
>  			q->limits.raid_partial_stripes_expensive;
>  
> @@ -1430,9 +1432,6 @@ static int cached_dev_init(struct cached_dev *dc, unsigned int block_size)
>  	if (ret)
>  		return ret;
>  
> -	blk_queue_io_opt(dc->disk.disk->queue,
> -		max(queue_io_opt(dc->disk.disk->queue), queue_io_opt(q)));
> -
>  	atomic_set(&dc->io_errors, 0);
>  	dc->io_disable = false;
>  	dc->error_limit = DEFAULT_CACHED_DEV_ERROR_LIMIT;

Re: bcache queue limit cleanups

[Coly Li]

> 2024年3月3日 23:12，Christoph Hellwig <hch@lst.de> 写道：
> 
> On Mon, Feb 26, 2024 at 11:48:25AM +0100, Christoph Hellwig wrote:
>> this patch against Jens' for-6.9/block tree gets rid of the last
>> queue limit update in bcache by calculation the io_opt ahead of
>> time.
> 
> Any chance to get this patch reviewed?  It is one of just two
> queue limits API parts that still isn't reviewed.
> 
> 

Done. Another patch was already in Jens’ tree, I guess it might be late to add my Reviewed-by.

Thanks.

Coly Li

Re: [PATCH] bcache: move calculation of stripe_size and io_opt into bcache_device_init

[Jens Axboe]

On Mon, 26 Feb 2024 11:48:26 +0100, Christoph Hellwig wrote:
> bcache currently calculates the stripe size for the non-cached_dev
> case directly in bcache_device_init, but for the cached_dev case it does
> it in the caller.  Consolidate it in one places, which also enables
> setting the io_opt queue_limit before allocating the gendisk so that it
> can be passed in instead of changing the limit just after the allocation.
> 
> 
> [...]

Applied, thanks!

[1/1] bcache: move calculation of stripe_size and io_opt into bcache_device_init
      commit: 34a2cf3fbef17deee2d4d28c41e3cb8ac1929fda

Best regards,
-- 
Jens Axboe