[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

[Jens Axboe]

On 12/18/24 7:22 AM, Jens Axboe wrote:
> 
> 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

JFYI, gmail puts all your emails into spam, because it can't
authenticate them. This seemingly happens to everybody that uses
kernel.org as the email, if they haven't updated to the newer setup. You
should fix it, I only saw the emails because others replied.

And please trim the way excessive CC lists on patches like this.
Cleanups don't need to go to tons of unrelated lists or people.

-- 
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>