Skip to content

Commit

Permalink
xfs: grab active perag ref when reading AG headers
Browse files Browse the repository at this point in the history
This patch prepares scrub to deal with the possibility of tearing down
entire AGs by changing the order of resource acquisition to match the
rest of the XFS codebase.  In other words, scrub now grabs AG resources
in order of: perag structure, then AGI/AGF/AGFL buffers, then btree
cursors; and releases them in reverse order.

This requires us to distinguish xchk_ag_init callers -- some are
responding to a user request to check AG metadata, in which case we can
return ENOENT to userspace; but other callers have an ondisk reference
to an AG that they're trying to cross-reference.  In this second case,
the lack of an AG means there's ondisk corruption, since ondisk metadata
cannot point into nonexistent space.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>
  • Loading branch information
Darrick J. Wong committed Aug 9, 2021
1 parent f19ee6b commit 48c6615
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 44 deletions.
23 changes: 16 additions & 7 deletions fs/xfs/scrub/agheader.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ xchk_superblock_xref(

agbno = XFS_SB_BLOCK(mp);

error = xchk_ag_init(sc, agno, &sc->sa);
error = xchk_ag_init_existing(sc, agno, &sc->sa);
if (!xchk_xref_process_error(sc, agno, agbno, &error))
return;

Expand All @@ -63,6 +63,7 @@ xchk_superblock(
struct xfs_mount *mp = sc->mp;
struct xfs_buf *bp;
struct xfs_dsb *sb;
struct xfs_perag *pag;
xfs_agnumber_t agno;
uint32_t v2_ok;
__be32 features_mask;
Expand All @@ -73,6 +74,15 @@ xchk_superblock(
if (agno == 0)
return 0;

/*
* Grab an active reference to the perag structure. If we can't get
* it, we're racing with something that's tearing down the AG, so
* signal that the AG no longer exists.
*/
pag = xfs_perag_get(mp, agno);
if (!pag)
return -ENOENT;

error = xfs_sb_read_secondary(mp, sc->tp, agno, &bp);
/*
* The superblock verifier can return several different error codes
Expand All @@ -92,7 +102,7 @@ xchk_superblock(
break;
}
if (!xchk_process_error(sc, agno, XFS_SB_BLOCK(mp), &error))
return error;
goto out_pag;

sb = bp->b_addr;

Expand Down Expand Up @@ -336,7 +346,8 @@ xchk_superblock(
xchk_block_set_corrupt(sc, bp);

xchk_superblock_xref(sc, bp);

out_pag:
xfs_perag_put(pag);
return error;
}

Expand Down Expand Up @@ -527,6 +538,7 @@ xchk_agf(
xchk_buffer_recheck(sc, sc->sa.agf_bp);

agf = sc->sa.agf_bp->b_addr;
pag = sc->sa.pag;

/* Check the AG length */
eoag = be32_to_cpu(agf->agf_length);
Expand Down Expand Up @@ -582,15 +594,13 @@ xchk_agf(
xchk_block_set_corrupt(sc, sc->sa.agf_bp);

/* Do the incore counters match? */
pag = xfs_perag_get(mp, agno);
if (pag->pagf_freeblks != be32_to_cpu(agf->agf_freeblks))
xchk_block_set_corrupt(sc, sc->sa.agf_bp);
if (pag->pagf_flcount != be32_to_cpu(agf->agf_flcount))
xchk_block_set_corrupt(sc, sc->sa.agf_bp);
if (xfs_sb_version_haslazysbcount(&sc->mp->m_sb) &&
pag->pagf_btreeblks != be32_to_cpu(agf->agf_btreeblks))
xchk_block_set_corrupt(sc, sc->sa.agf_bp);
xfs_perag_put(pag);

xchk_agf_xref(sc);
out:
Expand Down Expand Up @@ -857,6 +867,7 @@ xchk_agi(
xchk_buffer_recheck(sc, sc->sa.agi_bp);

agi = sc->sa.agi_bp->b_addr;
pag = sc->sa.pag;

/* Check the AG length */
eoag = be32_to_cpu(agi->agi_length);
Expand Down Expand Up @@ -909,12 +920,10 @@ xchk_agi(
xchk_block_set_corrupt(sc, sc->sa.agi_bp);

/* Do the incore counters match? */
pag = xfs_perag_get(mp, agno);
if (pag->pagi_count != be32_to_cpu(agi->agi_count))
xchk_block_set_corrupt(sc, sc->sa.agi_bp);
if (pag->pagi_freecount != be32_to_cpu(agi->agi_freecount))
xchk_block_set_corrupt(sc, sc->sa.agi_bp);
xfs_perag_put(pag);

xchk_agi_xref(sc);
out:
Expand Down
3 changes: 0 additions & 3 deletions fs/xfs/scrub/agheader_repair.c
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,6 @@ xrep_agf(
if (!xfs_sb_version_hasrmapbt(&mp->m_sb))
return -EOPNOTSUPP;

xchk_perag_get(sc->mp, &sc->sa);
/*
* Make sure we have the AGF buffer, as scrub might have decided it
* was corrupt after xfs_alloc_read_agf failed with -EFSCORRUPTED.
Expand Down Expand Up @@ -641,7 +640,6 @@ xrep_agfl(
if (!xfs_sb_version_hasrmapbt(&mp->m_sb))
return -EOPNOTSUPP;

xchk_perag_get(sc->mp, &sc->sa);
xbitmap_init(&agfl_extents);

/*
Expand Down Expand Up @@ -896,7 +894,6 @@ xrep_agi(
if (!xfs_sb_version_hasrmapbt(&mp->m_sb))
return -EOPNOTSUPP;

xchk_perag_get(sc->mp, &sc->sa);
/*
* Make sure we have the AGI buffer, as scrub might have decided it
* was corrupt after xfs_ialloc_read_agi failed with -EFSCORRUPTED.
Expand Down
2 changes: 1 addition & 1 deletion fs/xfs/scrub/bmap.c
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ xchk_bmap_iextent_xref(
agbno = XFS_FSB_TO_AGBNO(mp, irec->br_startblock);
len = irec->br_blockcount;

error = xchk_ag_init(info->sc, agno, &info->sc->sa);
error = xchk_ag_init_existing(info->sc, agno, &info->sc->sa);
if (!xchk_fblock_process_error(info->sc, info->whichfork,
irec->br_startoff, &error))
return;
Expand Down
2 changes: 1 addition & 1 deletion fs/xfs/scrub/btree.c
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ xchk_btree_check_block_owner(

init_sa = bs->cur->bc_flags & XFS_BTREE_LONG_PTRS;
if (init_sa) {
error = xchk_ag_init(bs->sc, agno, &bs->sc->sa);
error = xchk_ag_init_existing(bs->sc, agno, &bs->sc->sa);
if (!xchk_btree_xref_process_error(bs->sc, bs->cur,
level, &error))
return error;
Expand Down
48 changes: 19 additions & 29 deletions fs/xfs/scrub/common.c
Original file line number Diff line number Diff line change
Expand Up @@ -394,11 +394,11 @@ want_ag_read_header_failure(
}

/*
* Grab all the headers for an AG.
* Grab the perag structure and all the headers for an AG.
*
* The headers should be released by xchk_ag_free, but as a fail
* safe we attach all the buffers we grab to the scrub transaction so
* they'll all be freed when we cancel it.
* The headers should be released by xchk_ag_free, but as a fail safe we attach
* all the buffers we grab to the scrub transaction so they'll all be freed
* when we cancel it. Returns ENOENT if we can't grab the perag structure.
*/
int
xchk_ag_read_headers(
Expand All @@ -409,22 +409,26 @@ xchk_ag_read_headers(
struct xfs_mount *mp = sc->mp;
int error;

ASSERT(!sa->pag);
sa->pag = xfs_perag_get(mp, agno);
if (!sa->pag)
return -ENOENT;

sa->agno = agno;

error = xfs_ialloc_read_agi(mp, sc->tp, agno, &sa->agi_bp);
if (error && want_ag_read_header_failure(sc, XFS_SCRUB_TYPE_AGI))
goto out;
return error;

error = xfs_alloc_read_agf(mp, sc->tp, agno, 0, &sa->agf_bp);
if (error && want_ag_read_header_failure(sc, XFS_SCRUB_TYPE_AGF))
goto out;
return error;

error = xfs_alloc_read_agfl(mp, sc->tp, agno, &sa->agfl_bp);
if (error && want_ag_read_header_failure(sc, XFS_SCRUB_TYPE_AGFL))
goto out;
error = 0;
out:
return error;
return error;

return 0;
}

/* Release all the AG btree cursors. */
Expand Down Expand Up @@ -461,7 +465,6 @@ xchk_ag_btcur_init(
{
struct xfs_mount *mp = sc->mp;

xchk_perag_get(sc->mp, sa);
if (sa->agf_bp &&
xchk_ag_btree_healthy_enough(sc, sa->pag, XFS_BTNUM_BNO)) {
/* Set up a bnobt cursor for cross-referencing. */
Expand Down Expand Up @@ -532,11 +535,11 @@ xchk_ag_free(
}

/*
* For scrub, grab the AGI and the AGF headers, in that order. Locking
* order requires us to get the AGI before the AGF. We use the
* transaction to avoid deadlocking on crosslinked metadata buffers;
* either the caller passes one in (bmap scrub) or we have to create a
* transaction ourselves.
* For scrub, grab the perag structure, the AGI, and the AGF headers, in that
* order. Locking order requires us to get the AGI before the AGF. We use the
* transaction to avoid deadlocking on crosslinked metadata buffers; either the
* caller passes one in (bmap scrub) or we have to create a transaction
* ourselves. Returns ENOENT if the perag struct cannot be grabbed.
*/
int
xchk_ag_init(
Expand All @@ -554,19 +557,6 @@ xchk_ag_init(
return 0;
}

/*
* Grab the per-ag structure if we haven't already gotten it. Teardown of the
* xchk_ag will release it for us.
*/
void
xchk_perag_get(
struct xfs_mount *mp,
struct xchk_ag *sa)
{
if (!sa->pag)
sa->pag = xfs_perag_get(mp, sa->agno);
}

/* Per-scrubber setup functions */

/*
Expand Down
18 changes: 17 additions & 1 deletion fs/xfs/scrub/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,23 @@ int xchk_setup_fscounters(struct xfs_scrub *sc);
void xchk_ag_free(struct xfs_scrub *sc, struct xchk_ag *sa);
int xchk_ag_init(struct xfs_scrub *sc, xfs_agnumber_t agno,
struct xchk_ag *sa);
void xchk_perag_get(struct xfs_mount *mp, struct xchk_ag *sa);

/*
* Grab all AG resources, treating the inability to grab the perag structure as
* a fs corruption. This is intended for callers checking an ondisk reference
* to a given AG, which means that the AG must still exist.
*/
static inline int
xchk_ag_init_existing(
struct xfs_scrub *sc,
xfs_agnumber_t agno,
struct xchk_ag *sa)
{
int error = xchk_ag_init(sc, agno, sa);

return error == -ENOENT ? -EFSCORRUPTED : error;
}

int xchk_ag_read_headers(struct xfs_scrub *sc, xfs_agnumber_t agno,
struct xchk_ag *sa);
void xchk_ag_btcur_free(struct xchk_ag *sa);
Expand Down
2 changes: 1 addition & 1 deletion fs/xfs/scrub/fscounters.c
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ xchk_fscount_btreeblks(
xfs_extlen_t blocks;
int error;

error = xchk_ag_init(sc, agno, &sc->sa);
error = xchk_ag_init_existing(sc, agno, &sc->sa);
if (error)
return error;

Expand Down
2 changes: 1 addition & 1 deletion fs/xfs/scrub/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ xchk_inode_xref(
agno = XFS_INO_TO_AGNO(sc->mp, ino);
agbno = XFS_INO_TO_AGBNO(sc->mp, ino);

error = xchk_ag_init(sc, agno, &sc->sa);
error = xchk_ag_init_existing(sc, agno, &sc->sa);
if (!xchk_xref_process_error(sc, agno, agbno, &error))
return;

Expand Down

0 comments on commit 48c6615

Please sign in to comment.