On Sat, May 09, 2020 at 09:31:59AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> Create some new support structures and functions to assist phase5 in
> using the btree bulk loader to reconstruct metadata btrees. This is the
> first step in removing the open-coded rebuilding code.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
FYI, unused variable warnings:
phase5.c: In function ‘phase5_func’:
phase5.c:2491:20: warning: unused variable ‘sc’ [-Wunused-variable]
2491 | struct repair_ctx sc = { .mp = mp, };
| ^~
At top level:
phase5.c:509:1: warning: ‘finish_rebuild’ defined but not used [-Wunused-function]
509 | finish_rebuild(
| ^~~~~~~~~~~~~~
phase5.c:468:1: warning: ‘rebuild_alloc_block’ defined but not used [-Wunused-function]
468 | rebuild_alloc_block(
| ^~~~~~~~~~~~~~~~~~~
phase5.c:381:1: warning: ‘setup_rebuild’ defined but not used [-Wunused-function]
381 | setup_rebuild(
| ^~~~~~~~~~~~~
phase5.c:366:1: warning: ‘init_rebuild’ defined but not used [-Wunused-function]
366 | init_rebuild(
| ^~~~~~~~~~~~
> repair/phase5.c | 240 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 219 insertions(+), 21 deletions(-)
>
>
> diff --git a/repair/phase5.c b/repair/phase5.c
> index f3be15de..7eb24519 100644
> --- a/repair/phase5.c
> +++ b/repair/phase5.c
...
> @@ -306,6 +324,157 @@ _("error - not enough free space in filesystem\n"));
...
> +/* Reserve blocks for the new btree. */
> +static void
> +setup_rebuild(
> + struct xfs_mount *mp,
> + xfs_agnumber_t agno,
> + struct bt_rebuild *btr,
> + uint32_t nr_blocks)
> +{
> + struct extent_tree_node *ext_ptr;
> + struct extent_tree_node *bno_ext_ptr;
> + uint32_t blocks_allocated = 0;
> + int error;
> +
> + /*
> + * grab the smallest extent and use it up, then get the
> + * next smallest. This mimics the init_*_cursor code.
> + */
> + ext_ptr = findfirst_bcnt_extent(agno);
> +
> + /*
> + * set up the free block array
> + */
> + while (blocks_allocated < nr_blocks) {
> + uint64_t len;
> + xfs_agblock_t new_start;
> + xfs_extlen_t new_len;
> +
> + if (!ext_ptr)
> + do_error(
> +_("error - not enough free space in filesystem\n"));
> +
> + /* Use up the extent we've got. */
> + len = min(ext_ptr->ex_blockcount,
> + btr->bload.nr_blocks - blocks_allocated);
What's the difference between the nr_blocks parameter and this one?
> + error = xrep_newbt_add_reservation(&btr->newbt,
> + XFS_AGB_TO_FSB(mp, agno,
> + ext_ptr->ex_startblock),
> + len, NULL);
> + if (error)
> + do_error(_("could not set up btree reservation: %s\n"),
> + strerror(-error));
> + blocks_allocated += len;
> +
> + error = rmap_add_ag_rec(mp, agno, ext_ptr->ex_startblock, len,
> + btr->newbt.oinfo.oi_owner);
> + if (error)
> + do_error(_("could not set up btree rmaps: %s\n"),
> + strerror(-error));
> +
> + /* Figure out if we're putting anything back. */
The remaining extent replacement bits of this loop looks like it could
warrant a little helper and a comment to explain exactly what's
happening at a high level.
Brian
> + new_start = ext_ptr->ex_startblock + len;
> + new_len = ext_ptr->ex_blockcount - len;
> +
> + /* Delete the used-up extent from both extent trees. */
> +#ifdef XR_BLD_FREE_TRACE
> + fprintf(stderr, "releasing extent: %u [%u %u]\n",
> + agno, ext_ptr->ex_startblock, ext_ptr->ex_blockcount);
> +#endif
> + bno_ext_ptr = find_bno_extent(agno, ext_ptr->ex_startblock);
> + ASSERT(bno_ext_ptr != NULL);
> + get_bno_extent(agno, bno_ext_ptr);
> + release_extent_tree_node(bno_ext_ptr);
> +
> + ext_ptr = get_bcnt_extent(agno, ext_ptr->ex_startblock,
> + ext_ptr->ex_blockcount);
> + ASSERT(ext_ptr != NULL);
> + release_extent_tree_node(ext_ptr);
> +
> + /*
> + * If we only used part of this last extent, then we need only
> + * to reinsert the extent in the extent trees and we're done.
> + */
> + if (new_len > 0) {
> + add_bno_extent(agno, new_start, new_len);
> + add_bcnt_extent(agno, new_start, new_len);
> + break;
> + }
> +
> + /* Otherwise, find the next biggest extent. */
> + ext_ptr = findfirst_bcnt_extent(agno);
> + }
> +#ifdef XR_BLD_FREE_TRACE
> + fprintf(stderr, "blocks_allocated = %d\n",
> + blocks_allocated);
> +#endif
> +}
> +
> +/* Feed one of the new btree blocks to the bulk loader. */
> +static int
> +rebuild_alloc_block(
> + struct xfs_btree_cur *cur,
> + union xfs_btree_ptr *ptr,
> + void *priv)
> +{
> + struct bt_rebuild *btr = priv;
> +
> + return xrep_newbt_claim_block(cur, &btr->newbt, ptr);
> +}
> +
> static void
> write_cursor(bt_status_t *curs)
> {
> @@ -336,6 +505,34 @@ finish_cursor(bt_status_t *curs)
> free(curs->btree_blocks);
> }
>
> +static void
> +finish_rebuild(
> + struct xfs_mount *mp,
> + struct bt_rebuild *btr)
> +{
> + struct xrep_newbt_resv *resv, *n;
> +
> + for_each_xrep_newbt_reservation(&btr->newbt, resv, n) {
> + xfs_agnumber_t agno;
> + xfs_agblock_t bno;
> + xfs_extlen_t len;
> +
> + if (resv->used >= resv->len)
> + continue;
> +
> + /* XXX: Shouldn't this go on the AGFL? */
> + /* Put back everything we didn't use. */
> + bno = XFS_FSB_TO_AGBNO(mp, resv->fsbno + resv->used);
> + agno = XFS_FSB_TO_AGNO(mp, resv->fsbno + resv->used);
> + len = resv->len - resv->used;
> +
> + add_bno_extent(agno, bno, len);
> + add_bcnt_extent(agno, bno, len);
> + }
> +
> + xrep_newbt_destroy(&btr->newbt, 0);
> +}
> +
> /*
> * We need to leave some free records in the tree for the corner case of
> * setting up the AGFL. This may require allocation of blocks, and as
> @@ -2290,28 +2487,29 @@ keep_fsinos(xfs_mount_t *mp)
>
> static void
> phase5_func(
> - xfs_mount_t *mp,
> - xfs_agnumber_t agno,
> - struct xfs_slab *lost_fsb)
> + struct xfs_mount *mp,
> + xfs_agnumber_t agno,
> + struct xfs_slab *lost_fsb)
> {
> - uint64_t num_inos;
> - uint64_t num_free_inos;
> - uint64_t finobt_num_inos;
> - uint64_t finobt_num_free_inos;
> - bt_status_t bno_btree_curs;
> - bt_status_t bcnt_btree_curs;
> - bt_status_t ino_btree_curs;
> - bt_status_t fino_btree_curs;
> - bt_status_t rmap_btree_curs;
> - bt_status_t refcnt_btree_curs;
> - int extra_blocks = 0;
> - uint num_freeblocks;
> - xfs_extlen_t freeblks1;
> + struct repair_ctx sc = { .mp = mp, };
> + struct agi_stat agi_stat = {0,};
> + uint64_t num_inos;
> + uint64_t num_free_inos;
> + uint64_t finobt_num_inos;
> + uint64_t finobt_num_free_inos;
> + bt_status_t bno_btree_curs;
> + bt_status_t bcnt_btree_curs;
> + bt_status_t ino_btree_curs;
> + bt_status_t fino_btree_curs;
> + bt_status_t rmap_btree_curs;
> + bt_status_t refcnt_btree_curs;
> + int extra_blocks = 0;
> + uint num_freeblocks;
> + xfs_extlen_t freeblks1;
> #ifdef DEBUG
> - xfs_extlen_t freeblks2;
> + xfs_extlen_t freeblks2;
> #endif
> - xfs_agblock_t num_extents;
> - struct agi_stat agi_stat = {0,};
> + xfs_agblock_t num_extents;
>
> if (verbose)
> do_log(_(" - agno = %d\n"), agno);
> @@ -2533,8 +2731,8 @@ inject_lost_blocks(
> if (error)
> goto out_cancel;
>
> - error = -libxfs_free_extent(tp, *fsb, 1, &XFS_RMAP_OINFO_AG,
> - XFS_AG_RESV_NONE);
> + error = -libxfs_free_extent(tp, *fsb, 1,
> + &XFS_RMAP_OINFO_ANY_OWNER, XFS_AG_RESV_NONE);
> if (error)
> goto out_cancel;
>
>
