Re: [PATCH 1/6] xfs: document the XFS_ALLOC_AGFL_RESERVE constant

[Brian Foster <bfoster@redhat.com>]

On Thu, Mar 17, 2022 at 02:21:01PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Currently, we use this undocumented macro to encode the minimum number
> of blocks needed to replenish a completely empty AGFL when an AG is
> nearly full.  This has lead to confusion on the part of the maintainers,
> so let's document what the value actually means, and move it to
> xfs_alloc.c since it's not used outside of that module.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_alloc.c |   23 ++++++++++++++++++-----
>  fs/xfs/libxfs/xfs_alloc.h |    1 -
>  2 files changed, 18 insertions(+), 6 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 353e53b892e6..b0678e96ce61 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -82,6 +82,19 @@ xfs_prealloc_blocks(
>  }
>  
>  /*
> + * The number of blocks per AG that we withhold from xfs_mod_fdblocks to
> + * guarantee that we can refill the AGFL prior to allocating space in a nearly
> + * full AG.  We require two blocks per free space btree because free space
> + * btrees shrink to a single block as the AG fills up, and any allocation can
> + * cause a btree split.  The rmap btree uses a per-AG reservation to withhold
> + * space from xfs_mod_fdblocks, so we do not account for that here.
> + */
> +#define XFS_ALLOCBT_AGFL_RESERVE	4
> +
> +/*
> + * Compute the number of blocks that we set aside to guarantee the ability to
> + * refill the AGFL and handle a full bmap btree split.
> + *
>   * In order to avoid ENOSPC-related deadlock caused by out-of-order locking of
>   * AGF buffer (PV 947395), we place constraints on the relationship among
>   * actual allocations for data blocks, freelist blocks, and potential file data
> @@ -93,14 +106,14 @@ xfs_prealloc_blocks(
>   * extents need to be actually allocated. To get around this, we explicitly set
>   * aside a few blocks which will not be reserved in delayed allocation.
>   *
> - * We need to reserve 4 fsbs _per AG_ for the freelist and 4 more to handle a
> - * potential split of the file's bmap btree.
> + * For each AG, we need to reserve enough blocks to replenish a totally empty
> + * AGFL and 4 more to handle a potential split of the file's bmap btree.
>   */
>  unsigned int
>  xfs_alloc_set_aside(
>  	struct xfs_mount	*mp)
>  {
> -	return mp->m_sb.sb_agcount * (XFS_ALLOC_AGFL_RESERVE + 4);
> +	return mp->m_sb.sb_agcount * (XFS_ALLOCBT_AGFL_RESERVE + 4);
>  }
>  
>  /*
> @@ -124,12 +137,12 @@ xfs_alloc_ag_max_usable(
>  	unsigned int		blocks;
>  
>  	blocks = XFS_BB_TO_FSB(mp, XFS_FSS_TO_BB(mp, 4)); /* ag headers */
> -	blocks += XFS_ALLOC_AGFL_RESERVE;
> +	blocks += XFS_ALLOCBT_AGFL_RESERVE;
>  	blocks += 3;			/* AGF, AGI btree root blocks */
>  	if (xfs_has_finobt(mp))
>  		blocks++;		/* finobt root block */
>  	if (xfs_has_rmapbt(mp))
> -		blocks++; 		/* rmap root block */
> +		blocks++;		/* rmap root block */
>  	if (xfs_has_reflink(mp))
>  		blocks++;		/* refcount root block */
>  
> diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h
> index 1c14a0b1abea..d4c057b764f9 100644
> --- a/fs/xfs/libxfs/xfs_alloc.h
> +++ b/fs/xfs/libxfs/xfs_alloc.h
> @@ -88,7 +88,6 @@ typedef struct xfs_alloc_arg {
>  #define XFS_ALLOC_NOBUSY		(1 << 2)/* Busy extents not allowed */
>  
>  /* freespace limit calculations */
> -#define XFS_ALLOC_AGFL_RESERVE	4
>  unsigned int xfs_alloc_set_aside(struct xfs_mount *mp);
>  unsigned int xfs_alloc_ag_max_usable(struct xfs_mount *mp);
>  
>