On Tue, May 19, 2020 at 06:51:40PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> Use extent records (not just raw fsbs) to track blocks that were lost
> during btree construction. This makes it somewhat more efficient.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
> repair/phase5.c | 61 ++++++++++++++++++++++++++++++++-----------------------
> 1 file changed, 35 insertions(+), 26 deletions(-)
>
>
> diff --git a/repair/phase5.c b/repair/phase5.c
> index 72c6908a..22007275 100644
> --- a/repair/phase5.c
> +++ b/repair/phase5.c
> @@ -45,6 +45,12 @@ struct bt_rebuild {
> };
> };
>
> +struct lost_fsb {
> + xfs_fsblock_t fsbno;
> + xfs_extlen_t len;
> +};
> +
Looks reasonable at a glance, but could we call the above structure an
extent and rename lost_fsbs to lost_extents or some such?
Brian
> +
> /*
> * extra metadata for the agi
> */
> @@ -314,21 +320,24 @@ static void
> finish_rebuild(
> struct xfs_mount *mp,
> struct bt_rebuild *btr,
> - struct xfs_slab *lost_fsb)
> + struct xfs_slab *lost_fsbs)
> {
> struct xrep_newbt_resv *resv, *n;
>
> for_each_xrep_newbt_reservation(&btr->newbt, resv, n) {
> - while (resv->used < resv->len) {
> - xfs_fsblock_t fsb = resv->fsbno + resv->used;
> - int error;
> + struct lost_fsb lost;
> + int error;
>
> - error = slab_add(lost_fsb, &fsb);
> - if (error)
> - do_error(
> + if (resv->used == resv->len)
> + continue;
> +
> + lost.fsbno = resv->fsbno + resv->used;
> + lost.len = resv->len - resv->used;
> + error = slab_add(lost_fsbs, &lost);
> + if (error)
> + do_error(
> _("Insufficient memory saving lost blocks.\n"));
> - resv->used++;
> - }
> + resv->used = resv->len;
> }
>
> xrep_newbt_destroy(&btr->newbt, 0);
> @@ -1036,7 +1045,7 @@ build_agf_agfl(
> int lostblocks, /* # blocks that will be lost */
> struct bt_rebuild *btr_rmap,
> struct bt_rebuild *btr_refc,
> - struct xfs_slab *lost_fsb)
> + struct xfs_slab *lost_fsbs)
> {
> struct extent_tree_node *ext_ptr;
> struct xfs_buf *agf_buf, *agfl_buf;
> @@ -1253,7 +1262,7 @@ static void
> phase5_func(
> struct xfs_mount *mp,
> xfs_agnumber_t agno,
> - struct xfs_slab *lost_fsb)
> + struct xfs_slab *lost_fsbs)
> {
> struct repair_ctx sc = { .mp = mp, };
> struct agi_stat agi_stat = {0,};
> @@ -1372,7 +1381,7 @@ _("unable to rebuild AG %u. Not enough free space in on-disk AG.\n"),
> * set up agf and agfl
> */
> build_agf_agfl(mp, agno, &btr_bno, &btr_cnt, freeblks1, extra_blocks,
> - &btr_rmap, &btr_refc, lost_fsb);
> + &btr_rmap, &btr_refc, lost_fsbs);
>
> /*
> * build inode allocation trees.
> @@ -1387,15 +1396,15 @@ _("unable to rebuild AG %u. Not enough free space in on-disk AG.\n"),
> /*
> * tear down cursors
> */
> - finish_rebuild(mp, &btr_bno, lost_fsb);
> - finish_rebuild(mp, &btr_cnt, lost_fsb);
> - finish_rebuild(mp, &btr_ino, lost_fsb);
> + finish_rebuild(mp, &btr_bno, lost_fsbs);
> + finish_rebuild(mp, &btr_cnt, lost_fsbs);
> + finish_rebuild(mp, &btr_ino, lost_fsbs);
> if (xfs_sb_version_hasfinobt(&mp->m_sb))
> - finish_rebuild(mp, &btr_fino, lost_fsb);
> + finish_rebuild(mp, &btr_fino, lost_fsbs);
> if (xfs_sb_version_hasrmapbt(&mp->m_sb))
> - finish_rebuild(mp, &btr_rmap, lost_fsb);
> + finish_rebuild(mp, &btr_rmap, lost_fsbs);
> if (xfs_sb_version_hasreflink(&mp->m_sb))
> - finish_rebuild(mp, &btr_refc, lost_fsb);
> + finish_rebuild(mp, &btr_refc, lost_fsbs);
>
> /*
> * release the incore per-AG bno/bcnt trees so the extent nodes
> @@ -1414,19 +1423,19 @@ inject_lost_blocks(
> {
> struct xfs_trans *tp = NULL;
> struct xfs_slab_cursor *cur = NULL;
> - xfs_fsblock_t *fsb;
> + struct lost_fsb *lost;
> int error;
>
> error = init_slab_cursor(lost_fsbs, NULL, &cur);
> if (error)
> return error;
>
> - while ((fsb = pop_slab_cursor(cur)) != NULL) {
> + while ((lost = pop_slab_cursor(cur)) != NULL) {
> error = -libxfs_trans_alloc_rollable(mp, 16, &tp);
> if (error)
> goto out_cancel;
>
> - error = -libxfs_free_extent(tp, *fsb, 1,
> + error = -libxfs_free_extent(tp, lost->fsbno, lost->len,
> &XFS_RMAP_OINFO_ANY_OWNER, XFS_AG_RESV_NONE);
> if (error)
> goto out_cancel;
> @@ -1447,7 +1456,7 @@ inject_lost_blocks(
> void
> phase5(xfs_mount_t *mp)
> {
> - struct xfs_slab *lost_fsb;
> + struct xfs_slab *lost_fsbs;
> xfs_agnumber_t agno;
> int error;
>
> @@ -1490,12 +1499,12 @@ phase5(xfs_mount_t *mp)
> if (sb_fdblocks_ag == NULL)
> do_error(_("cannot alloc sb_fdblocks_ag buffers\n"));
>
> - error = init_slab(&lost_fsb, sizeof(xfs_fsblock_t));
> + error = init_slab(&lost_fsbs, sizeof(struct lost_fsb));
> if (error)
> do_error(_("cannot alloc lost block slab\n"));
>
> for (agno = 0; agno < mp->m_sb.sb_agcount; agno++)
> - phase5_func(mp, agno, lost_fsb);
> + phase5_func(mp, agno, lost_fsbs);
>
> print_final_rpt();
>
> @@ -1538,10 +1547,10 @@ _("unable to add AG %u reverse-mapping data to btree.\n"), agno);
> * Put blocks that were unnecessarily reserved for btree
> * reconstruction back into the filesystem free space data.
> */
> - error = inject_lost_blocks(mp, lost_fsb);
> + error = inject_lost_blocks(mp, lost_fsbs);
> if (error)
> do_error(_("Unable to reinsert lost blocks into filesystem.\n"));
> - free_slab(&lost_fsb);
> + free_slab(&lost_fsbs);
>
> bad_ino_btree = 0;
>
>
