From: Brian Foster <bfoster@redhat.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: sandeen@sandeen.net, Eric Sandeen <sandeen@redhat.com>,
linux-xfs@vger.kernel.org
Subject: Re: [PATCH 2/9] xfs_repair: remove gratuitous code block in phase5
Date: Wed, 27 May 2020 08:15:41 -0400 [thread overview]
Message-ID: <20200527121541.GB12014@bfoster> (raw)
In-Reply-To: <158993945558.983175.3730752781062943521.stgit@magnolia>
On Tue, May 19, 2020 at 06:50:55PM -0700, Darrick J. Wong wrote:
> From: Eric Sandeen <sandeen@redhat.com>
>
> A commit back in 2008 removed a "for" loop ahead of this code block, but
> left the indented code block in place. Remove it for clarity and reflow
> comments & lines as needed.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
Reviewed-by: Brian Foster <bfoster@redhat.com>
> repair/phase5.c | 317 ++++++++++++++++++++++++++-----------------------------
> 1 file changed, 151 insertions(+), 166 deletions(-)
>
>
> diff --git a/repair/phase5.c b/repair/phase5.c
> index 677297fe..84c05a13 100644
> --- a/repair/phase5.c
> +++ b/repair/phase5.c
> @@ -2313,201 +2313,186 @@ phase5_func(
> if (verbose)
> do_log(_(" - agno = %d\n"), agno);
>
> - {
> - /*
> - * build up incore bno and bcnt extent btrees
> - */
> - num_extents = mk_incore_fstree(mp, agno);
> + /*
> + * build up incore bno and bcnt extent btrees
> + */
> + num_extents = mk_incore_fstree(mp, agno);
>
> #ifdef XR_BLD_FREE_TRACE
> - fprintf(stderr, "# of bno extents is %d\n",
> - count_bno_extents(agno));
> + fprintf(stderr, "# of bno extents is %d\n", count_bno_extents(agno));
> #endif
>
> - if (num_extents == 0) {
> - /*
> - * XXX - what we probably should do here is pick an
> - * inode for a regular file in the allocation group
> - * that has space allocated and shoot it by traversing
> - * the bmap list and putting all its extents on the
> - * incore freespace trees, clearing the inode,
> - * and clearing the in-use bit in the incore inode
> - * tree. Then try mk_incore_fstree() again.
> - */
> - do_error(_("unable to rebuild AG %u. "
> - "Not enough free space in on-disk AG.\n"),
> - agno);
> - }
> -
> - /*
> - * ok, now set up the btree cursors for the
> - * on-disk btrees (includs pre-allocating all
> - * required blocks for the trees themselves)
> - */
> - init_ino_cursor(mp, agno, &ino_btree_curs, &num_inos,
> - &num_free_inos, 0);
> -
> - if (xfs_sb_version_hasfinobt(&mp->m_sb))
> - init_ino_cursor(mp, agno, &fino_btree_curs,
> - &finobt_num_inos, &finobt_num_free_inos,
> - 1);
> -
> - sb_icount_ag[agno] += num_inos;
> - sb_ifree_ag[agno] += num_free_inos;
> -
> - /*
> - * Set up the btree cursors for the on-disk rmap btrees,
> - * which includes pre-allocating all required blocks.
> - */
> - init_rmapbt_cursor(mp, agno, &rmap_btree_curs);
> -
> + if (num_extents == 0) {
> /*
> - * Set up the btree cursors for the on-disk refcount btrees,
> - * which includes pre-allocating all required blocks.
> + * XXX - what we probably should do here is pick an inode for
> + * a regular file in the allocation group that has space
> + * allocated and shoot it by traversing the bmap list and
> + * putting all its extents on the incore freespace trees,
> + * clearing the inode, and clearing the in-use bit in the
> + * incore inode tree. Then try mk_incore_fstree() again.
> */
> - init_refc_cursor(mp, agno, &refcnt_btree_curs);
> -
> - num_extents = count_bno_extents_blocks(agno, &num_freeblocks);
> + do_error(
> +_("unable to rebuild AG %u. Not enough free space in on-disk AG.\n"),
> + agno);
> + }
> +
> + /*
> + * ok, now set up the btree cursors for the on-disk btrees (includes
> + * pre-allocating all required blocks for the trees themselves)
> + */
> + init_ino_cursor(mp, agno, &ino_btree_curs, &num_inos,
> + &num_free_inos, 0);
> +
> + if (xfs_sb_version_hasfinobt(&mp->m_sb))
> + init_ino_cursor(mp, agno, &fino_btree_curs, &finobt_num_inos,
> + &finobt_num_free_inos, 1);
> +
> + sb_icount_ag[agno] += num_inos;
> + sb_ifree_ag[agno] += num_free_inos;
> +
> + /*
> + * Set up the btree cursors for the on-disk rmap btrees, which includes
> + * pre-allocating all required blocks.
> + */
> + init_rmapbt_cursor(mp, agno, &rmap_btree_curs);
> +
> + /*
> + * Set up the btree cursors for the on-disk refcount btrees,
> + * which includes pre-allocating all required blocks.
> + */
> + init_refc_cursor(mp, agno, &refcnt_btree_curs);
> +
> + num_extents = count_bno_extents_blocks(agno, &num_freeblocks);
> + /*
> + * lose two blocks per AG -- the space tree roots are counted as
> + * allocated since the space trees always have roots
> + */
> + sb_fdblocks_ag[agno] += num_freeblocks - 2;
> +
> + if (num_extents == 0) {
> /*
> - * lose two blocks per AG -- the space tree roots
> - * are counted as allocated since the space trees
> - * always have roots
> + * XXX - what we probably should do here is pick an inode for
> + * a regular file in the allocation group that has space
> + * allocated and shoot it by traversing the bmap list and
> + * putting all its extents on the incore freespace trees,
> + * clearing the inode, and clearing the in-use bit in the
> + * incore inode tree. Then try mk_incore_fstree() again.
> */
> - sb_fdblocks_ag[agno] += num_freeblocks - 2;
> -
> - if (num_extents == 0) {
> - /*
> - * XXX - what we probably should do here is pick an
> - * inode for a regular file in the allocation group
> - * that has space allocated and shoot it by traversing
> - * the bmap list and putting all its extents on the
> - * incore freespace trees, clearing the inode,
> - * and clearing the in-use bit in the incore inode
> - * tree. Then try mk_incore_fstree() again.
> - */
> - do_error(
> - _("unable to rebuild AG %u. No free space.\n"), agno);
> - }
> + do_error(_("unable to rebuild AG %u. No free space.\n"), agno);
> + }
>
> #ifdef XR_BLD_FREE_TRACE
> - fprintf(stderr, "# of bno extents is %d\n", num_extents);
> + fprintf(stderr, "# of bno extents is %d\n", num_extents);
> #endif
>
> - /*
> - * track blocks that we might really lose
> - */
> - extra_blocks = calculate_freespace_cursor(mp, agno,
> - &num_extents, &bno_btree_curs);
> + /*
> + * track blocks that we might really lose
> + */
> + extra_blocks = calculate_freespace_cursor(mp, agno,
> + &num_extents, &bno_btree_curs);
>
> - /*
> - * freespace btrees live in the "free space" but
> - * the filesystem treats AGFL blocks as allocated
> - * since they aren't described by the freespace trees
> - */
> + /*
> + * freespace btrees live in the "free space" but the filesystem treats
> + * AGFL blocks as allocated since they aren't described by the
> + * freespace trees
> + */
>
> - /*
> - * see if we can fit all the extra blocks into the AGFL
> - */
> - extra_blocks = (extra_blocks - libxfs_agfl_size(mp) > 0)
> - ? extra_blocks - libxfs_agfl_size(mp)
> - : 0;
> + /*
> + * see if we can fit all the extra blocks into the AGFL
> + */
> + extra_blocks = (extra_blocks - libxfs_agfl_size(mp) > 0) ?
> + extra_blocks - libxfs_agfl_size(mp) : 0;
>
> - if (extra_blocks > 0)
> - sb_fdblocks_ag[agno] -= extra_blocks;
> + if (extra_blocks > 0)
> + sb_fdblocks_ag[agno] -= extra_blocks;
>
> - bcnt_btree_curs = bno_btree_curs;
> + bcnt_btree_curs = bno_btree_curs;
>
> - bno_btree_curs.owner = XFS_RMAP_OWN_AG;
> - bcnt_btree_curs.owner = XFS_RMAP_OWN_AG;
> - setup_cursor(mp, agno, &bno_btree_curs);
> - setup_cursor(mp, agno, &bcnt_btree_curs);
> + bno_btree_curs.owner = XFS_RMAP_OWN_AG;
> + bcnt_btree_curs.owner = XFS_RMAP_OWN_AG;
> + setup_cursor(mp, agno, &bno_btree_curs);
> + setup_cursor(mp, agno, &bcnt_btree_curs);
>
> #ifdef XR_BLD_FREE_TRACE
> - fprintf(stderr, "# of bno extents is %d\n",
> - count_bno_extents(agno));
> - fprintf(stderr, "# of bcnt extents is %d\n",
> - count_bcnt_extents(agno));
> + fprintf(stderr, "# of bno extents is %d\n", count_bno_extents(agno));
> + fprintf(stderr, "# of bcnt extents is %d\n", count_bcnt_extents(agno));
> #endif
>
> - /*
> - * now rebuild the freespace trees
> - */
> - freeblks1 = build_freespace_tree(mp, agno,
> + /*
> + * now rebuild the freespace trees
> + */
> + freeblks1 = build_freespace_tree(mp, agno,
> &bno_btree_curs, XFS_BTNUM_BNO);
> #ifdef XR_BLD_FREE_TRACE
> - fprintf(stderr, "# of free blocks == %d\n", freeblks1);
> + fprintf(stderr, "# of free blocks == %d\n", freeblks1);
> #endif
> - write_cursor(&bno_btree_curs);
> + write_cursor(&bno_btree_curs);
>
> #ifdef DEBUG
> - freeblks2 = build_freespace_tree(mp, agno,
> - &bcnt_btree_curs, XFS_BTNUM_CNT);
> + freeblks2 = build_freespace_tree(mp, agno,
> + &bcnt_btree_curs, XFS_BTNUM_CNT);
> #else
> - (void) build_freespace_tree(mp, agno,
> - &bcnt_btree_curs, XFS_BTNUM_CNT);
> + (void) build_freespace_tree(mp, agno, &bcnt_btree_curs, XFS_BTNUM_CNT);
> #endif
> - write_cursor(&bcnt_btree_curs);
> -
> - ASSERT(freeblks1 == freeblks2);
> -
> - if (xfs_sb_version_hasrmapbt(&mp->m_sb)) {
> - build_rmap_tree(mp, agno, &rmap_btree_curs);
> - write_cursor(&rmap_btree_curs);
> - sb_fdblocks_ag[agno] += (rmap_btree_curs.num_tot_blocks -
> - rmap_btree_curs.num_free_blocks) - 1;
> - }
> -
> - if (xfs_sb_version_hasreflink(&mp->m_sb)) {
> - build_refcount_tree(mp, agno, &refcnt_btree_curs);
> - write_cursor(&refcnt_btree_curs);
> - }
> -
> - /*
> - * set up agf and agfl
> - */
> - build_agf_agfl(mp, agno, &bno_btree_curs,
> - &bcnt_btree_curs, freeblks1, extra_blocks,
> - &rmap_btree_curs, &refcnt_btree_curs, lost_fsb);
> - /*
> - * build inode allocation tree.
> - */
> - build_ino_tree(mp, agno, &ino_btree_curs, XFS_BTNUM_INO,
> - &agi_stat);
> - write_cursor(&ino_btree_curs);
> -
> - /*
> - * build free inode tree
> - */
> - if (xfs_sb_version_hasfinobt(&mp->m_sb)) {
> - build_ino_tree(mp, agno, &fino_btree_curs,
> - XFS_BTNUM_FINO, NULL);
> - write_cursor(&fino_btree_curs);
> - }
> -
> - /* build the agi */
> - build_agi(mp, agno, &ino_btree_curs, &fino_btree_curs,
> - &agi_stat);
> -
> - /*
> - * tear down cursors
> - */
> - finish_cursor(&bno_btree_curs);
> - finish_cursor(&ino_btree_curs);
> - if (xfs_sb_version_hasrmapbt(&mp->m_sb))
> - finish_cursor(&rmap_btree_curs);
> - if (xfs_sb_version_hasreflink(&mp->m_sb))
> - finish_cursor(&refcnt_btree_curs);
> - if (xfs_sb_version_hasfinobt(&mp->m_sb))
> - finish_cursor(&fino_btree_curs);
> - finish_cursor(&bcnt_btree_curs);
> -
> - /*
> - * release the incore per-AG bno/bcnt trees so
> - * the extent nodes can be recycled
> - */
> - release_agbno_extent_tree(agno);
> - release_agbcnt_extent_tree(agno);
> + write_cursor(&bcnt_btree_curs);
> +
> + ASSERT(freeblks1 == freeblks2);
> +
> + if (xfs_sb_version_hasrmapbt(&mp->m_sb)) {
> + build_rmap_tree(mp, agno, &rmap_btree_curs);
> + write_cursor(&rmap_btree_curs);
> + sb_fdblocks_ag[agno] += (rmap_btree_curs.num_tot_blocks -
> + rmap_btree_curs.num_free_blocks) - 1;
> + }
> +
> + if (xfs_sb_version_hasreflink(&mp->m_sb)) {
> + build_refcount_tree(mp, agno, &refcnt_btree_curs);
> + write_cursor(&refcnt_btree_curs);
> }
> +
> + /*
> + * set up agf and agfl
> + */
> + build_agf_agfl(mp, agno, &bno_btree_curs,
> + &bcnt_btree_curs, freeblks1, extra_blocks,
> + &rmap_btree_curs, &refcnt_btree_curs, lost_fsb);
> + /*
> + * build inode allocation tree.
> + */
> + build_ino_tree(mp, agno, &ino_btree_curs, XFS_BTNUM_INO, &agi_stat);
> + write_cursor(&ino_btree_curs);
> +
> + /*
> + * build free inode tree
> + */
> + if (xfs_sb_version_hasfinobt(&mp->m_sb)) {
> + build_ino_tree(mp, agno, &fino_btree_curs,
> + XFS_BTNUM_FINO, NULL);
> + write_cursor(&fino_btree_curs);
> + }
> +
> + /* build the agi */
> + build_agi(mp, agno, &ino_btree_curs, &fino_btree_curs, &agi_stat);
> +
> + /*
> + * tear down cursors
> + */
> + finish_cursor(&bno_btree_curs);
> + finish_cursor(&ino_btree_curs);
> + if (xfs_sb_version_hasrmapbt(&mp->m_sb))
> + finish_cursor(&rmap_btree_curs);
> + if (xfs_sb_version_hasreflink(&mp->m_sb))
> + finish_cursor(&refcnt_btree_curs);
> + if (xfs_sb_version_hasfinobt(&mp->m_sb))
> + finish_cursor(&fino_btree_curs);
> + finish_cursor(&bcnt_btree_curs);
> +
> + /*
> + * release the incore per-AG bno/bcnt trees so the extent nodes
> + * can be recycled
> + */
> + release_agbno_extent_tree(agno);
> + release_agbcnt_extent_tree(agno);
> PROG_RPT_INC(prog_rpt_done[agno], 1);
> }
>
>
next prev parent reply other threads:[~2020-05-27 12:15 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-20 1:50 [PATCH v5 0/9] xfs_repair: use btree bulk loading Darrick J. Wong
2020-05-20 1:50 ` [PATCH 1/9] xfs_repair: port the online repair newbt structure Darrick J. Wong
2020-05-27 12:15 ` Brian Foster
2020-05-27 22:34 ` Darrick J. Wong
2020-05-28 15:08 ` Brian Foster
2020-05-29 21:01 ` Darrick J. Wong
2020-06-01 12:03 ` Brian Foster
2020-06-02 0:12 ` Darrick J. Wong
2020-05-20 1:50 ` [PATCH 2/9] xfs_repair: remove gratuitous code block in phase5 Darrick J. Wong
2020-05-27 12:15 ` Brian Foster [this message]
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-05-20 1:51 ` [PATCH 4/9] xfs_repair: rebuild free space btrees with bulk loader Darrick J. Wong
2020-05-28 15:10 ` Brian Foster
2020-05-29 21:39 ` Darrick J. Wong
2020-06-01 12:05 ` Brian Foster
2020-06-02 0:21 ` Darrick J. Wong
2020-05-20 1:51 ` [PATCH 5/9] xfs_repair: rebuild inode " Darrick J. Wong
2020-05-28 15:11 ` Brian Foster
2020-05-29 22:18 ` Darrick J. Wong
2020-05-29 22:32 ` Darrick J. Wong
2020-05-20 1:51 ` [PATCH 6/9] xfs_repair: rebuild reverse mapping " Darrick J. Wong
2020-05-20 1:51 ` [PATCH 7/9] xfs_repair: rebuild refcount " Darrick J. Wong
2020-05-20 1:51 ` [PATCH 8/9] xfs_repair: remove old btree rebuild support code Darrick J. Wong
2020-05-20 1:51 ` [PATCH 9/9] xfs_repair: track blocks lost during btree construction via extents Darrick J. Wong
2020-05-28 17:00 ` Brian Foster
2020-05-29 22:19 ` 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=20200527121541.GB12014@bfoster \
--to=bfoster@redhat.com \
--cc=darrick.wong@oracle.com \
--cc=linux-xfs@vger.kernel.org \
--cc=sandeen@redhat.com \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.