[PATCH 0/2] block size limit cleanups

[PATCH 0/2] block size limit cleanups

[Luis Chamberlain]

This spins off two change which introduces no functional changes from the
bs > ps block device patch series [0]. These are just cleanups.

[0] https://lkml.kernel.org/r/20241214031050.1337920-1-mcgrof@kernel.org

Luis Chamberlain (2):
  block/bdev: use helper for max block size check
  nvme: use blk_validate_block_size() for max LBA check

 block/bdev.c             | 3 +--
 drivers/nvme/host/core.c | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

-- 
2.43.0

[PATCH 1/2] block/bdev: use helper for max block size check

[Luis Chamberlain]

We already have a helper for checking the limits on the block size
both low and high, just use that.

No functional changes.

Reviewed-by: John Garry <john.g.garry@oracle.com>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 block/bdev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/block/bdev.c b/block/bdev.c
index 738e3c8457e7..9d73a8fbf7f9 100644
--- a/block/bdev.c
+++ b/block/bdev.c
@@ -155,8 +155,7 @@ int set_blocksize(struct file *file, int size)
 	struct inode *inode = file->f_mapping->host;
 	struct block_device *bdev = I_BDEV(inode);
 
-	/* Size must be a power of two, and between 512 and PAGE_SIZE */
-	if (size > PAGE_SIZE || size < 512 || !is_power_of_2(size))
+	if (blk_validate_block_size(size))
 		return -EINVAL;
 
 	/* Size cannot be smaller than the size supported by the device */
-- 
2.43.0

[PATCH 2/2] nvme: use blk_validate_block_size() for max LBA check

[Luis Chamberlain]

The block layer already has support to validates proper block sizes
with blk_validate_block_size(), we can leverage that as well.

No functional changes.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/nvme/host/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index d169a30eb935..a970168a3014 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2034,7 +2034,7 @@ static bool nvme_update_disk_info(struct nvme_ns *ns, struct nvme_id_ns *id,
 	 * or smaller than a sector size yet, so catch this early and don't
 	 * allow block I/O.
 	 */
-	if (head->lba_shift > PAGE_SHIFT || head->lba_shift < SECTOR_SHIFT) {
+	if (blk_validate_block_size(bs)) {
 		bs = (1 << 9);
 		valid = false;
 	}
-- 
2.43.0

Re: [PATCH 2/2] nvme: use blk_validate_block_size() for max LBA check

[John Garry]

On 18/12/2024 02:02, Luis Chamberlain wrote:

nit: is this really an LBA check?

> The block layer already has support to validates proper block sizes
> with blk_validate_block_size(), we can leverage that as well.
> 
> No functional changes.
> 
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>

Most of my comments are minor/off topic, so:

Reviewed-by: John Garry <john.g.garry@oracle.com>

> ---
>   drivers/nvme/host/core.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index d169a30eb935..a970168a3014 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2034,7 +2034,7 @@ static bool nvme_update_disk_info(struct nvme_ns *ns, struct nvme_id_ns *id,
>   	 * or smaller than a sector size yet, so catch this early and don't
>   	 * allow block I/O.

I'm not sure if this comment is really still of value or can be reduced

>   	 */
> -	if (head->lba_shift > PAGE_SHIFT || head->lba_shift < SECTOR_SHIFT) {
> +	if (blk_validate_block_size(bs)) {

There was only a single user of this outside the block layer, and I was 
hoping that this function could eventually be internalized - not any longer.

>   		bs = (1 << 9);

comment on original code:

		bs = SECTOR_SIZE

seems more obvious

>   		valid = false;
>   	}

Re: [PATCH 0/2] block size limit cleanups

[Keith Busch]

On Tue, Dec 17, 2024 at 06:02:10PM -0800, Luis Chamberlain wrote:
> This spins off two change which introduces no functional changes from the
> bs > ps block device patch series [0]. These are just cleanups.

Looks good.

Reviewed-by: Keith Busch <kbusch@kernel.org>

Re: [PATCH 0/2] block size limit cleanups

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

(the CC list feels a little excessive, though)

Re: [PATCH 0/2] block size limit cleanups

[Jens Axboe]

On Tue, 17 Dec 2024 18:02:10 -0800, Luis Chamberlain wrote:
> This spins off two change which introduces no functional changes from the
> bs > ps block device patch series [0]. These are just cleanups.
> 
> [0] https://lkml.kernel.org/r/20241214031050.1337920-1-mcgrof@kernel.org
> 
> Luis Chamberlain (2):
>   block/bdev: use helper for max block size check
>   nvme: use blk_validate_block_size() for max LBA check
> 
> [...]

Applied, thanks!

[1/2] block/bdev: use helper for max block size check
      commit: 26fff8a4432ffd03409346b7dae1e1a2c5318b7c
[2/2] nvme: use blk_validate_block_size() for max LBA check
      commit: 51588b1b77b65cd0fb3440f78f37bef7178a2715

Best regards,
-- 
Jens Axboe

Re: [PATCH 0/2] block size limit cleanups

[Max Gurtovoy]

On 18/12/2024 4:02, Luis Chamberlain wrote:
> This spins off two change which introduces no functional changes from the
> bs > ps block device patch series [0]. These are just cleanups.
>
> [0] https://lkml.kernel.org/r/20241214031050.1337920-1-mcgrof@kernel.org
>
> Luis Chamberlain (2):
>    block/bdev: use helper for max block size check
>    nvme: use blk_validate_block_size() for max LBA check
>
>   block/bdev.c             | 3 +--
>   drivers/nvme/host/core.c | 2 +-
>   2 files changed, 2 insertions(+), 3 deletions(-)

Looks good,

Reviewed-by: Max Gurtovoy <mgurtovoy@nvidia.com>