On Wed, May 19, 2021 at 11:20:52AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_refcount.c | 40 ++++++++++++++++--------------
> fs/xfs/libxfs/xfs_refcount.h | 9 ++++++-
> fs/xfs/libxfs/xfs_refcount_btree.c | 22 +++++++---------
> fs/xfs/libxfs/xfs_refcount_btree.h | 4 +--
> fs/xfs/scrub/agheader_repair.c | 2 +-
> fs/xfs/scrub/bmap.c | 8 +++---
> fs/xfs/scrub/common.c | 2 +-
> fs/xfs/xfs_fsmap.c | 3 +--
> fs/xfs/xfs_reflink.c | 4 +--
> 9 files changed, 50 insertions(+), 44 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
> index 1c2bd2949d7d..fd2b9cd7ec66 100644
> --- a/fs/xfs/libxfs/xfs_refcount.c
> +++ b/fs/xfs/libxfs/xfs_refcount.c
> @@ -22,6 +22,7 @@
> #include "xfs_bit.h"
> #include "xfs_refcount.h"
> #include "xfs_rmap.h"
> +#include "xfs_ag.h"
>
> /* Allowable refcount adjustment amounts. */
> enum xfs_refc_adjust_op {
> @@ -1142,30 +1143,30 @@ xfs_refcount_finish_one(
> struct xfs_btree_cur *rcur;
> struct xfs_buf *agbp = NULL;
> int error = 0;
> - xfs_agnumber_t agno;
> xfs_agblock_t bno;
> xfs_agblock_t new_agbno;
> unsigned long nr_ops = 0;
> int shape_changes = 0;
> + struct xfs_perag *pag;
>
> - agno = XFS_FSB_TO_AGNO(mp, startblock);
> - ASSERT(agno != NULLAGNUMBER);
> + pag = xfs_perag_get(mp, XFS_FSB_TO_AGNO(mp, startblock));
> bno = XFS_FSB_TO_AGBNO(mp, startblock);
>
> trace_xfs_refcount_deferred(mp, XFS_FSB_TO_AGNO(mp, startblock),
> type, XFS_FSB_TO_AGBNO(mp, startblock),
> blockcount);
>
> - if (XFS_TEST_ERROR(false, mp,
> - XFS_ERRTAG_REFCOUNT_FINISH_ONE))
> - return -EIO;
> + if (XFS_TEST_ERROR(false, mp, XFS_ERRTAG_REFCOUNT_FINISH_ONE)) {
> + error = -EIO;
> + goto out_drop;
> + }
>
> /*
> * If we haven't gotten a cursor or the cursor AG doesn't match
> * the startblock, get one now.
> */
> rcur = *pcur;
> - if (rcur != NULL && rcur->bc_ag.agno != agno) {
> + if (rcur != NULL && rcur->bc_ag.pag != pag) {
> nr_ops = rcur->bc_ag.refc.nr_ops;
> shape_changes = rcur->bc_ag.refc.shape_changes;
> xfs_refcount_finish_one_cleanup(tp, rcur, 0);
> @@ -1173,12 +1174,12 @@ xfs_refcount_finish_one(
> *pcur = NULL;
> }
> if (rcur == NULL) {
> - error = xfs_alloc_read_agf(tp->t_mountp, tp, agno,
> + error = xfs_alloc_read_agf(tp->t_mountp, tp, pag->pag_agno,
> XFS_ALLOC_FLAG_FREEING, &agbp);
> if (error)
> - return error;
> + goto out_drop;
>
> - rcur = xfs_refcountbt_init_cursor(mp, tp, agbp, agno, NULL);
> + rcur = xfs_refcountbt_init_cursor(mp, tp, agbp, pag);
> rcur->bc_ag.refc.nr_ops = nr_ops;
> rcur->bc_ag.refc.shape_changes = shape_changes;
> }
> @@ -1188,12 +1189,12 @@ xfs_refcount_finish_one(
> case XFS_REFCOUNT_INCREASE:
> error = xfs_refcount_adjust(rcur, bno, blockcount, &new_agbno,
> new_len, XFS_REFCOUNT_ADJUST_INCREASE, NULL);
> - *new_fsb = XFS_AGB_TO_FSB(mp, agno, new_agbno);
> + *new_fsb = XFS_AGB_TO_FSB(mp, pag->pag_agno, new_agbno);
> break;
> case XFS_REFCOUNT_DECREASE:
> error = xfs_refcount_adjust(rcur, bno, blockcount, &new_agbno,
> new_len, XFS_REFCOUNT_ADJUST_DECREASE, NULL);
> - *new_fsb = XFS_AGB_TO_FSB(mp, agno, new_agbno);
> + *new_fsb = XFS_AGB_TO_FSB(mp, pag->pag_agno, new_agbno);
> break;
> case XFS_REFCOUNT_ALLOC_COW:
> *new_fsb = startblock + blockcount;
> @@ -1210,8 +1211,10 @@ xfs_refcount_finish_one(
> error = -EFSCORRUPTED;
> }
> if (!error && *new_len > 0)
> - trace_xfs_refcount_finish_one_leftover(mp, agno, type,
> + trace_xfs_refcount_finish_one_leftover(mp, pag->pag_agno, type,
> bno, blockcount, new_agbno, *new_len);
> +out_drop:
> + xfs_perag_put(pag);
> return error;
> }
>
> @@ -1672,7 +1675,7 @@ xfs_refcount_recover_extent(
> int
> xfs_refcount_recover_cow_leftovers(
> struct xfs_mount *mp,
> - xfs_agnumber_t agno)
> + struct xfs_perag *pag)
> {
> struct xfs_trans *tp;
> struct xfs_btree_cur *cur;
> @@ -1704,10 +1707,10 @@ xfs_refcount_recover_cow_leftovers(
> if (error)
> return error;
>
> - error = xfs_alloc_read_agf(mp, tp, agno, 0, &agbp);
> + error = xfs_alloc_read_agf(mp, tp, pag->pag_agno, 0, &agbp);
> if (error)
> goto out_trans;
> - cur = xfs_refcountbt_init_cursor(mp, tp, agbp, agno, NULL);
> + cur = xfs_refcountbt_init_cursor(mp, tp, agbp, pag);
>
> /* Find all the leftover CoW staging extents. */
> memset(&low, 0, sizeof(low));
> @@ -1729,11 +1732,12 @@ xfs_refcount_recover_cow_leftovers(
> if (error)
> goto out_free;
>
> - trace_xfs_refcount_recover_extent(mp, agno, &rr->rr_rrec);
> + trace_xfs_refcount_recover_extent(mp, pag->pag_agno,
> + &rr->rr_rrec);
>
> /* Free the orphan record */
> agbno = rr->rr_rrec.rc_startblock - XFS_REFC_COW_START;
> - fsb = XFS_AGB_TO_FSB(mp, agno, agbno);
> + fsb = XFS_AGB_TO_FSB(mp, pag->pag_agno, agbno);
> xfs_refcount_free_cow_extent(tp, fsb,
> rr->rr_rrec.rc_blockcount);
>
> diff --git a/fs/xfs/libxfs/xfs_refcount.h b/fs/xfs/libxfs/xfs_refcount.h
> index 209795539c8d..9f6e9aae4da0 100644
> --- a/fs/xfs/libxfs/xfs_refcount.h
> +++ b/fs/xfs/libxfs/xfs_refcount.h
> @@ -6,6 +6,13 @@
> #ifndef __XFS_REFCOUNT_H__
> #define __XFS_REFCOUNT_H__
>
> +struct xfs_trans;
> +struct xfs_mount;
> +struct xfs_perag;
> +struct xfs_btree_cur;
> +struct xfs_bmbt_irec;
> +struct xfs_refcount_irec;
> +
> extern int xfs_refcount_lookup_le(struct xfs_btree_cur *cur,
> xfs_agblock_t bno, int *stat);
> extern int xfs_refcount_lookup_ge(struct xfs_btree_cur *cur,
> @@ -50,7 +57,7 @@ void xfs_refcount_alloc_cow_extent(struct xfs_trans *tp, xfs_fsblock_t fsb,
> void xfs_refcount_free_cow_extent(struct xfs_trans *tp, xfs_fsblock_t fsb,
> xfs_extlen_t len);
> extern int xfs_refcount_recover_cow_leftovers(struct xfs_mount *mp,
> - xfs_agnumber_t agno);
> + struct xfs_perag *pag);
>
> /*
> * While we're adjusting the refcounts records of an extent, we have
> diff --git a/fs/xfs/libxfs/xfs_refcount_btree.c b/fs/xfs/libxfs/xfs_refcount_btree.c
> index 74f8ac0209f1..8f6577cb3475 100644
> --- a/fs/xfs/libxfs/xfs_refcount_btree.c
> +++ b/fs/xfs/libxfs/xfs_refcount_btree.c
> @@ -26,7 +26,7 @@ xfs_refcountbt_dup_cursor(
> struct xfs_btree_cur *cur)
> {
> return xfs_refcountbt_init_cursor(cur->bc_mp, cur->bc_tp,
> - cur->bc_ag.agbp, cur->bc_ag.agno, cur->bc_ag.pag);
> + cur->bc_ag.agbp, cur->bc_ag.pag);
> }
>
> STATIC void
> @@ -316,13 +316,11 @@ static struct xfs_btree_cur *
> xfs_refcountbt_init_common(
> struct xfs_mount *mp,
> struct xfs_trans *tp,
> - xfs_agnumber_t agno,
> struct xfs_perag *pag)
> {
> struct xfs_btree_cur *cur;
>
> - ASSERT(agno != NULLAGNUMBER);
> - ASSERT(agno < mp->m_sb.sb_agcount);
> + ASSERT(pag->pag_agno < mp->m_sb.sb_agcount);
When would this not be true? It's fine if you want to keep this as a
sanity check, but I thought perag structures trivially satisfied this
relation?
Either way,
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
>
> cur = kmem_cache_zalloc(xfs_btree_cur_zone, GFP_NOFS | __GFP_NOFAIL);
> cur->bc_tp = tp;
> @@ -331,13 +329,12 @@ xfs_refcountbt_init_common(
> cur->bc_blocklog = mp->m_sb.sb_blocklog;
> cur->bc_statoff = XFS_STATS_CALC_INDEX(xs_refcbt_2);
>
> - cur->bc_ag.agno = agno;
> cur->bc_flags |= XFS_BTREE_CRC_BLOCKS;
> - if (pag) {
> - /* take a reference for the cursor */
> - atomic_inc(&pag->pag_ref);
> - }
> +
> + /* take a reference for the cursor */
> + atomic_inc(&pag->pag_ref);
> cur->bc_ag.pag = pag;
> + cur->bc_ag.agno = pag->pag_agno;
>
> cur->bc_ag.refc.nr_ops = 0;
> cur->bc_ag.refc.shape_changes = 0;
> @@ -351,13 +348,12 @@ xfs_refcountbt_init_cursor(
> struct xfs_mount *mp,
> struct xfs_trans *tp,
> struct xfs_buf *agbp,
> - xfs_agnumber_t agno,
> struct xfs_perag *pag)
> {
> struct xfs_agf *agf = agbp->b_addr;
> struct xfs_btree_cur *cur;
>
> - cur = xfs_refcountbt_init_common(mp, tp, agno, pag);
> + cur = xfs_refcountbt_init_common(mp, tp, pag);
> cur->bc_nlevels = be32_to_cpu(agf->agf_refcount_level);
> cur->bc_ag.agbp = agbp;
> return cur;
> @@ -368,11 +364,11 @@ struct xfs_btree_cur *
> xfs_refcountbt_stage_cursor(
> struct xfs_mount *mp,
> struct xbtree_afakeroot *afake,
> - xfs_agnumber_t agno)
> + struct xfs_perag *pag)
> {
> struct xfs_btree_cur *cur;
>
> - cur = xfs_refcountbt_init_common(mp, NULL, agno, NULL);
> + cur = xfs_refcountbt_init_common(mp, NULL, pag);
> xfs_btree_stage_afakeroot(cur, afake);
> return cur;
> }
> diff --git a/fs/xfs/libxfs/xfs_refcount_btree.h b/fs/xfs/libxfs/xfs_refcount_btree.h
> index 8b82a39f104a..bd9ed9e1e41f 100644
> --- a/fs/xfs/libxfs/xfs_refcount_btree.h
> +++ b/fs/xfs/libxfs/xfs_refcount_btree.h
> @@ -47,9 +47,9 @@ struct xbtree_afakeroot;
>
> extern struct xfs_btree_cur *xfs_refcountbt_init_cursor(struct xfs_mount *mp,
> struct xfs_trans *tp, struct xfs_buf *agbp,
> - xfs_agnumber_t agno, struct xfs_perag *pag);
> + struct xfs_perag *pag);
> struct xfs_btree_cur *xfs_refcountbt_stage_cursor(struct xfs_mount *mp,
> - struct xbtree_afakeroot *afake, xfs_agnumber_t agno);
> + struct xbtree_afakeroot *afake, struct xfs_perag *pag);
> extern int xfs_refcountbt_maxrecs(int blocklen, bool leaf);
> extern void xfs_refcountbt_compute_maxlevels(struct xfs_mount *mp);
>
> diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
> index 981c689e3d95..251410c19198 100644
> --- a/fs/xfs/scrub/agheader_repair.c
> +++ b/fs/xfs/scrub/agheader_repair.c
> @@ -282,7 +282,7 @@ xrep_agf_calc_from_btrees(
> /* Update the AGF counters from the refcountbt. */
> if (xfs_sb_version_hasreflink(&mp->m_sb)) {
> cur = xfs_refcountbt_init_cursor(mp, sc->tp, agf_bp,
> - sc->sa.agno, sc->sa.pag);
> + sc->sa.pag);
> error = xfs_btree_count_blocks(cur, &blocks);
> if (error)
> goto err;
> diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c
> index dbe7b65f8da1..864c107666d5 100644
> --- a/fs/xfs/scrub/bmap.c
> +++ b/fs/xfs/scrub/bmap.c
> @@ -545,18 +545,18 @@ STATIC int
> xchk_bmap_check_ag_rmaps(
> struct xfs_scrub *sc,
> int whichfork,
> - xfs_agnumber_t agno)
> + struct xfs_perag *pag)
> {
> struct xchk_bmap_check_rmap_info sbcri;
> struct xfs_btree_cur *cur;
> struct xfs_buf *agf;
> int error;
>
> - error = xfs_alloc_read_agf(sc->mp, sc->tp, agno, 0, &agf);
> + error = xfs_alloc_read_agf(sc->mp, sc->tp, pag->pag_agno, 0, &agf);
> if (error)
> return error;
>
> - cur = xfs_rmapbt_init_cursor(sc->mp, sc->tp, agf, sc->sa.pag);
> + cur = xfs_rmapbt_init_cursor(sc->mp, sc->tp, agf, pag);
>
> sbcri.sc = sc;
> sbcri.whichfork = whichfork;
> @@ -610,7 +610,7 @@ xchk_bmap_check_rmaps(
> return 0;
>
> for_each_perag(sc->mp, agno, pag) {
> - error = xchk_bmap_check_ag_rmaps(sc, whichfork, pag->pag_agno);
> + error = xchk_bmap_check_ag_rmaps(sc, whichfork, pag);
> if (error)
> break;
> if (sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
> diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c
> index 48381c1adeed..cc7688ce79b2 100644
> --- a/fs/xfs/scrub/common.c
> +++ b/fs/xfs/scrub/common.c
> @@ -500,7 +500,7 @@ xchk_ag_btcur_init(
> if (sa->agf_bp && xfs_sb_version_hasreflink(&mp->m_sb) &&
> xchk_ag_btree_healthy_enough(sc, sa->pag, XFS_BTNUM_REFC)) {
> sa->refc_cur = xfs_refcountbt_init_cursor(mp, sc->tp,
> - sa->agf_bp, agno, sa->pag);
> + sa->agf_bp, sa->pag);
> }
> }
>
> diff --git a/fs/xfs/xfs_fsmap.c b/fs/xfs/xfs_fsmap.c
> index 7bfe9ea35de0..623cabaeafee 100644
> --- a/fs/xfs/xfs_fsmap.c
> +++ b/fs/xfs/xfs_fsmap.c
> @@ -210,8 +210,7 @@ xfs_getfsmap_is_shared(
>
> /* Are there any shared blocks here? */
> flen = 0;
> - cur = xfs_refcountbt_init_cursor(mp, tp, info->agf_bp,
> - info->pag->pag_agno, info->pag);
> + cur = xfs_refcountbt_init_cursor(mp, tp, info->agf_bp, info->pag);
>
> error = xfs_refcount_find_shared(cur, rec->rm_startblock,
> rec->rm_blockcount, &fbno, &flen, false);
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 28ffe1817f9b..c256104772cb 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -144,7 +144,7 @@ xfs_reflink_find_shared(
> if (error)
> return error;
>
> - cur = xfs_refcountbt_init_cursor(mp, tp, agbp, agno, NULL);
> + cur = xfs_refcountbt_init_cursor(mp, tp, agbp, agbp->b_pag);
>
> error = xfs_refcount_find_shared(cur, agbno, aglen, fbno, flen,
> find_end_of_shared);
> @@ -763,7 +763,7 @@ xfs_reflink_recover_cow(
> return 0;
>
> for_each_perag(mp, agno, pag) {
> - error = xfs_refcount_recover_cow_leftovers(mp, pag->pag_agno);
> + error = xfs_refcount_recover_cow_leftovers(mp, pag);
> if (error) {
> xfs_perag_put(pag);
> break;
> --
> 2.31.1
>
