On Wed, Oct 09, 2019 at 09:48:47AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> When we're discarding old btree blocks after a repair, only invalidate
> the buffers for the ones that we're freeing -- if the metadata was
> crosslinked with another data structure, we don't want to touch it.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
> fs/xfs/scrub/repair.c | 76 +++++++++++++++++++++++--------------------------
> fs/xfs/scrub/repair.h | 1 -
> 2 files changed, 35 insertions(+), 42 deletions(-)
>
>
> diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
> index 3a58788e0bd8..e21faef6db5a 100644
> --- a/fs/xfs/scrub/repair.c
> +++ b/fs/xfs/scrub/repair.c
...
> @@ -515,6 +477,35 @@ xrep_put_freelist(
> return 0;
> }
>
> +/* Try to invalidate the incore buffer for a block that we're about to free. */
> +STATIC void
> +xrep_reap_invalidate_block(
> + struct xfs_scrub *sc,
> + xfs_fsblock_t fsbno)
> +{
> + struct xfs_buf *bp;
> +
> + /*
> + * For each block in each extent, see if there's an incore buffer for
> + * exactly that block; if so, invalidate it. The buffer cache only
> + * lets us look for one buffer at a time, so we have to look one block
> + * at a time. Avoid invalidating AG headers and post-EOFS blocks
> + * because we never own those; and if we can't TRYLOCK the buffer we
> + * assume it's owned by someone else.
> + */
> + /* Skip AG headers and post-EOFS blocks */
The comment doesn't seem to quite go with the implementation any longer.
Also, there's probably no need to have two separate comments here.
Otherwise looks Ok.
Brian
> + if (!xfs_verify_fsbno(sc->mp, fsbno))
> + return;
> + bp = xfs_buf_incore(sc->mp->m_ddev_targp,
> + XFS_FSB_TO_DADDR(sc->mp, fsbno),
> + XFS_FSB_TO_BB(sc->mp, 1), XBF_TRYLOCK);
> + if (!bp)
> + return;
> +
> + xfs_trans_bjoin(sc->tp, bp);
> + xfs_trans_binval(sc->tp, bp);
> +}
> +
> /* Dispose of a single block. */
> STATIC int
> xrep_reap_block(
> @@ -568,12 +559,15 @@ xrep_reap_block(
> * blow on writeout, the filesystem will shut down, and the admin gets
> * to run xfs_repair.
> */
> - if (has_other_rmap)
> + if (has_other_rmap) {
> error = xfs_rmap_free(sc->tp, agf_bp, agno, agbno, 1, oinfo);
> - else if (resv == XFS_AG_RESV_AGFL)
> + } else if (resv == XFS_AG_RESV_AGFL) {
> + xrep_reap_invalidate_block(sc, fsbno);
> error = xrep_put_freelist(sc, agbno);
> - else
> + } else {
> + xrep_reap_invalidate_block(sc, fsbno);
> error = xfs_free_extent(sc->tp, fsbno, 1, oinfo, resv);
> + }
> if (agf_bp != sc->sa.agf_bp)
> xfs_trans_brelse(sc->tp, agf_bp);
> if (error)
> diff --git a/fs/xfs/scrub/repair.h b/fs/xfs/scrub/repair.h
> index 60c61d7052a8..eab41928990f 100644
> --- a/fs/xfs/scrub/repair.h
> +++ b/fs/xfs/scrub/repair.h
> @@ -31,7 +31,6 @@ int xrep_init_btblock(struct xfs_scrub *sc, xfs_fsblock_t fsb,
> struct xfs_bitmap;
>
> int xrep_fix_freelist(struct xfs_scrub *sc, bool can_shrink);
> -int xrep_invalidate_blocks(struct xfs_scrub *sc, struct xfs_bitmap *btlist);
> int xrep_reap_extents(struct xfs_scrub *sc, struct xfs_bitmap *exlist,
> const struct xfs_owner_info *oinfo, enum xfs_ag_resv_type type);
>
>
