From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:53471 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750998AbcGRNeg (ORCPT ); Mon, 18 Jul 2016 09:34:36 -0400 Date: Mon, 18 Jul 2016 09:34:34 -0400 From: Brian Foster To: "Darrick J. Wong" Cc: david@fromorbit.com, linux-fsdevel@vger.kernel.org, vishal.l.verma@intel.com, xfs@oss.sgi.com Subject: Re: [PATCH 048/119] xfs: don't update rmapbt when fixing agfl Message-ID: <20160718133434.GG27380@bfoster.bfoster> References: <146612627129.12839.3827886950949809165.stgit@birch.djwong.org> <146612657955.12839.6406507864344687918.stgit@birch.djwong.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <146612657955.12839.6406507864344687918.stgit@birch.djwong.org> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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 > --- > 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