diff mbox series

[10/12] xfs_repair: rebuild refcount btrees with bulk loader

Message ID 159107207766.315004.3208486320108630923.stgit@magnolia (mailing list archive)
State Accepted
Headers show
Series xfs_repair: use btree bulk loading | expand

Commit Message

Darrick J. Wong June 2, 2020, 4:27 a.m. UTC
From: Darrick J. Wong <darrick.wong@oracle.com>

Use the btree bulk loading functions to rebuild the refcount btrees
and drop the open-coded implementation.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/libxfs_api_defs.h |    1 
 repair/agbtree.c         |   71 ++++++++++
 repair/agbtree.h         |    5 +
 repair/phase5.c          |  341 ++--------------------------------------------
 4 files changed, 93 insertions(+), 325 deletions(-)

Comments

Brian Foster June 18, 2020, 3:26 p.m. UTC | #1
On Mon, Jun 01, 2020 at 09:27:57PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Use the btree bulk loading functions to rebuild the refcount btrees
> and drop the open-coded implementation.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  libxfs/libxfs_api_defs.h |    1 
>  repair/agbtree.c         |   71 ++++++++++
>  repair/agbtree.h         |    5 +
>  repair/phase5.c          |  341 ++--------------------------------------------
>  4 files changed, 93 insertions(+), 325 deletions(-)
> 
> 
...
> diff --git a/repair/phase5.c b/repair/phase5.c
> index 1c6448f4..ad009416 100644
> --- a/repair/phase5.c
> +++ b/repair/phase5.c
...
> @@ -817,10 +510,14 @@ build_agf_agfl(
>  				cpu_to_be32(btr_rmap->newbt.afake.af_blocks);
>  	}
>  
> -	agf->agf_refcount_root = cpu_to_be32(refcnt_bt->root);
> -	agf->agf_refcount_level = cpu_to_be32(refcnt_bt->num_levels);
> -	agf->agf_refcount_blocks = cpu_to_be32(refcnt_bt->num_tot_blocks -
> -			refcnt_bt->num_free_blocks);
> +	if (xfs_sb_version_hasreflink(&mp->m_sb)) {
> +		agf->agf_refcount_root =
> +				cpu_to_be32(btr_refc->newbt.afake.af_root);
> +		agf->agf_refcount_level =
> +				cpu_to_be32(btr_refc->newbt.afake.af_levels);
> +		agf->agf_refcount_blocks =
> +				cpu_to_be32(btr_refc->newbt.afake.af_blocks);
> +	}

It looks like the previous cursor variant (refcnt_bt) would be zeroed
out if the feature isn't enabled (causing this to zero out the agf
fields on disk), whereas now we only write the fields when the feature
is enabled. Any concern over removing that zeroing behavior? Also note
that an assert further down unconditionally reads the
->agf_refcount_root field.

BTW, I suppose the same question may apply to the previous patch as
well...

Brian

>  
>  	/*
>  	 * Count and record the number of btree blocks consumed if required.
> @@ -981,7 +678,7 @@ phase5_func(
>  	struct bt_rebuild	btr_ino;
>  	struct bt_rebuild	btr_fino;
>  	struct bt_rebuild	btr_rmap;
> -	bt_status_t		refcnt_btree_curs;
> +	struct bt_rebuild	btr_refc;
>  	int			extra_blocks = 0;
>  	uint			num_freeblocks;
>  	xfs_agblock_t		num_extents;
> @@ -1017,11 +714,7 @@ _("unable to rebuild AG %u.  Not enough free space in on-disk AG.\n"),
>  
>  	init_rmapbt_cursor(&sc, agno, num_freeblocks, &btr_rmap);
>  
> -	/*
> -	 * 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);
> +	init_refc_cursor(&sc, agno, num_freeblocks, &btr_refc);
>  
>  	num_extents = count_bno_extents_blocks(agno, &num_freeblocks);
>  	/*
> @@ -1085,16 +778,14 @@ _("unable to rebuild AG %u.  Not enough free space in on-disk AG.\n"),
>  		sb_fdblocks_ag[agno] += btr_rmap.newbt.afake.af_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(&sc, agno, &btr_refc);
>  
>  	/*
>  	 * set up agf and agfl
>  	 */
> -	build_agf_agfl(mp, agno, &btr_bno, &btr_cnt, &btr_rmap,
> -			&refcnt_btree_curs, lost_fsb);
> +	build_agf_agfl(mp, agno, &btr_bno, &btr_cnt, &btr_rmap, &btr_refc,
> +			lost_fsb);
>  
>  	build_inode_btrees(&sc, agno, &btr_ino, &btr_fino);
>  
> @@ -1112,7 +803,7 @@ _("unable to rebuild AG %u.  Not enough free space in on-disk AG.\n"),
>  	if (xfs_sb_version_hasrmapbt(&mp->m_sb))
>  		finish_rebuild(mp, &btr_rmap, lost_fsb);
>  	if (xfs_sb_version_hasreflink(&mp->m_sb))
> -		finish_cursor(&refcnt_btree_curs);
> +		finish_rebuild(mp, &btr_refc, lost_fsb);
>  
>  	/*
>  	 * release the incore per-AG bno/bcnt trees so the extent nodes
>
Darrick J. Wong June 18, 2020, 4:56 p.m. UTC | #2
On Thu, Jun 18, 2020 at 11:26:17AM -0400, Brian Foster wrote:
> On Mon, Jun 01, 2020 at 09:27:57PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Use the btree bulk loading functions to rebuild the refcount btrees
> > and drop the open-coded implementation.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  libxfs/libxfs_api_defs.h |    1 
> >  repair/agbtree.c         |   71 ++++++++++
> >  repair/agbtree.h         |    5 +
> >  repair/phase5.c          |  341 ++--------------------------------------------
> >  4 files changed, 93 insertions(+), 325 deletions(-)
> > 
> > 
> ...
> > diff --git a/repair/phase5.c b/repair/phase5.c
> > index 1c6448f4..ad009416 100644
> > --- a/repair/phase5.c
> > +++ b/repair/phase5.c
> ...
> > @@ -817,10 +510,14 @@ build_agf_agfl(
> >  				cpu_to_be32(btr_rmap->newbt.afake.af_blocks);
> >  	}
> >  
> > -	agf->agf_refcount_root = cpu_to_be32(refcnt_bt->root);
> > -	agf->agf_refcount_level = cpu_to_be32(refcnt_bt->num_levels);
> > -	agf->agf_refcount_blocks = cpu_to_be32(refcnt_bt->num_tot_blocks -
> > -			refcnt_bt->num_free_blocks);
> > +	if (xfs_sb_version_hasreflink(&mp->m_sb)) {
> > +		agf->agf_refcount_root =
> > +				cpu_to_be32(btr_refc->newbt.afake.af_root);
> > +		agf->agf_refcount_level =
> > +				cpu_to_be32(btr_refc->newbt.afake.af_levels);
> > +		agf->agf_refcount_blocks =
> > +				cpu_to_be32(btr_refc->newbt.afake.af_blocks);
> > +	}
> 
> It looks like the previous cursor variant (refcnt_bt) would be zeroed
> out if the feature isn't enabled (causing this to zero out the agf
> fields on disk), whereas now we only write the fields when the feature
> is enabled. Any concern over removing that zeroing behavior? Also note
> that an assert further down unconditionally reads the
> ->agf_refcount_root field.
> 
> BTW, I suppose the same question may apply to the previous patch as
> well...

I'll double check, but we do memset the AGF (and AGI) to zero before we
start initializing things, so the asserts should be fine even on
!reflink filesystems.

--D

> Brian
> 
> >  
> >  	/*
> >  	 * Count and record the number of btree blocks consumed if required.
> > @@ -981,7 +678,7 @@ phase5_func(
> >  	struct bt_rebuild	btr_ino;
> >  	struct bt_rebuild	btr_fino;
> >  	struct bt_rebuild	btr_rmap;
> > -	bt_status_t		refcnt_btree_curs;
> > +	struct bt_rebuild	btr_refc;
> >  	int			extra_blocks = 0;
> >  	uint			num_freeblocks;
> >  	xfs_agblock_t		num_extents;
> > @@ -1017,11 +714,7 @@ _("unable to rebuild AG %u.  Not enough free space in on-disk AG.\n"),
> >  
> >  	init_rmapbt_cursor(&sc, agno, num_freeblocks, &btr_rmap);
> >  
> > -	/*
> > -	 * 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);
> > +	init_refc_cursor(&sc, agno, num_freeblocks, &btr_refc);
> >  
> >  	num_extents = count_bno_extents_blocks(agno, &num_freeblocks);
> >  	/*
> > @@ -1085,16 +778,14 @@ _("unable to rebuild AG %u.  Not enough free space in on-disk AG.\n"),
> >  		sb_fdblocks_ag[agno] += btr_rmap.newbt.afake.af_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(&sc, agno, &btr_refc);
> >  
> >  	/*
> >  	 * set up agf and agfl
> >  	 */
> > -	build_agf_agfl(mp, agno, &btr_bno, &btr_cnt, &btr_rmap,
> > -			&refcnt_btree_curs, lost_fsb);
> > +	build_agf_agfl(mp, agno, &btr_bno, &btr_cnt, &btr_rmap, &btr_refc,
> > +			lost_fsb);
> >  
> >  	build_inode_btrees(&sc, agno, &btr_ino, &btr_fino);
> >  
> > @@ -1112,7 +803,7 @@ _("unable to rebuild AG %u.  Not enough free space in on-disk AG.\n"),
> >  	if (xfs_sb_version_hasrmapbt(&mp->m_sb))
> >  		finish_rebuild(mp, &btr_rmap, lost_fsb);
> >  	if (xfs_sb_version_hasreflink(&mp->m_sb))
> > -		finish_cursor(&refcnt_btree_curs);
> > +		finish_rebuild(mp, &btr_refc, lost_fsb);
> >  
> >  	/*
> >  	 * release the incore per-AG bno/bcnt trees so the extent nodes
> > 
>
Brian Foster June 18, 2020, 5:05 p.m. UTC | #3
On Thu, Jun 18, 2020 at 09:56:22AM -0700, Darrick J. Wong wrote:
> On Thu, Jun 18, 2020 at 11:26:17AM -0400, Brian Foster wrote:
> > On Mon, Jun 01, 2020 at 09:27:57PM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > 
> > > Use the btree bulk loading functions to rebuild the refcount btrees
> > > and drop the open-coded implementation.
> > > 
> > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > ---
> > >  libxfs/libxfs_api_defs.h |    1 
> > >  repair/agbtree.c         |   71 ++++++++++
> > >  repair/agbtree.h         |    5 +
> > >  repair/phase5.c          |  341 ++--------------------------------------------
> > >  4 files changed, 93 insertions(+), 325 deletions(-)
> > > 
> > > 
> > ...
> > > diff --git a/repair/phase5.c b/repair/phase5.c
> > > index 1c6448f4..ad009416 100644
> > > --- a/repair/phase5.c
> > > +++ b/repair/phase5.c
> > ...
> > > @@ -817,10 +510,14 @@ build_agf_agfl(
> > >  				cpu_to_be32(btr_rmap->newbt.afake.af_blocks);
> > >  	}
> > >  
> > > -	agf->agf_refcount_root = cpu_to_be32(refcnt_bt->root);
> > > -	agf->agf_refcount_level = cpu_to_be32(refcnt_bt->num_levels);
> > > -	agf->agf_refcount_blocks = cpu_to_be32(refcnt_bt->num_tot_blocks -
> > > -			refcnt_bt->num_free_blocks);
> > > +	if (xfs_sb_version_hasreflink(&mp->m_sb)) {
> > > +		agf->agf_refcount_root =
> > > +				cpu_to_be32(btr_refc->newbt.afake.af_root);
> > > +		agf->agf_refcount_level =
> > > +				cpu_to_be32(btr_refc->newbt.afake.af_levels);
> > > +		agf->agf_refcount_blocks =
> > > +				cpu_to_be32(btr_refc->newbt.afake.af_blocks);
> > > +	}
> > 
> > It looks like the previous cursor variant (refcnt_bt) would be zeroed
> > out if the feature isn't enabled (causing this to zero out the agf
> > fields on disk), whereas now we only write the fields when the feature
> > is enabled. Any concern over removing that zeroing behavior? Also note
> > that an assert further down unconditionally reads the
> > ->agf_refcount_root field.
> > 
> > BTW, I suppose the same question may apply to the previous patch as
> > well...
> 
> I'll double check, but we do memset the AGF (and AGI) to zero before we
> start initializing things, so the asserts should be fine even on
> !reflink filesystems.
> 

Ah, so the implicit per-field zeroing behavior of the old implementation
is superfluous. Assert aside, I just wanted to make sure we weren't
removing some subtle mechanism for clearing unused metadata fields if
they happened to contain garbage. That is not the case, so this one
looks fine to me as well:

Reviewed-by: Brian Foster <bfoster@redhat.com>

> --D
> 
> > Brian
> > 
> > >  
> > >  	/*
> > >  	 * Count and record the number of btree blocks consumed if required.
> > > @@ -981,7 +678,7 @@ phase5_func(
> > >  	struct bt_rebuild	btr_ino;
> > >  	struct bt_rebuild	btr_fino;
> > >  	struct bt_rebuild	btr_rmap;
> > > -	bt_status_t		refcnt_btree_curs;
> > > +	struct bt_rebuild	btr_refc;
> > >  	int			extra_blocks = 0;
> > >  	uint			num_freeblocks;
> > >  	xfs_agblock_t		num_extents;
> > > @@ -1017,11 +714,7 @@ _("unable to rebuild AG %u.  Not enough free space in on-disk AG.\n"),
> > >  
> > >  	init_rmapbt_cursor(&sc, agno, num_freeblocks, &btr_rmap);
> > >  
> > > -	/*
> > > -	 * 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);
> > > +	init_refc_cursor(&sc, agno, num_freeblocks, &btr_refc);
> > >  
> > >  	num_extents = count_bno_extents_blocks(agno, &num_freeblocks);
> > >  	/*
> > > @@ -1085,16 +778,14 @@ _("unable to rebuild AG %u.  Not enough free space in on-disk AG.\n"),
> > >  		sb_fdblocks_ag[agno] += btr_rmap.newbt.afake.af_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(&sc, agno, &btr_refc);
> > >  
> > >  	/*
> > >  	 * set up agf and agfl
> > >  	 */
> > > -	build_agf_agfl(mp, agno, &btr_bno, &btr_cnt, &btr_rmap,
> > > -			&refcnt_btree_curs, lost_fsb);
> > > +	build_agf_agfl(mp, agno, &btr_bno, &btr_cnt, &btr_rmap, &btr_refc,
> > > +			lost_fsb);
> > >  
> > >  	build_inode_btrees(&sc, agno, &btr_ino, &btr_fino);
> > >  
> > > @@ -1112,7 +803,7 @@ _("unable to rebuild AG %u.  Not enough free space in on-disk AG.\n"),
> > >  	if (xfs_sb_version_hasrmapbt(&mp->m_sb))
> > >  		finish_rebuild(mp, &btr_rmap, lost_fsb);
> > >  	if (xfs_sb_version_hasreflink(&mp->m_sb))
> > > -		finish_cursor(&refcnt_btree_curs);
> > > +		finish_rebuild(mp, &btr_refc, lost_fsb);
> > >  
> > >  	/*
> > >  	 * release the incore per-AG bno/bcnt trees so the extent nodes
> > > 
> > 
>
diff mbox series

Patch

diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
index 0026ca45..1a7cdbf9 100644
--- a/libxfs/libxfs_api_defs.h
+++ b/libxfs/libxfs_api_defs.h
@@ -135,6 +135,7 @@ 
 #define xfs_refcountbt_calc_reserves	libxfs_refcountbt_calc_reserves
 #define xfs_refcountbt_init_cursor	libxfs_refcountbt_init_cursor
 #define xfs_refcountbt_maxrecs		libxfs_refcountbt_maxrecs
+#define xfs_refcountbt_stage_cursor	libxfs_refcountbt_stage_cursor
 #define xfs_refcount_get_rec		libxfs_refcount_get_rec
 #define xfs_refcount_lookup_le		libxfs_refcount_lookup_le
 
diff --git a/repair/agbtree.c b/repair/agbtree.c
index 7b075a52..d3639fe4 100644
--- a/repair/agbtree.c
+++ b/repair/agbtree.c
@@ -585,3 +585,74 @@  _("Error %d while creating rmap btree for AG %u.\n"), error, agno);
 	libxfs_btree_del_cursor(btr->cur, 0);
 	free_slab_cursor(&btr->slab_cursor);
 }
+
+/* rebuild the refcount tree */
+
+/* Grab one refcount record. */
+static int
+get_refcountbt_record(
+	struct xfs_btree_cur		*cur,
+	void				*priv)
+{
+	struct xfs_refcount_irec	*rec;
+	struct bt_rebuild		*btr = priv;
+
+	rec = pop_slab_cursor(btr->slab_cursor);
+	memcpy(&cur->bc_rec.rc, rec, sizeof(struct xfs_refcount_irec));
+	return 0;
+}
+
+/* Set up the refcount rebuild parameters. */
+void
+init_refc_cursor(
+	struct repair_ctx	*sc,
+	xfs_agnumber_t		agno,
+	unsigned int		free_space,
+	struct bt_rebuild	*btr)
+{
+	int			error;
+
+	if (!xfs_sb_version_hasreflink(&sc->mp->m_sb))
+		return;
+
+	init_rebuild(sc, &XFS_RMAP_OINFO_REFC, free_space, btr);
+	btr->cur = libxfs_refcountbt_stage_cursor(sc->mp, &btr->newbt.afake,
+			agno);
+
+	btr->bload.get_record = get_refcountbt_record;
+	btr->bload.claim_block = rebuild_claim_block;
+
+	/* Compute how many blocks we'll need. */
+	error = -libxfs_btree_bload_compute_geometry(btr->cur, &btr->bload,
+			refcount_record_count(sc->mp, agno));
+	if (error)
+		do_error(
+_("Unable to compute refcount btree geometry, error %d.\n"), error);
+
+	reserve_btblocks(sc->mp, agno, btr, btr->bload.nr_blocks);
+}
+
+/* Rebuild a refcount btree. */
+void
+build_refcount_tree(
+	struct repair_ctx	*sc,
+	xfs_agnumber_t		agno,
+	struct bt_rebuild	*btr)
+{
+	int			error;
+
+	error = init_refcount_cursor(agno, &btr->slab_cursor);
+	if (error)
+		do_error(
+_("Insufficient memory to construct refcount cursor.\n"));
+
+	/* Add all observed refcount records. */
+	error = -libxfs_btree_bload(btr->cur, &btr->bload, btr);
+	if (error)
+		do_error(
+_("Error %d while creating refcount btree for AG %u.\n"), error, agno);
+
+	/* Since we're not writing the AGF yet, no need to commit the cursor */
+	libxfs_btree_del_cursor(btr->cur, 0);
+	free_slab_cursor(&btr->slab_cursor);
+}
diff --git a/repair/agbtree.h b/repair/agbtree.h
index ca6e70de..6bbeb022 100644
--- a/repair/agbtree.h
+++ b/repair/agbtree.h
@@ -54,4 +54,9 @@  void init_rmapbt_cursor(struct repair_ctx *sc, xfs_agnumber_t agno,
 void build_rmap_tree(struct repair_ctx *sc, xfs_agnumber_t agno,
 		struct bt_rebuild *btr);
 
+void init_refc_cursor(struct repair_ctx *sc, xfs_agnumber_t agno,
+		unsigned int free_space, struct bt_rebuild *btr);
+void build_refcount_tree(struct repair_ctx *sc, xfs_agnumber_t agno,
+		struct bt_rebuild *btr);
+
 #endif /* __XFS_REPAIR_AG_BTREE_H__ */
diff --git a/repair/phase5.c b/repair/phase5.c
index 1c6448f4..ad009416 100644
--- a/repair/phase5.c
+++ b/repair/phase5.c
@@ -417,313 +417,6 @@  build_agi(
 	libxfs_buf_relse(agi_buf);
 }
 
-/* rebuild the refcount tree */
-
-/*
- * we don't have to worry here about how chewing up free extents
- * may perturb things because reflink tree building happens before
- * freespace tree building.
- */
-static void
-init_refc_cursor(
-	struct xfs_mount	*mp,
-	xfs_agnumber_t		agno,
-	struct bt_status	*btree_curs)
-{
-	size_t			num_recs;
-	int			level;
-	struct bt_stat_level	*lptr;
-	struct bt_stat_level	*p_lptr;
-	xfs_extlen_t		blocks_allocated;
-
-	if (!xfs_sb_version_hasreflink(&mp->m_sb)) {
-		memset(btree_curs, 0, sizeof(struct bt_status));
-		return;
-	}
-
-	lptr = &btree_curs->level[0];
-	btree_curs->init = 1;
-	btree_curs->owner = XFS_RMAP_OWN_REFC;
-
-	/*
-	 * build up statistics
-	 */
-	num_recs = refcount_record_count(mp, agno);
-	if (num_recs == 0) {
-		/*
-		 * easy corner-case -- no refcount records
-		 */
-		lptr->num_blocks = 1;
-		lptr->modulo = 0;
-		lptr->num_recs_pb = 0;
-		lptr->num_recs_tot = 0;
-
-		btree_curs->num_levels = 1;
-		btree_curs->num_tot_blocks = btree_curs->num_free_blocks = 1;
-
-		setup_cursor(mp, agno, btree_curs);
-
-		return;
-	}
-
-	blocks_allocated = lptr->num_blocks = howmany(num_recs,
-					mp->m_refc_mxr[0]);
-
-	lptr->modulo = num_recs % lptr->num_blocks;
-	lptr->num_recs_pb = num_recs / lptr->num_blocks;
-	lptr->num_recs_tot = num_recs;
-	level = 1;
-
-	if (lptr->num_blocks > 1)  {
-		for (; btree_curs->level[level-1].num_blocks > 1
-				&& level < XFS_BTREE_MAXLEVELS;
-				level++)  {
-			lptr = &btree_curs->level[level];
-			p_lptr = &btree_curs->level[level - 1];
-			lptr->num_blocks = howmany(p_lptr->num_blocks,
-					mp->m_refc_mxr[1]);
-			lptr->modulo = p_lptr->num_blocks % lptr->num_blocks;
-			lptr->num_recs_pb = p_lptr->num_blocks
-					/ lptr->num_blocks;
-			lptr->num_recs_tot = p_lptr->num_blocks;
-
-			blocks_allocated += lptr->num_blocks;
-		}
-	}
-	ASSERT(lptr->num_blocks == 1);
-	btree_curs->num_levels = level;
-
-	btree_curs->num_tot_blocks = btree_curs->num_free_blocks
-			= blocks_allocated;
-
-	setup_cursor(mp, agno, btree_curs);
-}
-
-static void
-prop_refc_cursor(
-	struct xfs_mount	*mp,
-	xfs_agnumber_t		agno,
-	struct bt_status	*btree_curs,
-	xfs_agblock_t		startbno,
-	int			level)
-{
-	struct xfs_btree_block	*bt_hdr;
-	struct xfs_refcount_key	*bt_key;
-	xfs_refcount_ptr_t	*bt_ptr;
-	xfs_agblock_t		agbno;
-	struct bt_stat_level	*lptr;
-	const struct xfs_buf_ops *ops = btnum_to_ops(XFS_BTNUM_REFC);
-	int			error;
-
-	level++;
-
-	if (level >= btree_curs->num_levels)
-		return;
-
-	lptr = &btree_curs->level[level];
-	bt_hdr = XFS_BUF_TO_BLOCK(lptr->buf_p);
-
-	if (be16_to_cpu(bt_hdr->bb_numrecs) == 0)  {
-		/*
-		 * this only happens once to initialize the
-		 * first path up the left side of the tree
-		 * where the agbno's are already set up
-		 */
-		prop_refc_cursor(mp, agno, btree_curs, startbno, level);
-	}
-
-	if (be16_to_cpu(bt_hdr->bb_numrecs) ==
-				lptr->num_recs_pb + (lptr->modulo > 0))  {
-		/*
-		 * write out current prev block, grab us a new block,
-		 * and set the rightsib pointer of current block
-		 */
-#ifdef XR_BLD_INO_TRACE
-		fprintf(stderr, " ino prop agbno %d ", lptr->prev_agbno);
-#endif
-		if (lptr->prev_agbno != NULLAGBLOCK)  {
-			ASSERT(lptr->prev_buf_p != NULL);
-			libxfs_buf_mark_dirty(lptr->prev_buf_p);
-			libxfs_buf_relse(lptr->prev_buf_p);
-		}
-		lptr->prev_agbno = lptr->agbno;
-		lptr->prev_buf_p = lptr->buf_p;
-		agbno = get_next_blockaddr(agno, level, btree_curs);
-
-		bt_hdr->bb_u.s.bb_rightsib = cpu_to_be32(agbno);
-
-		error = -libxfs_buf_get(mp->m_dev,
-				XFS_AGB_TO_DADDR(mp, agno, agbno),
-				XFS_FSB_TO_BB(mp, 1), &lptr->buf_p);
-		if (error)
-			do_error(_("Cannot grab refcountbt buffer, err=%d"),
-					error);
-		lptr->agbno = agbno;
-
-		if (lptr->modulo)
-			lptr->modulo--;
-
-		/*
-		 * initialize block header
-		 */
-		lptr->buf_p->b_ops = ops;
-		bt_hdr = XFS_BUF_TO_BLOCK(lptr->buf_p);
-		memset(bt_hdr, 0, mp->m_sb.sb_blocksize);
-		libxfs_btree_init_block(mp, lptr->buf_p, XFS_BTNUM_REFC,
-					level, 0, agno);
-
-		bt_hdr->bb_u.s.bb_leftsib = cpu_to_be32(lptr->prev_agbno);
-
-		/*
-		 * propagate extent record for first extent in new block up
-		 */
-		prop_refc_cursor(mp, agno, btree_curs, startbno, level);
-	}
-	/*
-	 * add inode info to current block
-	 */
-	be16_add_cpu(&bt_hdr->bb_numrecs, 1);
-
-	bt_key = XFS_REFCOUNT_KEY_ADDR(bt_hdr,
-				    be16_to_cpu(bt_hdr->bb_numrecs));
-	bt_ptr = XFS_REFCOUNT_PTR_ADDR(bt_hdr,
-				    be16_to_cpu(bt_hdr->bb_numrecs),
-				    mp->m_refc_mxr[1]);
-
-	bt_key->rc_startblock = cpu_to_be32(startbno);
-	*bt_ptr = cpu_to_be32(btree_curs->level[level-1].agbno);
-}
-
-/*
- * rebuilds a refcount btree given a cursor.
- */
-static void
-build_refcount_tree(
-	struct xfs_mount	*mp,
-	xfs_agnumber_t		agno,
-	struct bt_status	*btree_curs)
-{
-	xfs_agnumber_t		i;
-	xfs_agblock_t		j;
-	xfs_agblock_t		agbno;
-	struct xfs_btree_block	*bt_hdr;
-	struct xfs_refcount_irec	*refc_rec;
-	struct xfs_slab_cursor	*refc_cur;
-	struct xfs_refcount_rec	*bt_rec;
-	struct bt_stat_level	*lptr;
-	const struct xfs_buf_ops *ops = btnum_to_ops(XFS_BTNUM_REFC);
-	int			numrecs;
-	int			level = btree_curs->num_levels;
-	int			error;
-
-	for (i = 0; i < level; i++)  {
-		lptr = &btree_curs->level[i];
-
-		agbno = get_next_blockaddr(agno, i, btree_curs);
-		error = -libxfs_buf_get(mp->m_dev,
-				XFS_AGB_TO_DADDR(mp, agno, agbno),
-				XFS_FSB_TO_BB(mp, 1), &lptr->buf_p);
-		if (error)
-			do_error(_("Cannot grab refcountbt buffer, err=%d"),
-					error);
-
-		if (i == btree_curs->num_levels - 1)
-			btree_curs->root = agbno;
-
-		lptr->agbno = agbno;
-		lptr->prev_agbno = NULLAGBLOCK;
-		lptr->prev_buf_p = NULL;
-		/*
-		 * initialize block header
-		 */
-
-		lptr->buf_p->b_ops = ops;
-		bt_hdr = XFS_BUF_TO_BLOCK(lptr->buf_p);
-		memset(bt_hdr, 0, mp->m_sb.sb_blocksize);
-		libxfs_btree_init_block(mp, lptr->buf_p, XFS_BTNUM_REFC,
-					i, 0, agno);
-	}
-
-	/*
-	 * run along leaf, setting up records.  as we have to switch
-	 * blocks, call the prop_refc_cursor routine to set up the new
-	 * pointers for the parent.  that can recurse up to the root
-	 * if required.  set the sibling pointers for leaf level here.
-	 */
-	error = init_refcount_cursor(agno, &refc_cur);
-	if (error)
-		do_error(
-_("Insufficient memory to construct refcount cursor."));
-	refc_rec = pop_slab_cursor(refc_cur);
-	lptr = &btree_curs->level[0];
-
-	for (i = 0; i < lptr->num_blocks; i++)  {
-		numrecs = lptr->num_recs_pb + (lptr->modulo > 0);
-		ASSERT(refc_rec != NULL || numrecs == 0);
-
-		/*
-		 * block initialization, lay in block header
-		 */
-		lptr->buf_p->b_ops = ops;
-		bt_hdr = XFS_BUF_TO_BLOCK(lptr->buf_p);
-		memset(bt_hdr, 0, mp->m_sb.sb_blocksize);
-		libxfs_btree_init_block(mp, lptr->buf_p, XFS_BTNUM_REFC,
-					0, 0, agno);
-
-		bt_hdr->bb_u.s.bb_leftsib = cpu_to_be32(lptr->prev_agbno);
-		bt_hdr->bb_numrecs = cpu_to_be16(numrecs);
-
-		if (lptr->modulo > 0)
-			lptr->modulo--;
-
-		if (lptr->num_recs_pb > 0)
-			prop_refc_cursor(mp, agno, btree_curs,
-					refc_rec->rc_startblock, 0);
-
-		bt_rec = (struct xfs_refcount_rec *)
-			  ((char *)bt_hdr + XFS_REFCOUNT_BLOCK_LEN);
-		for (j = 0; j < be16_to_cpu(bt_hdr->bb_numrecs); j++) {
-			ASSERT(refc_rec != NULL);
-			bt_rec[j].rc_startblock =
-					cpu_to_be32(refc_rec->rc_startblock);
-			bt_rec[j].rc_blockcount =
-					cpu_to_be32(refc_rec->rc_blockcount);
-			bt_rec[j].rc_refcount = cpu_to_be32(refc_rec->rc_refcount);
-
-			refc_rec = pop_slab_cursor(refc_cur);
-		}
-
-		if (refc_rec != NULL)  {
-			/*
-			 * get next leaf level block
-			 */
-			if (lptr->prev_buf_p != NULL)  {
-#ifdef XR_BLD_RL_TRACE
-				fprintf(stderr, "writing refcntbt agbno %u\n",
-					lptr->prev_agbno);
-#endif
-				ASSERT(lptr->prev_agbno != NULLAGBLOCK);
-				libxfs_buf_mark_dirty(lptr->prev_buf_p);
-				libxfs_buf_relse(lptr->prev_buf_p);
-			}
-			lptr->prev_buf_p = lptr->buf_p;
-			lptr->prev_agbno = lptr->agbno;
-			lptr->agbno = get_next_blockaddr(agno, 0, btree_curs);
-			bt_hdr->bb_u.s.bb_rightsib = cpu_to_be32(lptr->agbno);
-
-			error = -libxfs_buf_get(mp->m_dev,
-					XFS_AGB_TO_DADDR(mp, agno, lptr->agbno),
-					XFS_FSB_TO_BB(mp, 1),
-					&lptr->buf_p);
-			if (error)
-				do_error(
-	_("Cannot grab refcountbt buffer, err=%d"),
-						error);
-		}
-	}
-	free_slab_cursor(&refc_cur);
-}
-
 /* Fill the AGFL with any leftover bnobt rebuilder blocks. */
 static void
 fill_agfl(
@@ -759,7 +452,7 @@  build_agf_agfl(
 	struct bt_rebuild	*btr_bno,
 	struct bt_rebuild	*btr_cnt,
 	struct bt_rebuild	*btr_rmap,
-	struct bt_status	*refcnt_bt,
+	struct bt_rebuild	*btr_refc,
 	struct xfs_slab		*lost_fsb)
 {
 	struct extent_tree_node	*ext_ptr;
@@ -817,10 +510,14 @@  build_agf_agfl(
 				cpu_to_be32(btr_rmap->newbt.afake.af_blocks);
 	}
 
-	agf->agf_refcount_root = cpu_to_be32(refcnt_bt->root);
-	agf->agf_refcount_level = cpu_to_be32(refcnt_bt->num_levels);
-	agf->agf_refcount_blocks = cpu_to_be32(refcnt_bt->num_tot_blocks -
-			refcnt_bt->num_free_blocks);
+	if (xfs_sb_version_hasreflink(&mp->m_sb)) {
+		agf->agf_refcount_root =
+				cpu_to_be32(btr_refc->newbt.afake.af_root);
+		agf->agf_refcount_level =
+				cpu_to_be32(btr_refc->newbt.afake.af_levels);
+		agf->agf_refcount_blocks =
+				cpu_to_be32(btr_refc->newbt.afake.af_blocks);
+	}
 
 	/*
 	 * Count and record the number of btree blocks consumed if required.
@@ -981,7 +678,7 @@  phase5_func(
 	struct bt_rebuild	btr_ino;
 	struct bt_rebuild	btr_fino;
 	struct bt_rebuild	btr_rmap;
-	bt_status_t		refcnt_btree_curs;
+	struct bt_rebuild	btr_refc;
 	int			extra_blocks = 0;
 	uint			num_freeblocks;
 	xfs_agblock_t		num_extents;
@@ -1017,11 +714,7 @@  _("unable to rebuild AG %u.  Not enough free space in on-disk AG.\n"),
 
 	init_rmapbt_cursor(&sc, agno, num_freeblocks, &btr_rmap);
 
-	/*
-	 * 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);
+	init_refc_cursor(&sc, agno, num_freeblocks, &btr_refc);
 
 	num_extents = count_bno_extents_blocks(agno, &num_freeblocks);
 	/*
@@ -1085,16 +778,14 @@  _("unable to rebuild AG %u.  Not enough free space in on-disk AG.\n"),
 		sb_fdblocks_ag[agno] += btr_rmap.newbt.afake.af_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(&sc, agno, &btr_refc);
 
 	/*
 	 * set up agf and agfl
 	 */
-	build_agf_agfl(mp, agno, &btr_bno, &btr_cnt, &btr_rmap,
-			&refcnt_btree_curs, lost_fsb);
+	build_agf_agfl(mp, agno, &btr_bno, &btr_cnt, &btr_rmap, &btr_refc,
+			lost_fsb);
 
 	build_inode_btrees(&sc, agno, &btr_ino, &btr_fino);
 
@@ -1112,7 +803,7 @@  _("unable to rebuild AG %u.  Not enough free space in on-disk AG.\n"),
 	if (xfs_sb_version_hasrmapbt(&mp->m_sb))
 		finish_rebuild(mp, &btr_rmap, lost_fsb);
 	if (xfs_sb_version_hasreflink(&mp->m_sb))
-		finish_cursor(&refcnt_btree_curs);
+		finish_rebuild(mp, &btr_refc, lost_fsb);
 
 	/*
 	 * release the incore per-AG bno/bcnt trees so the extent nodes