xfs
[Top] [All Lists]

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

To: "Darrick J. Wong" <darrick.wong@xxxxxxxxxx>
Subject: Re: [PATCH 048/119] xfs: don't update rmapbt when fixing agfl
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Mon, 18 Jul 2016 09:34:34 -0400
Cc: david@xxxxxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx, vishal.l.verma@xxxxxxxxx, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <146612657955.12839.6406507864344687918.stgit@xxxxxxxxxxxxxxxx>
References: <146612627129.12839.3827886950949809165.stgit@xxxxxxxxxxxxxxxx> <146612657955.12839.6406507864344687918.stgit@xxxxxxxxxxxxxxxx>
User-agent: Mutt/1.6.1 (2016-04-27)
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@xxxxxxxxxx>
> ---
>  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@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

<Prev in Thread] Current Thread [Next in Thread>