Re: [PATCH 7/7] xfs: remove the bt_meta_sectorsize field in struct buftarg

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

On Tue, Jul 01, 2025 at 12:40:41PM +0200, Christoph Hellwig wrote:
> The file system only has a single file system sector size.  Read that
> from the in-core super block to avoid confusion about the two different
> "sector sizes" stored in the buftarg.

Yeah, that whole bt_meta_sectorsize stuff was confusing.

>                                        Note that this loosens the
> alignment asserts for memory backed buftargs that set the page size here.

Doesn't matter at all since shmem files are byte addressable anyway. :)

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_buf.c     | 20 +++++---------------
>  fs/xfs/xfs_buf.h     | 15 ---------------
>  fs/xfs/xfs_buf_mem.c |  2 --
>  3 files changed, 5 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index b73da43f489c..0f20d9514d0d 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -387,17 +387,18 @@ xfs_buf_map_verify(
>  	struct xfs_buftarg	*btp,
>  	struct xfs_buf_map	*map)
>  {
> +	struct xfs_mount	*mp = btp->bt_mount;
>  	xfs_daddr_t		eofs;
>  
>  	/* Check for IOs smaller than the sector size / not sector aligned */
> -	ASSERT(!(BBTOB(map->bm_len) < btp->bt_meta_sectorsize));
> -	ASSERT(!(BBTOB(map->bm_bn) & (xfs_off_t)btp->bt_meta_sectormask));
> +	ASSERT(!(BBTOB(map->bm_len) < mp->m_sb.sb_sectsize));

Urgh, should the polarity of this be reversed?

	ASSERT(BBTOB(map->bm_len) >= mp->m_sb.sb_sectsize);

Though I don't really care enough to block the patch so
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> +	ASSERT(!(BBTOB(map->bm_bn) & (xfs_off_t)(mp->m_sb.sb_sectsize - 1)));
>  
>  	/*
>  	 * Corrupted block numbers can get through to here, unfortunately, so we
>  	 * have to check that the buffer falls within the filesystem bounds.
>  	 */
> -	eofs = XFS_FSB_TO_BB(btp->bt_mount, btp->bt_mount->m_sb.sb_dblocks);
> +	eofs = XFS_FSB_TO_BB(mp, mp->m_sb.sb_dblocks);
>  	if (map->bm_bn < 0 || map->bm_bn >= eofs) {
>  		xfs_alert(btp->bt_mount,
>  			  "%s: daddr 0x%llx out of range, EOFS 0x%llx",
> @@ -1726,10 +1727,6 @@ xfs_configure_buftarg(
>  
>  	ASSERT(btp->bt_bdev != NULL);
>  
> -	/* Set up metadata sector size info */
> -	btp->bt_meta_sectorsize = sectorsize;
> -	btp->bt_meta_sectormask = sectorsize - 1;
> -
>  	error = bdev_validate_blocksize(btp->bt_bdev, sectorsize);
>  	if (error) {
>  		xfs_warn(btp->bt_mount,
> @@ -1816,14 +1813,7 @@ xfs_alloc_buftarg(
>  	if (error)
>  		goto error_free;
>  
> -	/*
> -	 * When allocating the buftargs we have not yet read the super block and
> -	 * thus don't know the file system sector size yet.
> -	 */
> -	btp->bt_meta_sectorsize = bdev_logical_block_size(btp->bt_bdev);
> -	btp->bt_meta_sectormask = btp->bt_meta_sectorsize - 1;
> -
> -	error = xfs_init_buftarg(btp, btp->bt_meta_sectorsize,
> +	error = xfs_init_buftarg(btp, bdev_logical_block_size(btp->bt_bdev),
>  				mp->m_super->s_id);
>  	if (error)
>  		goto error_free;
> diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
> index b269e115d9ac..8edfd9ed799e 100644
> --- a/fs/xfs/xfs_buf.h
> +++ b/fs/xfs/xfs_buf.h
> @@ -79,19 +79,6 @@ struct xfs_buf_cache {
>  int xfs_buf_cache_init(struct xfs_buf_cache *bch);
>  void xfs_buf_cache_destroy(struct xfs_buf_cache *bch);
>  
> -/*
> - * The xfs_buftarg contains 2 notions of "sector size" -
> - *
> - * 1) The metadata sector size, which is the minimum unit and
> - *    alignment of IO which will be performed by metadata operations.
> - * 2) The device logical sector size
> - *
> - * The first is specified at mkfs time, and is stored on-disk in the
> - * superblock's sb_sectsize.
> - *
> - * The latter is derived from the underlying device, and controls direct IO
> - * alignment constraints.
> - */
>  struct xfs_buftarg {
>  	dev_t			bt_dev;
>  	struct block_device	*bt_bdev;
> @@ -99,8 +86,6 @@ struct xfs_buftarg {
>  	struct file		*bt_file;
>  	u64			bt_dax_part_off;
>  	struct xfs_mount	*bt_mount;
> -	unsigned int		bt_meta_sectorsize;
> -	size_t			bt_meta_sectormask;
>  	size_t			bt_logical_sectorsize;
>  	size_t			bt_logical_sectormask;
>  
> diff --git a/fs/xfs/xfs_buf_mem.c b/fs/xfs/xfs_buf_mem.c
> index dcbfa274e06d..46f527750d34 100644
> --- a/fs/xfs/xfs_buf_mem.c
> +++ b/fs/xfs/xfs_buf_mem.c
> @@ -90,8 +90,6 @@ xmbuf_alloc(
>  	btp->bt_dev = (dev_t)-1U;
>  	btp->bt_bdev = NULL; /* in-memory buftargs have no bdev */
>  	btp->bt_file = file;
> -	btp->bt_meta_sectorsize = XMBUF_BLOCKSIZE;
> -	btp->bt_meta_sectormask = XMBUF_BLOCKSIZE - 1;
>  
>  	error = xfs_init_buftarg(btp, XMBUF_BLOCKSIZE, descr);
>  	if (error)
> -- 
> 2.47.2
> 
>