From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from aserp1040.oracle.com ([141.146.126.69]:26906 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751320AbcGRPyI (ORCPT ); Mon, 18 Jul 2016 11:54:08 -0400 Date: Mon, 18 Jul 2016 08:53:49 -0700 From: "Darrick J. Wong" To: Brian Foster 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: <20160718155349.GA2494@birch.djwong.org> References: <146612627129.12839.3827886950949809165.stgit@birch.djwong.org> <146612657955.12839.6406507864344687918.stgit@birch.djwong.org> <20160718133434.GG27380@bfoster.bfoster> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160718133434.GG27380@bfoster.bfoster> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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 > > --- > > 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@oss.sgi.com > > http://oss.sgi.com/mailman/listinfo/xfs