Re: [PATCH 048/119] xfs: don't update rmapbt when fixing agfl

[Brian Foster <bfoster@redhat.com>]

On Thu, Jun 16, 2016 at 06:22:59PM -0700, Darrick J. Wong wrote:
> Allow a caller of xfs_alloc_fix_freelist to disable rmapbt updates
> when fixing the AG freelist.  xfs_repair needs this during phase 5
> to be able to adjust the freelist while it's reconstructing the rmap
> btree; the missing entries will be added back at the very end of
> phase 5 once the AGFL contents settle down.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_alloc.c |   40 ++++++++++++++++++++++++++--------------
>  fs/xfs/libxfs/xfs_alloc.h |    3 +++
>  2 files changed, 29 insertions(+), 14 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 4c8ffd4..6eabab1 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -2092,26 +2092,38 @@ xfs_alloc_fix_freelist(
>  	 * anything other than extra overhead when we need to put more blocks
>  	 * back on the free list? Maybe we should only do this when space is
>  	 * getting low or the AGFL is more than half full?
> +	 *
> +	 * The NOSHRINK flag prevents the AGFL from being shrunk if it's too
> +	 * big; the NORMAP flag prevents AGFL expand/shrink operations from
> +	 * updating the rmapbt.  Both flags are used in xfs_repair while we're
> +	 * rebuilding the rmapbt, and neither are used by the kernel.  They're
> +	 * both required to ensure that rmaps are correctly recorded for the
> +	 * regenerated AGFL, bnobt, and cntbt.  See repair/phase5.c and
> +	 * repair/rmap.c in xfsprogs for details.
>  	 */
> -	xfs_rmap_ag_owner(&targs.oinfo, XFS_RMAP_OWN_AG);
> -	while (pag->pagf_flcount > need) {
> -		struct xfs_buf	*bp;
> +	memset(&targs, 0, sizeof(targs));
> +	if (!(flags & XFS_ALLOC_FLAG_NOSHRINK)) {
> +		if (!(flags & XFS_ALLOC_FLAG_NORMAP))
> +			xfs_rmap_ag_owner(&targs.oinfo, XFS_RMAP_OWN_AG);

Can we get away with setting targs.oinfo once rather than here and
below? If so, I think something like the following might clean this up a
bit and save some indentation:

	memset(targs, 0, ...);
	if (!(flags & NORMAP))
		xfs_rmap_ag_owner(...);
	while (!(flags & NOSHRINK) &&
	       flcount > need) {
		...
	}
	...

Hm?

Brian

> +		while (pag->pagf_flcount > need) {
> +			struct xfs_buf	*bp;
>  
> -		error = xfs_alloc_get_freelist(tp, agbp, &bno, 0);
> -		if (error)
> -			goto out_agbp_relse;
> -		error = xfs_free_ag_extent(tp, agbp, args->agno, bno, 1,
> -					   &targs.oinfo, 1);
> -		if (error)
> -			goto out_agbp_relse;
> -		bp = xfs_btree_get_bufs(mp, tp, args->agno, bno, 0);
> -		xfs_trans_binval(tp, bp);
> +			error = xfs_alloc_get_freelist(tp, agbp, &bno, 0);
> +			if (error)
> +				goto out_agbp_relse;
> +			error = xfs_free_ag_extent(tp, agbp, args->agno, bno, 1,
> +						   &targs.oinfo, 1);
> +			if (error)
> +				goto out_agbp_relse;
> +			bp = xfs_btree_get_bufs(mp, tp, args->agno, bno, 0);
> +			xfs_trans_binval(tp, bp);
> +		}
>  	}
>  
> -	memset(&targs, 0, sizeof(targs));
>  	targs.tp = tp;
>  	targs.mp = mp;
> -	xfs_rmap_ag_owner(&targs.oinfo, XFS_RMAP_OWN_AG);
> +	if (!(flags & XFS_ALLOC_FLAG_NORMAP))
> +		xfs_rmap_ag_owner(&targs.oinfo, XFS_RMAP_OWN_AG);
>  	targs.agbp = agbp;
>  	targs.agno = args->agno;
>  	targs.alignment = targs.minlen = targs.prod = targs.isfl = 1;
> diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h
> index 7b6c66b..7b9e67e 100644
> --- a/fs/xfs/libxfs/xfs_alloc.h
> +++ b/fs/xfs/libxfs/xfs_alloc.h
> @@ -54,6 +54,9 @@ typedef unsigned int xfs_alloctype_t;
>   */
>  #define	XFS_ALLOC_FLAG_TRYLOCK	0x00000001  /* use trylock for buffer locking */
>  #define	XFS_ALLOC_FLAG_FREEING	0x00000002  /* indicate caller is freeing extents*/
> +#define	XFS_ALLOC_FLAG_NORMAP	0x00000004  /* don't modify the rmapbt */
> +#define	XFS_ALLOC_FLAG_NOSHRINK	0x00000008  /* don't shrink the freelist */
> +
>  
>  /*
>   * Argument structure for xfs_alloc routines.
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs