| Message ID | 151398984612.18741.15845883360126485561.stgit@magnolia (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Fri, Dec 22, 2017 at 04:44:06PM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > When scanning a metadata btree block, cross-reference the block location > with the free space btree and the reverse mapping btree to ensure that > the rmapbt knows about the block and the bnobt does not. Add a > mechanism to defer checks when we happen to be scanning the bnobt/rmapbt > itself because it's less efficient to repeatedly clone and destroy the > cursor. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > fs/xfs/scrub/btree.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 92 insertions(+) > > > diff --git a/fs/xfs/scrub/btree.c b/fs/xfs/scrub/btree.c > index 36cff8f..9151499 100644 > --- a/fs/xfs/scrub/btree.c > +++ b/fs/xfs/scrub/btree.c > @@ -362,6 +362,80 @@ xfs_scrub_btree_block_check_siblings( > return error; > } > > +struct check_owner { > + struct list_head list; > + xfs_daddr_t daddr; > + int level; > +}; > + > +/* > + * Make sure this btree block isn't in the free list and that there's > + * an rmap record for it. > + */ > +STATIC int > +xfs_scrub_btree_check_block_owner( > + struct xfs_scrub_btree *bs, > + int level, > + xfs_daddr_t daddr) > +{ > + struct xfs_scrub_ag sa = { 0 }; > + struct xfs_scrub_ag *psa; > + xfs_agnumber_t agno; > + int error = 0; > + > + agno = xfs_daddr_to_agno(bs->cur->bc_mp, daddr); > + > + if (bs->cur->bc_flags & XFS_BTREE_LONG_PTRS) { > + error = xfs_scrub_ag_init(bs->sc, agno, &sa); > + if (!xfs_scrub_btree_xref_process_error(bs->sc, bs->cur, > + level, &error)) > + return error; > + psa = &sa; > + } else { > + psa = &bs->sc->sa; > + } > + > + if (psa == &sa) > + xfs_scrub_ag_free(bs->sc, &sa); > + > + return error; > +} I'm missing something here - where's the owner check? Cheers, Dave.
On Fri, Jan 05, 2018 at 01:24:36PM +1100, Dave Chinner wrote: > On Fri, Dec 22, 2017 at 04:44:06PM -0800, Darrick J. Wong wrote: > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > When scanning a metadata btree block, cross-reference the block location > > with the free space btree and the reverse mapping btree to ensure that > > the rmapbt knows about the block and the bnobt does not. Add a > > mechanism to defer checks when we happen to be scanning the bnobt/rmapbt > > itself because it's less efficient to repeatedly clone and destroy the > > cursor. > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > --- > > fs/xfs/scrub/btree.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 92 insertions(+) > > > > > > diff --git a/fs/xfs/scrub/btree.c b/fs/xfs/scrub/btree.c > > index 36cff8f..9151499 100644 > > --- a/fs/xfs/scrub/btree.c > > +++ b/fs/xfs/scrub/btree.c > > @@ -362,6 +362,80 @@ xfs_scrub_btree_block_check_siblings( > > return error; > > } > > > > +struct check_owner { > > + struct list_head list; > > + xfs_daddr_t daddr; > > + int level; > > +}; > > + > > +/* > > + * Make sure this btree block isn't in the free list and that there's > > + * an rmap record for it. > > + */ > > +STATIC int > > +xfs_scrub_btree_check_block_owner( > > + struct xfs_scrub_btree *bs, > > + int level, > > + xfs_daddr_t daddr) > > +{ > > + struct xfs_scrub_ag sa = { 0 }; > > + struct xfs_scrub_ag *psa; > > + xfs_agnumber_t agno; > > + int error = 0; > > + > > + agno = xfs_daddr_to_agno(bs->cur->bc_mp, daddr); > > + > > + if (bs->cur->bc_flags & XFS_BTREE_LONG_PTRS) { > > + error = xfs_scrub_ag_init(bs->sc, agno, &sa); > > + if (!xfs_scrub_btree_xref_process_error(bs->sc, bs->cur, > > + level, &error)) > > + return error; > > + psa = &sa; > > + } else { > > + psa = &bs->sc->sa; > > + } > > + > > + if (psa == &sa) > > + xfs_scrub_ag_free(bs->sc, &sa); > > + > > + return error; > > +} > > I'm missing something here - where's the owner check? "xfs: cross-reference with the bnobt" and "xfs: cross-reference with the rmapbt" add the actual meat of checking the owner. Both of those patches create helpers to deal with checking the owner and recording error statuses and uses them, so this patch only provides the shell of the btree block checking apparatus. Will update the commit message to say that this is only the framework. --D > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 04, 2018 at 06:53:20PM -0800, Darrick J. Wong wrote: > On Fri, Jan 05, 2018 at 01:24:36PM +1100, Dave Chinner wrote: > > On Fri, Dec 22, 2017 at 04:44:06PM -0800, Darrick J. Wong wrote: > > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > > > When scanning a metadata btree block, cross-reference the block location > > > with the free space btree and the reverse mapping btree to ensure that > > > the rmapbt knows about the block and the bnobt does not. Add a > > > mechanism to defer checks when we happen to be scanning the bnobt/rmapbt > > > itself because it's less efficient to repeatedly clone and destroy the > > > cursor. > > > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > > --- > > > fs/xfs/scrub/btree.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 92 insertions(+) > > > > > > > > > diff --git a/fs/xfs/scrub/btree.c b/fs/xfs/scrub/btree.c > > > index 36cff8f..9151499 100644 > > > --- a/fs/xfs/scrub/btree.c > > > +++ b/fs/xfs/scrub/btree.c > > > @@ -362,6 +362,80 @@ xfs_scrub_btree_block_check_siblings( > > > return error; > > > } > > > > > > +struct check_owner { > > > + struct list_head list; > > > + xfs_daddr_t daddr; > > > + int level; > > > +}; > > > + > > > +/* > > > + * Make sure this btree block isn't in the free list and that there's > > > + * an rmap record for it. > > > + */ > > > +STATIC int > > > +xfs_scrub_btree_check_block_owner( > > > + struct xfs_scrub_btree *bs, > > > + int level, > > > + xfs_daddr_t daddr) > > > +{ > > > + struct xfs_scrub_ag sa = { 0 }; > > > + struct xfs_scrub_ag *psa; > > > + xfs_agnumber_t agno; > > > + int error = 0; > > > + > > > + agno = xfs_daddr_to_agno(bs->cur->bc_mp, daddr); > > > + > > > + if (bs->cur->bc_flags & XFS_BTREE_LONG_PTRS) { > > > + error = xfs_scrub_ag_init(bs->sc, agno, &sa); > > > + if (!xfs_scrub_btree_xref_process_error(bs->sc, bs->cur, > > > + level, &error)) > > > + return error; > > > + psa = &sa; > > > + } else { > > > + psa = &bs->sc->sa; > > > + } > > > + > > > + if (psa == &sa) > > > + xfs_scrub_ag_free(bs->sc, &sa); > > > + > > > + return error; > > > +} > > > > I'm missing something here - where's the owner check? > > "xfs: cross-reference with the bnobt" and "xfs: cross-reference with the > rmapbt" add the actual meat of checking the owner. Both of those > patches create helpers to deal with checking the owner and recording > error statuses and uses them, so this patch only provides the shell of > the btree block checking apparatus. > > Will update the commit message to say that this is only the framework. OK, with that you can add Reviewed-by: Dave Chinner <dchinner@redhat.com>
diff --git a/fs/xfs/scrub/btree.c b/fs/xfs/scrub/btree.c index 36cff8f..9151499 100644 --- a/fs/xfs/scrub/btree.c +++ b/fs/xfs/scrub/btree.c @@ -362,6 +362,80 @@ xfs_scrub_btree_block_check_siblings( return error; } +struct check_owner { + struct list_head list; + xfs_daddr_t daddr; + int level; +}; + +/* + * Make sure this btree block isn't in the free list and that there's + * an rmap record for it. + */ +STATIC int +xfs_scrub_btree_check_block_owner( + struct xfs_scrub_btree *bs, + int level, + xfs_daddr_t daddr) +{ + struct xfs_scrub_ag sa = { 0 }; + struct xfs_scrub_ag *psa; + xfs_agnumber_t agno; + int error = 0; + + agno = xfs_daddr_to_agno(bs->cur->bc_mp, daddr); + + if (bs->cur->bc_flags & XFS_BTREE_LONG_PTRS) { + error = xfs_scrub_ag_init(bs->sc, agno, &sa); + if (!xfs_scrub_btree_xref_process_error(bs->sc, bs->cur, + level, &error)) + return error; + psa = &sa; + } else { + psa = &bs->sc->sa; + } + + if (psa == &sa) + xfs_scrub_ag_free(bs->sc, &sa); + + return error; +} + +/* Check the owner of a btree block. */ +STATIC int +xfs_scrub_btree_check_owner( + struct xfs_scrub_btree *bs, + int level, + struct xfs_buf *bp) +{ + struct xfs_btree_cur *cur = bs->cur; + struct check_owner *co; + + if ((cur->bc_flags & XFS_BTREE_ROOT_IN_INODE) && bp == NULL) + return 0; + + /* + * We want to cross-reference each btree block with the bnobt + * and the rmapbt. We cannot cross-reference the bnobt or + * rmapbt while scanning the bnobt or rmapbt, respectively, + * because we cannot alter the cursor and we'd prefer not to + * duplicate cursors. Therefore, save the buffer daddr for + * later scanning. + */ + if (cur->bc_btnum == XFS_BTNUM_BNO || cur->bc_btnum == XFS_BTNUM_RMAP) { + co = kmem_alloc(sizeof(struct check_owner), + KM_MAYFAIL | KM_NOFS); + if (!co) + return -ENOMEM; + co->level = level; + co->daddr = XFS_BUF_ADDR(bp); + list_add_tail(&co->list, &bs->to_check); + return 0; + } + + return xfs_scrub_btree_check_block_owner(bs, level, XFS_BUF_ADDR(bp)); +} + /* * Grab and scrub a btree block given a btree pointer. Returns block * and buffer pointers (if applicable) if they're ok to use. @@ -398,6 +472,14 @@ xfs_scrub_btree_get_block( } /* + * Check the block's owner; this function absorbs error codes + * for us. + */ + error = xfs_scrub_btree_check_owner(bs, level, *pbp); + if (error) + return error; + + /* * Check the block's siblings; this function absorbs error codes * for us. */ @@ -468,6 +550,8 @@ xfs_scrub_btree( struct xfs_btree_block *block; int level; struct xfs_buf *bp; + struct check_owner *co; + struct check_owner *n; int i; int error = 0; @@ -559,6 +643,14 @@ xfs_scrub_btree( } out: + /* Process deferred owner checks on btree blocks. */ + list_for_each_entry_safe(co, n, &bs.to_check, list) { + if (!error) + error = xfs_scrub_btree_check_block_owner(&bs, + co->level, co->daddr); + list_del(&co->list); + kmem_free(co); + } return error; }