Re: [RF[CRAP] 2/2] xfs: stop using set_blocksize

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Apr 15, 2025 at 09:46:09PM -0700, Christoph Hellwig wrote:
> 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?

Ok.

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

When even is it null?  I thought it would either be the holder or
bdev_inode if not.

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

Ok.  bdev_validate_block_size is a much better name for a tighter
function...

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

...and xfs can call sync_blockdev directly from xfs_setsize_buftarg.
I imagine we still want any dirty pagecache to get flushed before we
start submitting our own read bios.

--D