| Message ID | 158993950078.983175.14943057067035503330.stgit@magnolia (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xfs_repair: use btree bulk loading | expand |
On Tue, May 19, 2020 at 06:51:40PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > 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@oracle.com> > --- > 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; > >
On Thu, May 28, 2020 at 01:00:12PM -0400, Brian Foster wrote: > On Tue, May 19, 2020 at 06:51:40PM -0700, Darrick J. Wong wrote: > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > 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@oracle.com> > > --- > > 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? Ok. I suspect this ought to be converted to libfrog/bitmap from the slab cursors and whatnot that it uses now. --D > 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; > > > > >
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; +}; + + /* * 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;