* [PATCH 1/3] xfs: xrep_reap_extents should not destroy the bitmap
2020-01-01 1:01 [PATCH v2 0/3] xfs: fix online repair block reaping Darrick J. Wong
@ 2020-01-01 1:01 ` Darrick J. Wong
2020-01-01 1:01 ` [PATCH 2/3] xfs: only invalidate blocks if we're going to free them Darrick J. Wong
2020-01-01 1:01 ` [PATCH 3/3] xfs: use deferred frees to reap old btree blocks Darrick J. Wong
2 siblings, 0 replies; 5+ messages in thread
From: Darrick J. Wong @ 2020-01-01 1:01 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, Brian Foster
From: Darrick J. Wong <darrick.wong@oracle.com>
Remove the xfs_bitmap_destroy call from the end of xrep_reap_extents
because this sort of violates our rule that the function initializing a
structure should destroy it.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
---
| 2 +-
fs/xfs/scrub/repair.c | 4 +---
2 files changed, 2 insertions(+), 4 deletions(-)
--git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
index 7a1a38b636a9..8fcd43040c96 100644
--- a/fs/xfs/scrub/agheader_repair.c
+++ b/fs/xfs/scrub/agheader_repair.c
@@ -698,7 +698,7 @@ xrep_agfl(
goto err;
/* Dump any AGFL overflow. */
- return xrep_reap_extents(sc, &agfl_extents, &XFS_RMAP_OINFO_AG,
+ error = xrep_reap_extents(sc, &agfl_extents, &XFS_RMAP_OINFO_AG,
XFS_AG_RESV_AGFL);
err:
xfs_bitmap_destroy(&agfl_extents);
diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
index b70a88bc975e..3a58788e0bd8 100644
--- a/fs/xfs/scrub/repair.c
+++ b/fs/xfs/scrub/repair.c
@@ -613,11 +613,9 @@ xrep_reap_extents(
error = xrep_reap_block(sc, fsbno, oinfo, type);
if (error)
- goto out;
+ break;
}
-out:
- xfs_bitmap_destroy(bitmap);
return error;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] xfs: only invalidate blocks if we're going to free them
2020-01-01 1:01 [PATCH v2 0/3] xfs: fix online repair block reaping Darrick J. Wong
2020-01-01 1:01 ` [PATCH 1/3] xfs: xrep_reap_extents should not destroy the bitmap Darrick J. Wong
@ 2020-01-01 1:01 ` Darrick J. Wong
2020-01-01 1:01 ` [PATCH 3/3] xfs: use deferred frees to reap old btree blocks Darrick J. Wong
2 siblings, 0 replies; 5+ messages in thread
From: Darrick J. Wong @ 2020-01-01 1:01 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
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@oracle.com>
---
fs/xfs/scrub/repair.c | 74 ++++++++++++++++++++++---------------------------
fs/xfs/scrub/repair.h | 1 -
2 files changed, 33 insertions(+), 42 deletions(-)
diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
index 3a58788e0bd8..62a9b77eba3d 100644
--- a/fs/xfs/scrub/repair.c
+++ b/fs/xfs/scrub/repair.c
@@ -423,44 +423,6 @@ xrep_init_btblock(
* buffers associated with @bitmap.
*/
-/*
- * 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 xfs_bitmap *bitmap)
-{
- struct xfs_bitmap_range *bmr;
- struct xfs_bitmap_range *n;
- struct xfs_buf *bp;
- xfs_fsblock_t fsbno;
-
- /*
- * 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.
- */
- for_each_xfs_bitmap_block(fsbno, bmr, n, bitmap) {
- /* Skip AG headers and post-EOFS blocks */
- if (!xfs_verify_fsbno(sc->mp, fsbno))
- continue;
- 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) {
- xfs_trans_bjoin(sc->tp, bp);
- xfs_trans_binval(sc->tp, bp);
- }
- }
-
- return 0;
-}
-
/* Ensure the freelist is the correct size. */
int
xrep_fix_freelist(
@@ -515,6 +477,33 @@ 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;
+
+ /*
+ * 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; and if we can't TRYLOCK the buffer we assume it's owned
+ * by someone else.
+ */
+ 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 +557,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);
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 3/3] xfs: use deferred frees to reap old btree blocks
2020-01-01 1:01 [PATCH v2 0/3] xfs: fix online repair block reaping Darrick J. Wong
2020-01-01 1:01 ` [PATCH 1/3] xfs: xrep_reap_extents should not destroy the bitmap Darrick J. Wong
2020-01-01 1:01 ` [PATCH 2/3] xfs: only invalidate blocks if we're going to free them Darrick J. Wong
@ 2020-01-01 1:01 ` Darrick J. Wong
2 siblings, 0 replies; 5+ messages in thread
From: Darrick J. Wong @ 2020-01-01 1:01 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Use deferred frees (EFIs) to reap the blocks of a btree that we just
replaced. This helps us to shrink the window in which those old blocks
could be lost due to a system crash, though we try to flush the EFIs
every few hundred blocks so that we don't also overflow the transaction
reservations during and after we commit the new btree.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/scrub/repair.c | 29 ++++++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
index 62a9b77eba3d..477df76174b9 100644
--- a/fs/xfs/scrub/repair.c
+++ b/fs/xfs/scrub/repair.c
@@ -24,6 +24,7 @@
#include "xfs_extent_busy.h"
#include "xfs_ag_resv.h"
#include "xfs_quota.h"
+#include "xfs_bmap.h"
#include "scrub/scrub.h"
#include "scrub/common.h"
#include "scrub/trace.h"
@@ -510,13 +511,15 @@ xrep_reap_block(
struct xfs_scrub *sc,
xfs_fsblock_t fsbno,
const struct xfs_owner_info *oinfo,
- enum xfs_ag_resv_type resv)
+ enum xfs_ag_resv_type resv,
+ unsigned int *deferred)
{
struct xfs_btree_cur *cur;
struct xfs_buf *agf_bp = NULL;
xfs_agnumber_t agno;
xfs_agblock_t agbno;
bool has_other_rmap;
+ bool need_roll = true;
int error;
agno = XFS_FSB_TO_AGNO(sc->mp, fsbno);
@@ -563,14 +566,24 @@ xrep_reap_block(
xrep_reap_invalidate_block(sc, fsbno);
error = xrep_put_freelist(sc, agbno);
} else {
+ /*
+ * Use deferred frees to get rid of the old btree blocks to try
+ * to minimize the window in which we could crash and lose the
+ * old blocks. However, we still need to roll the transaction
+ * every 100 or so EFIs so that we don't exceed the log
+ * reservation.
+ */
xrep_reap_invalidate_block(sc, fsbno);
- error = xfs_free_extent(sc->tp, fsbno, 1, oinfo, resv);
+ __xfs_bmap_add_free(sc->tp, fsbno, 1, oinfo, false);
+ (*deferred)++;
+ need_roll = *deferred > 100;
}
if (agf_bp != sc->sa.agf_bp)
xfs_trans_brelse(sc->tp, agf_bp);
- if (error)
+ if (error || !need_roll)
return error;
+ *deferred = 0;
if (sc->ip)
return xfs_trans_roll_inode(&sc->tp, sc->ip);
return xrep_roll_ag_trans(sc);
@@ -592,6 +605,7 @@ xrep_reap_extents(
struct xfs_bitmap_range *bmr;
struct xfs_bitmap_range *n;
xfs_fsblock_t fsbno;
+ unsigned int deferred = 0;
int error = 0;
ASSERT(xfs_sb_version_hasrmapbt(&sc->mp->m_sb));
@@ -603,12 +617,17 @@ xrep_reap_extents(
XFS_FSB_TO_AGNO(sc->mp, fsbno),
XFS_FSB_TO_AGBNO(sc->mp, fsbno), 1);
- error = xrep_reap_block(sc, fsbno, oinfo, type);
+ error = xrep_reap_block(sc, fsbno, oinfo, type, &deferred);
if (error)
break;
}
- return error;
+ if (error || deferred == 0)
+ return error;
+
+ if (sc->ip)
+ return xfs_trans_roll_inode(&sc->tp, sc->ip);
+ return xrep_roll_ag_trans(sc);
}
/*
^ permalink raw reply related [flat|nested] 5+ messages in thread