Re: [RFC PATCH 2/4] xfs: check ag is empty

[Dave Chinner <david@fromorbit.com>]

On Thu, Apr 15, 2021 at 03:52:38AM +0800, Gao Xiang wrote:
> After a perag is stableized as inactive, we could check if such ag
> is empty. In order to achieve that, AGFL is also needed to be
> emptified in advance to make sure that only one freespace extent
> will exist here.
> 
> Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
> ---
>  fs/xfs/libxfs/xfs_alloc.c | 97 +++++++++++++++++++++++++++++++++++++++
>  fs/xfs/libxfs/xfs_alloc.h |  4 ++
>  2 files changed, 101 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 01d4e4d4c1d6..60a8c134c00e 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -2474,6 +2474,103 @@ xfs_defer_agfl_block(
>  	xfs_defer_add(tp, XFS_DEFER_OPS_TYPE_AGFL_FREE, &new->xefi_list);
>  }
>  
> +int
> +xfs_ag_emptify_agfl(
> +	struct xfs_buf		*agfbp)
> +{
> +	struct xfs_mount	*mp = agfbp->b_mount;
> +	struct xfs_perag	*pag = agfbp->b_pag;
> +	struct xfs_trans	*tp;
> +	int			error;
> +	struct xfs_owner_info	oinfo = XFS_RMAP_OINFO_AG;
> +
> +	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_growdata, 0, 0,
> +				XFS_TRANS_RESERVE, &tp);
> +	if (error)
> +		return error;
> +
> +	/* attach to the transaction and keep it from unlocked */
> +	xfs_trans_bjoin(tp, agfbp);
> +	xfs_trans_bhold(tp, agfbp);
> +
> +	while (pag->pagf_flcount) {
> +		xfs_agblock_t	bno;
> +		int		error;
> +
> +		error = xfs_alloc_get_freelist(tp, agfbp, &bno, 0);
> +		if (error)
> +			break;
> +
> +		ASSERT(bno != NULLAGBLOCK);
> +		xfs_defer_agfl_block(tp, pag->pag_agno, bno, &oinfo);
> +	}
> +	xfs_trans_set_sync(tp);
> +	xfs_trans_commit(tp);
> +	return error;
> +}

Why do we need to empty the agfl to determine the AG is empty?

> +int
> +xfs_ag_is_empty(
> +	struct xfs_buf		*agfbp)
> +{
> +	struct xfs_mount	*mp = agfbp->b_mount;
> +	struct xfs_perag	*pag = agfbp->b_pag;
> +	struct xfs_agf		*agf = agfbp->b_addr;
> +	struct xfs_btree_cur	*cnt_cur;
> +	xfs_agblock_t		nfbno;
> +	xfs_extlen_t		nflen;
> +	int			error, i;
> +
> +	if (!pag->pag_inactive)
> +		return -EINVAL;
> +
> +	if (pag->pagf_freeblks + pag->pagf_flcount !=
> +	    be32_to_cpu(agf->agf_length) - mp->m_ag_prealloc_blocks)
> +		return -ENOTEMPTY;

This is the empty check right here, yes?

Hmmm - this has to fail if the log is in this AG, right? Because we
can't move the log (yet), so we should explicitly be checking that
the log is in this AG before check the amount of free space...

> +
> +	if (pag->pagf_flcount) {
> +		error = xfs_ag_emptify_agfl(agfbp);
> +		if (error)
> +			return error;
> +
> +		if (pag->pagf_freeblks !=
> +		    be32_to_cpu(agf->agf_length) - mp->m_ag_prealloc_blocks)
> +			return -ENOTEMPTY;
> +	}
> +
> +	if (pag->pagi_count > 0 || pag->pagi_freecount > 0)
> +		return -ENOTEMPTY;
> +
> +	if (be32_to_cpu(agf->agf_levels[XFS_BTNUM_BNO]) > 1 ||
> +	    be32_to_cpu(agf->agf_levels[XFS_BTNUM_CNT]) > 1)
> +		return -ENOTEMPTY;
> +
> +	cnt_cur = xfs_allocbt_init_cursor(mp, NULL, agfbp,
> +					  pag->pag_agno, XFS_BTNUM_CNT);
> +	ASSERT(cnt_cur->bc_nlevels == 1);
> +	error = xfs_alloc_lookup_ge(cnt_cur, 0,
> +				    be32_to_cpu(agf->agf_longest), &i);
> +	if (error || !i)
> +		goto out;
> +
> +	error = xfs_alloc_get_rec(cnt_cur, &nfbno, &nflen, &i);
> +	if (error)
> +		goto out;
> +
> +	if (XFS_IS_CORRUPT(mp, i != 1)) {
> +		error = -EFSCORRUPTED;
> +		goto out;
> +	}
> +
> +	error = -ENOTEMPTY;
> +	if (nfbno == mp->m_ag_prealloc_blocks &&
> +	    nflen == pag->pagf_freeblks)
> +		error = 0;

Ah, that's why you are trying to empty the AGFL.

This won't work because the AG btree roots can be anywhere in the AG
once the tree has grown beyond a single block. Hence when the AG is
fully empty, the btree root blocks can still break up free space
into multiple extents that are each less than a maximally sized
single extent. e.g. from my workstation:

$ xfs_db -r -c "agf 0" -c "p" /dev/mapper/base-main 
magicnum = 0x58414746
versionnum = 1
seqno = 0
length = 13732864
bnoroot = 29039
cntroot = 922363
rmaproot = 8461704
refcntroot = 6
bnolevel = 2
cntlevel = 2
rmaplevel = 3
....

none of the root blocks are inside the m_ag_prealloc_blocks region
of the AG. m_ag_prealloc_blocks is just for space accounting and
does not imply physical location of the btree root blocks...

Hence I think the only checks that need to be done here are whether
the number of free blocks equals the maximal number of blocks
available in the given AG.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com