On Mon, Apr 14, 2025 at 05:33:08PM -0700, Darrick J. Wong wrote:
> +/*
> + * For bdev filesystems that do not use buffer heads, check that this block
> + * size is acceptable and flush dirty pagecache to disk.
> + */
Can you turn this into a full fledged kerneldoc comment?
> +int bdev_use_blocksize(struct file *file, int size)
> +{
> + struct inode *inode = file->f_mapping->host;
> + struct block_device *bdev = I_BDEV(inode);
> +
> + if (blk_validate_block_size(size))
> + return -EINVAL;
> +
> + /* Size cannot be smaller than the size supported by the device */
> + if (size < bdev_logical_block_size(bdev))
> + return -EINVAL;
> +
> + if (!file->private_data)
> + return -EINVAL;
This private_data check looks really confusing. Looking it up I see
that it is directly copied from set_blocksize, but it could really
use a comment. Or in fact be removed here and kept in set_blocksize
only as we don't care about an exclusive opener at all. Even there
a comment would be rather helpful, though.
> +
> + return sync_blockdev(bdev);
> +}
I don't think we need sync_blockdev here as we don't touch the
bdev page cache. Maybe XFS wants to still call it, but it feels
wrong in a helper just validating the block size.
So maybe drop it, rename the helper to bdev_validate_block_size
and use it in set_blocksize instead of duplicating the logic?
> + error = bdev_use_blocksize(btp->bt_bdev_file, sectorsize);
.. and then split using it in XFS into a separate patch from adding
the block layer helper.
