xfs: only invalidate blocks if we're going to free them

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>
Reviewed-by: Dave Chinner <dchinner@redhat.com>

diff --git a/fs/xfs/scrub/reap.c b/fs/xfs/scrub/reap.c
index 774dd8a..b332b0e 100644 (file)
--- 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 601caa7..e01d63a 100644 (file)
--- 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 */