Re: [PATCH 2/9] xfs_repair: unindent phase 5 function

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Thu, May 14, 2020 at 11:09:50AM -0400, Brian Foster wrote:
> On Sat, May 09, 2020 at 09:31:53AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Remove the unnecessary indent in phase5_func.  No functional changes.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  repair/phase5.c |  309 +++++++++++++++++++++++++++----------------------------
> >  1 file changed, 154 insertions(+), 155 deletions(-)
> > 
> > 
> > diff --git a/repair/phase5.c b/repair/phase5.c
> > index 17b57448..f3be15de 100644
> > --- a/repair/phase5.c
> > +++ b/repair/phase5.c
> > @@ -2316,201 +2316,200 @@ 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));
> 
> Some of these multi-line statements (and comments) can be widened to 80
> chars, otherwise looks like a nice cleanup to me.

Yeah.  I think Eric's version of this fixed those things so maybe I'll
just pull his in and rebase...

--D

> Brian
> 
> >  #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);
> > -		}
> > -
> > +	if (num_extents == 0)  {
> >  		/*
> > -		 * ok, now set up the btree cursors for the
> > -		 * on-disk btrees (includs pre-allocating all
> > -		 * required blocks for the trees themselves)
> > +		 * 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_ino_cursor(mp, agno, &ino_btree_curs, &num_inos,
> > -				&num_free_inos, 0);
> > +		do_error(_("unable to rebuild AG %u.  "
> > +			  "Not enough free space in on-disk AG.\n"),
> > +			agno);
> > +	}
> >  
> > -		if (xfs_sb_version_hasfinobt(&mp->m_sb))
> > -			init_ino_cursor(mp, agno, &fino_btree_curs,
> > -					&finobt_num_inos, &finobt_num_free_inos,
> > -					1);
> > +	/*
> > +	 * 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);
> >  
> > -		sb_icount_ag[agno] += num_inos;
> > -		sb_ifree_ag[agno] += num_free_inos;
> > +	if (xfs_sb_version_hasfinobt(&mp->m_sb))
> > +		init_ino_cursor(mp, agno, &fino_btree_curs,
> > +				&finobt_num_inos, &finobt_num_free_inos,
> > +				1);
> >  
> > -		/*
> > -		 * 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);
> > +	sb_icount_ag[agno] += num_inos;
> > +	sb_ifree_ag[agno] += num_free_inos;
> >  
> > -		/*
> > -		 * 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);
> > +	/*
> > +	 * 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);
> >  
> > -		num_extents = count_bno_extents_blocks(agno, &num_freeblocks);
> > +	/*
> > +	 * 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,
> > -					&bno_btree_curs, XFS_BTNUM_BNO);
> > +	/*
> > +	 * 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);
> > +	write_cursor(&bcnt_btree_curs);
> >  
> > -		ASSERT(freeblks1 == freeblks2);
> > +	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_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);
> > -		}
> > +	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);
> > +	/*
> > +	 * 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 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);
> > +	/* 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);
> > +	/*
> > +	 * 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);
> >  
> > -		/*
> > -		 * 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);
> >  }
> >  
> > 
>