Re: [RFC v2 09/11] block/bdev: lift block size restrictions and use common definition

Hannes Reinecke
2024-12-17T10:05:52Z

On 12/14/24 04:10, Luis Chamberlain wrote:
> We now can support blocksizes larger than PAGE_SIZE, so lift
> the restriction up to the max supported page cache order and
> just bake this into a common helper used by the block layer.
>
> We bound ourselves to 64k as a sensible limit. The hard limit,
> however is 1 << (PAGE_SHIFT + MAX_PAGECACHE_ORDER).
>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
> block/bdev.c | 5 ++---
> include/linux/blkdev.h | 11 ++++++++++-
> 2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/block/bdev.c b/block/bdev.c
> index 167d82b46781..b57dc4bff81b 100644
> --- a/block/bdev.c
> +++ b/block/bdev.c
> @@ -157,8 +157,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 */
> @@ -185,7 +184,7 @@ int sb_set_blocksize(struct super_block *sb, int size)
> if (set_blocksize(sb->s_bdev_file, size))
> return 0;
> /* If we get here, we know size is power of two
> - * and it's value is between 512 and PAGE_SIZE */
> + * and its value is larger than 512 */
> sb->s_blocksize = size;
> sb->s_blocksize_bits = blksize_bits(size);
> return sb->s_blocksize;
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 08a727b40816..a7303a55ed2a 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -269,10 +269,19 @@ static inline dev_t disk_devt(struct gendisk *disk)
> return MKDEV(disk->major, disk->first_minor);
> }
>
> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> +/*
> + * The hard limit is (1 << (PAGE_SHIFT + MAX_PAGECACHE_ORDER).
As this is the hard limit, one wonders why we don't use it ...
So please add a comment why we restrict it to 64k.

> + */
> +#define BLK_MAX_BLOCK_SIZE (SZ_64K)
> +#else
> +#define BLK_MAX_BLOCK_SIZE (PAGE_SIZE)
> +#endif
> +
> /* blk_validate_limits() validates bsize, so drivers don't usually need to */
> static inline int blk_validate_block_size(unsigned long bsize)
> {
> - if (bsize < 512 || bsize > PAGE_SIZE || !is_power_of_2(bsize))
> + if (bsize < 512 || bsize > BLK_MAX_BLOCK_SIZE || !is_power_of_2(bsize))
> return -EINVAL;
>
> return 0;

Otherwise:
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich