On Sat, Aug 11, 2018 at 08:35:09AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> In xrep_findroot_block, if we find a candidate root block with sibling
> pointers or sibling blocks on the same tree level, we should not return
> that block as a tree root because root blocks cannot have siblings.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
> fs/xfs/scrub/repair.c | 47 +++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 41 insertions(+), 6 deletions(-)
Looks fine
Reviewed-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
>
>
> diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
> index 17cf48564390..42d8c798ce7d 100644
> --- a/fs/xfs/scrub/repair.c
> +++ b/fs/xfs/scrub/repair.c
> @@ -690,6 +690,7 @@ xrep_findroot_block(
> struct xfs_buf *bp;
> struct xfs_btree_block *btblock;
> xfs_daddr_t daddr;
> + int block_level;
> int error;
>
> daddr = XFS_AGB_TO_DADDR(mp, ri->sc->sa.agno, agbno);
> @@ -727,17 +728,51 @@ xrep_findroot_block(
> goto out;
> bp->b_ops = fab->buf_ops;
>
> - /* Ignore this block if it's lower in the tree than we've seen. */
> - if (fab->root != NULLAGBLOCK &&
> - xfs_btree_get_level(btblock) < fab->height)
> - goto out;
> -
> /* Make sure we pass the verifiers. */
> bp->b_ops->verify_read(bp);
> if (bp->b_error)
> goto out;
> +
> + /* If we've recorded a root candidate... */
> + block_level = xfs_btree_get_level(btblock);
> + if (fab->root != NULLAGBLOCK) {
> + /*
> + * ...and this no-sibling root block candidate has the same
> + * level as the recorded candidate, there's no way we're going
> + * to accept any candidates at this tree level. Stash a root
> + * block of zero because the height is still valid, but no
> + * AG btree can root at agblock 0. Callers should verify the
> + * root agbno with xfs_verify_agbno...
> + */
> + if (block_level + 1 == fab->height) {
> + fab->root = 0;
> + goto out;
> + }
> +
> + /*
> + * ...and this no-sibling root block is lower in the tree than
> + * the recorded root block candidate, just ignore it. There's
> + * still a strong chance that something is wrong with the btree
> + * itself, but that's not what we're fixing right now.
> + */
> + if (block_level < fab->height)
> + goto out;
> + }
> +
> + /*
> + * Root blocks can't have siblings. This level can't be the root, so
> + * record the tree height (but not the ag block pointer) to force us to
> + * look for a higher level in the tree.
> + */
> + if (btblock->bb_u.s.bb_leftsib != cpu_to_be32(NULLAGBLOCK) ||
> + btblock->bb_u.s.bb_rightsib != cpu_to_be32(NULLAGBLOCK)) {
> + fab->root = 0;
> + fab->height = block_level + 1;
> + goto out;
> + }
> +
> fab->root = agbno;
> - fab->height = xfs_btree_get_level(btblock) + 1;
> + fab->height = block_level + 1;
> *found_it = true;
>
> trace_xrep_findroot_block(mp, ri->sc->sa.agno, agbno,
>
--
Carlos
