[PATCH 3/9] xfs: only invalidate blocks if we're going to free them

Darrick J. Wong
2023-07-27T22:22:19Z

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 <djwong@kernel.org>
---
fs/xfs/scrub/reap.c | 96 +++++++++++++++++++++----------------------------
fs/xfs/scrub/repair.h | 1 -
2 files changed, 42 insertions(+), 55 deletions(-)

diff --git a/fs/xfs/scrub/reap.c b/fs/xfs/scrub/reap.c
index 774dd8a12b2a6..b332b0e8e2594 100644
--- a/fs/xfs/scrub/reap.c
+++ b/fs/xfs/scrub/reap.c
@@ -70,54 +70,10 @@
*
* The caller is responsible for locking the AG headers for the entire rebuild
* operation so that nothing else can sneak in and change the AG state while
- * we're not looking. We also assume that the caller already invalidated any
- * buffers associated with @bitmap.
+ * we're not looking. We must also invalidate any buffers associated with
+ * @bitmap.
*/

-static int
-xrep_invalidate_block(
- uint64_t fsbno,
- void *priv)
-{
- struct xfs_scrub *sc = priv;
- struct xfs_buf *bp;
- int error;
-
- /* Skip AG headers and post-EOFS blocks */
- if (!xfs_verify_fsbno(sc->mp, fsbno))
- return 0;
-
- error = xfs_buf_incore(sc->mp->m_ddev_targp,
- XFS_FSB_TO_DADDR(sc->mp, fsbno),
- XFS_FSB_TO_BB(sc->mp, 1), XBF_TRYLOCK, &bp);
- if (error)
- return 0;
-
- xfs_trans_bjoin(sc->tp, bp);
- xfs_trans_binval(sc->tp, bp);
- return 0;
-}
-
-/*
- * Invalidate buffers for per-AG btree blocks we're dumping. This function
- * is not intended for use with file data repairs; we have bunmapi for that.
- */
-int
-xrep_invalidate_blocks(
- struct xfs_scrub *sc,
- struct xbitmap *bitmap)
-{
- /*
- * 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.
- */
- return xbitmap_walk_bits(bitmap, xrep_invalidate_block, sc);
-}
-
/* Information about reaping extents after a repair. */
struct xrep_reap_state {
struct xfs_scrub *sc;
@@ -127,9 +83,7 @@ struct xrep_reap_state {
enum xfs_ag_resv_type resv;
};

-/*
- * Put a block back on the AGFL.
- */
+/* Put a block back on the AGFL. */
STATIC int
xrep_put_freelist(
struct xfs_scrub *sc,
@@ -168,6 +122,37 @@ xrep_put_freelist(
return 0;
}

+/* Try to invalidate the incore buffer for a block that we're about to free. */
+STATIC void
+xrep_block_reap_binval(
+ struct xfs_scrub *sc,
+ xfs_fsblock_t fsbno)
+{
+ struct xfs_buf *bp = NULL;
+ int error;
+
+ /*
+ * If there's an incore buffer for exactly this block, invalidate it.
+ * Avoid invalidating AG headers and post-EOFS blocks because we never
+ * own those.
+ */
+ if (!xfs_verify_fsbno(sc->mp, fsbno))
+ return;
+
+ /*
+ * We assume that the lack of any other known owners means that the
+ * buffer can be locked without risk of deadlocking.
+ */
+ error = xfs_buf_incore(sc->mp->m_ddev_targp,
+ XFS_FSB_TO_DADDR(sc->mp, fsbno),
+ XFS_FSB_TO_BB(sc->mp, 1), 0, &bp);
+ if (error)
+ return;
+
+ xfs_trans_bjoin(sc->tp, bp);
+ xfs_trans_binval(sc->tp, bp);
+}
+
/* Dispose of a single block. */
STATIC int
xrep_reap_block(
@@ -225,14 +210,17 @@ xrep_reap_block(
* blow on writeout, the filesystem will shut down, and the admin gets
* to run xfs_repair.
*/
- if (has_other_rmap)
- error = xfs_rmap_free(sc->tp, agf_bp, sc->sa.pag, agbno,
- 1, rs->oinfo);
- else if (rs->resv == XFS_AG_RESV_AGFL)
+ if (has_other_rmap) {
+ error = xfs_rmap_free(sc->tp, agf_bp, sc->sa.pag, agbno, 1,
+ rs->oinfo);
+ } else if (rs->resv == XFS_AG_RESV_AGFL) {
+ xrep_block_reap_binval(sc, fsbno);
error = xrep_put_freelist(sc, agbno);
- else
+ } else {
+ xrep_block_reap_binval(sc, fsbno);
error = xfs_free_extent(sc->tp, sc->sa.pag, agbno, 1, rs->oinfo,
rs->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 601caa70f8703..e01d63a4a93b4 100644
--- a/fs/xfs/scrub/repair.h
+++ b/fs/xfs/scrub/repair.h
@@ -28,7 +28,6 @@ struct xbitmap;
struct xagb_bitmap;

int xrep_fix_freelist(struct xfs_scrub *sc, bool can_shrink);
-int xrep_invalidate_blocks(struct xfs_scrub *sc, struct xbitmap *btlist);

struct xrep_find_ag_btree {
/* in: rmap owner of the btree we're looking for */