From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Brian Foster <bfoster@redhat.com>
Cc: sandeen@sandeen.net, linux-xfs@vger.kernel.org
Subject: Re: [PATCH 3/9] xfs_repair: create a new class of btree rebuild cursors
Date: Thu, 14 May 2020 12:47:01 -0700 [thread overview]
Message-ID: <20200514194701.GG6714@magnolia> (raw)
In-Reply-To: <20200514151107.GC50849@bfoster>
On Thu, May 14, 2020 at 11:11:07AM -0400, Brian Foster wrote:
> On Sat, May 09, 2020 at 09:31:59AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > 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@oracle.com>
> > ---
>
> 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(
> | ^~~~~~~~~~~~
Yeah... these aren't used by anything until the next patch, and I was
trying to keep the "infrastructure you need for X" changes separate from
"X". Though every time I do that, people complain about me adding code
that isn't used anywhere...
> > 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?
I think that's a bug, and should have been:
len = min(ext_ptr->ex_blockcount, nr_blocks - blocks_allocated);
> > + 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.
Ok, I'll figure something out.
--D
> 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;
> >
> >
>
next prev parent reply other threads:[~2020-05-14 19:49 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-09 16:31 [PATCH v4 0/9] xfs_repair: use btree bulk loading Darrick J. Wong
2020-05-09 16:31 ` [PATCH 1/9] xfs_repair: port the online repair newbt structure Darrick J. Wong
2020-05-14 15:09 ` Brian Foster
2020-05-14 19:20 ` Darrick J. Wong
2020-05-15 11:41 ` Brian Foster
2020-05-15 18:52 ` Darrick J. Wong
2020-05-15 19:43 ` Brian Foster
2020-05-09 16:31 ` [PATCH 2/9] xfs_repair: unindent phase 5 function Darrick J. Wong
2020-05-14 15:09 ` Brian Foster
2020-05-14 19:23 ` Darrick J. Wong
2020-05-09 16:31 ` [PATCH 3/9] xfs_repair: create a new class of btree rebuild cursors Darrick J. Wong
2020-05-14 15:11 ` Brian Foster
2020-05-14 19:47 ` Darrick J. Wong [this message]
2020-05-09 16:32 ` [PATCH 4/9] xfs_repair: rebuild free space btrees with bulk loader Darrick J. Wong
2020-05-14 15:12 ` Brian Foster
2020-05-14 19:53 ` Darrick J. Wong
2020-05-15 11:42 ` Brian Foster
2020-05-09 16:32 ` [PATCH 5/9] xfs_repair: rebuild inode " Darrick J. Wong
2020-05-09 16:32 ` [PATCH 6/9] xfs_repair: rebuild reverse mapping " Darrick J. Wong
2020-05-09 16:32 ` [PATCH 7/9] xfs_repair: rebuild refcount " Darrick J. Wong
2020-05-09 16:32 ` [PATCH 8/9] xfs_repair: remove old btree rebuild support code Darrick J. Wong
2020-05-09 16:32 ` [PATCH 9/9] xfs_repair: track blocks lost during btree construction via extents Darrick J. Wong
-- strict thread matches above, loose matches on Subject: below --
2020-05-20 1:50 [PATCH v5 0/9] xfs_repair: use btree bulk loading Darrick J. Wong
2020-05-20 1:51 ` [PATCH 3/9] xfs_repair: create a new class of btree rebuild cursors Darrick J. Wong
2020-05-27 12:18 ` Brian Foster
2020-05-27 22:07 ` Darrick J. Wong
2020-05-28 15:09 ` Brian Foster
2020-05-29 21:08 ` Darrick J. Wong
2020-03-04 3:29 [PATCH v3 0/9] xfs_repair: use btree bulk loading Darrick J. Wong
2020-03-04 3:29 ` [PATCH 3/9] xfs_repair: create a new class of btree rebuild cursors Darrick J. Wong
2020-01-01 1:21 [PATCH v2 0/9] xfs_repair: use btree bulk loading Darrick J. Wong
2020-01-01 1:21 ` [PATCH 3/9] xfs_repair: create a new class of btree rebuild cursors Darrick J. Wong
2019-10-29 23:45 [PATCH RFC 0/9] xfs_repair: use btree bulk loading Darrick J. Wong
2019-10-29 23:45 ` [PATCH 3/9] xfs_repair: create a new class of btree rebuild cursors Darrick J. Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200514194701.GG6714@magnolia \
--to=darrick.wong@oracle.com \
--cc=bfoster@redhat.com \
--cc=linux-xfs@vger.kernel.org \
--cc=sandeen@sandeen.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).