On Sun, Mar 20, 2022 at 09:43:43AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@xxxxxxxxxx>
>
> xfs_reserve_blocks controls the size of the user-visible free space
> reserve pool. Given the difference between the current and requested
> pool sizes, it will try to reserve free space from fdblocks. However,
> the amount requested from fdblocks is also constrained by the amount of
> space that we think xfs_mod_fdblocks will give us. We'll keep trying to
> reserve space so long as xfs_mod_fdblocks returns ENOSPC.
>
> In commit fd43cf600cf6, we decided that xfs_mod_fdblocks should not hand
> out the "free space" used by the free space btrees, because some portion
> of the free space btrees hold in reserve space for future btree
> expansion. Unfortunately, xfs_reserve_blocks' estimation of the number
> of blocks that it could request from xfs_mod_fdblocks was not updated to
> include m_allocbt_blks, so if space is extremely low, the caller hangs.
>
> Fix this by creating a function to estimate the number of blocks that
> can be reserved from fdblocks, which needs to exclude the set-aside and
> m_allocbt_blks.
>
> Found by running xfs/306 (which formats a single-AG 20MB filesystem)
> with an fstests configuration that specifies a 1k blocksize and a
> specially crafted log size that will consume 7/8 of the space (17920
> blocks, specifically) in that AG.
>
> Cc: Brian Foster <bfoster@xxxxxxxxxx>
> Fixes: fd43cf600cf6 ("xfs: set aside allocation btree blocks from block reservation")
> Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> ---
> fs/xfs/xfs_fsops.c | 2 +-
> fs/xfs/xfs_mount.c | 2 +-
> fs/xfs/xfs_mount.h | 15 +++++++++++++++
> 3 files changed, 17 insertions(+), 2 deletions(-)
>
>
...
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index 00720a02e761..da1b7056e743 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -479,6 +479,21 @@ extern void xfs_unmountfs(xfs_mount_t *);
> */
> #define XFS_FDBLOCKS_BATCH 1024
>
> +/*
> + * Estimate the amount of free space that is not available to userspace and is
> + * not explicitly reserved from the incore fdblocks:
> + *
> + * - Space reserved to ensure that we can always split a bmap btree
> + * - Free space btree blocks that are not available for allocation due to
> + * per-AG metadata reservations
> + */
What does this mean by "due to" perag res? That sounds like a separate
thing to me. Perhaps this could just say something like:
"Estimate the amount of accounted free space that is not available to
userspace. This includes the minimum number of blocks to support a bmbt
split (calculated at mount time) and the blocks currently in-use by the
allocation btrees."
Comment nit aside, this LGTM. Thanks for the rework..
Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
> +static inline uint64_t
> +xfs_fdblocks_unavailable(
> + struct xfs_mount *mp)
> +{
> + return mp->m_alloc_set_aside + atomic64_read(&mp->m_allocbt_blks);
> +}
> +
> extern int xfs_mod_fdblocks(struct xfs_mount *mp, int64_t delta,
> bool reserved);
> extern int xfs_mod_frextents(struct xfs_mount *mp, int64_t delta);
>
