On Mon, Jul 18, 2016 at 09:34:34AM -0400, Brian Foster wrote:
> 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?
Yeah, I think that is the case. In the end it'll look like:
memset(targs, 0...);
if (flags & NORMAP)
xfs_rmap_skip_update(&targs.oinfo);
else
xfs_rmap_ag_owner(&targs.oinfo...);
while (!(flags & NOSHRINK) && flcount > need) {
...
}
--D
>
> 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
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
