Re: [PATCH V9 12/19] xfs: Introduce macros to represent new maximum extent counts for data/attr forks

[Dave Chinner <david@fromorbit.com>]

On Wed, Apr 06, 2022 at 11:48:56AM +0530, Chandan Babu R wrote:
> This commit defines new macros to represent maximum extent counts allowed by
> filesystems which have support for large per-inode extent counters.
> 
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_bmap.c       |  9 ++++-----
>  fs/xfs/libxfs/xfs_bmap_btree.c |  3 ++-
>  fs/xfs/libxfs/xfs_format.h     | 24 ++++++++++++++++++++++--
>  fs/xfs/libxfs/xfs_inode_buf.c  |  4 +++-
>  fs/xfs/libxfs/xfs_inode_fork.c |  3 ++-
>  fs/xfs/libxfs/xfs_inode_fork.h | 21 +++++++++++++++++----
>  6 files changed, 50 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index b317226fb4ba..1254d4d4821e 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -61,10 +61,8 @@ xfs_bmap_compute_maxlevels(
>  	int		sz;		/* root block size */
>  
>  	/*
> -	 * The maximum number of extents in a file, hence the maximum number of
> -	 * leaf entries, is controlled by the size of the on-disk extent count,
> -	 * either a signed 32-bit number for the data fork, or a signed 16-bit
> -	 * number for the attr fork.
> +	 * The maximum number of extents in a fork, hence the maximum number of
> +	 * leaf entries, is controlled by the size of the on-disk extent count.
>  	 *
>  	 * Note that we can no longer assume that if we are in ATTR1 that the
>  	 * fork offset of all the inodes will be
> @@ -74,7 +72,8 @@ xfs_bmap_compute_maxlevels(
>  	 * ATTR2 we have to assume the worst case scenario of a minimum size
>  	 * available.
>  	 */
> -	maxleafents = xfs_iext_max_nextents(whichfork);
> +	maxleafents = xfs_iext_max_nextents(xfs_has_large_extent_counts(mp),
> +				whichfork);
>  	if (whichfork == XFS_DATA_FORK)
>  		sz = XFS_BMDR_SPACE_CALC(MINDBTPTRS);
>  	else

Just to confirm, the large extent count feature bit can only be
added when the filesystem is unmounted?

> diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c
> index 453309fc85f2..7aabeccea9ab 100644
> --- a/fs/xfs/libxfs/xfs_bmap_btree.c
> +++ b/fs/xfs/libxfs/xfs_bmap_btree.c
> @@ -611,7 +611,8 @@ xfs_bmbt_maxlevels_ondisk(void)
>  	minrecs[1] = xfs_bmbt_block_maxrecs(blocklen, false) / 2;
>  
>  	/* One extra level for the inode root. */
> -	return xfs_btree_compute_maxlevels(minrecs, MAXEXTNUM) + 1;
> +	return xfs_btree_compute_maxlevels(minrecs,
> +			XFS_MAX_EXTCNT_DATA_FORK_LARGE) + 1;
>  }

Why is this set to XFS_MAX_EXTCNT_DATA_FORK_LARGE rather than being
conditional xfs_has_large_extent_counts(mp)? i.e. if the feature bit
is not set, the maximum on-disk levels in the bmbt is determined by
XFS_MAX_EXTCNT_DATA_FORK_SMALL, not XFS_MAX_EXTCNT_DATA_FORK_LARGE.

The "_ondisk" suffix implies that it has something to do with the
on-disk format of the filesystem, but AFAICT what we are calculating
here is a constant used for in-memory structure allocation? There
needs to be something explained/changed here, because this is
confusing...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com