Re: [PATCH 16/22] xfs: remove agno from btree cursor

Brian Foster
2021-05-11T12:34:54Z

On Thu, May 06, 2021 at 05:20:48PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Now that everything passes a perag, the agno is not needed anymore.
> Convert all the users to use pag->pag_agno instead and remove the
> agno from the cursor. This was largely done as an automated search
> and replace.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
> fs/xfs/libxfs/xfs_alloc.c | 2 +-
> fs/xfs/libxfs/xfs_alloc_btree.c | 1 -
> fs/xfs/libxfs/xfs_btree.c | 12 ++--
> fs/xfs/libxfs/xfs_btree.h | 1 -
> fs/xfs/libxfs/xfs_ialloc.c | 2 +-
> fs/xfs/libxfs/xfs_ialloc_btree.c | 7 +-
> fs/xfs/libxfs/xfs_refcount.c | 82 +++++++++++-----------
> fs/xfs/libxfs/xfs_refcount_btree.c | 11 ++-
> fs/xfs/libxfs/xfs_rmap.c | 108 ++++++++++++++---------------
> fs/xfs/libxfs/xfs_rmap_btree.c | 1 -
> fs/xfs/scrub/agheader_repair.c | 2 +-
> fs/xfs/scrub/alloc.c | 3 +-
> fs/xfs/scrub/bmap.c | 2 +-
> fs/xfs/scrub/ialloc.c | 9 +--
> fs/xfs/scrub/refcount.c | 3 +-
> fs/xfs/scrub/rmap.c | 3 +-
> fs/xfs/scrub/trace.c | 3 +-
> fs/xfs/xfs_fsmap.c | 4 +-
> fs/xfs/xfs_trace.h | 4 +-
> 19 files changed, 130 insertions(+), 130 deletions(-)
>
...
> diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c
> index b23f949ee15c..d1dfad0204e3 100644
> --- a/fs/xfs/libxfs/xfs_rmap.c
> +++ b/fs/xfs/libxfs/xfs_rmap.c
...
> @@ -2389,7 +2389,7 @@ xfs_rmap_finish_one(
> * the startblock, get one now.
> */
> rcur = *pcur;
> - if (rcur != NULL && rcur->bc_ag.agno != pag->pag_agno) {
> + if (rcur != NULL && rcur->bc_ag.pag != pag) {

I wonder a bit about this sort of logic if the goal is to ultimately
allow for dynamic instantiation of perag structures, though it's
probably not an issue here.

> xfs_rmap_finish_one_cleanup(tp, rcur, 0);
> rcur = NULL;
> *pcur = NULL;
...
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 808ae337b222..5ba9c6396dcb 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -3730,7 +3730,7 @@ TRACE_EVENT(xfs_btree_commit_afakeroot,
> TP_fast_assign(
> __entry->dev = cur->bc_mp->m_super->s_dev;
> __entry->btnum = cur->bc_btnum;
> - __entry->agno = cur->bc_ag.agno;
> + __entry->agno = cur->bc_ag.pag->pag_agno;

It would be nice if we did this with some of the other tracepoints
rather than pulling ->pag_agno out at every callsite, but that's
probably something for another patch. All in all this looks fine to me:

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

> __entry->agbno = cur->bc_ag.afake->af_root;
> __entry->levels = cur->bc_ag.afake->af_levels;
> __entry->blocks = cur->bc_ag.afake->af_blocks;
> @@ -3845,7 +3845,7 @@ TRACE_EVENT(xfs_btree_bload_block,
> __entry->agno = XFS_FSB_TO_AGNO(cur->bc_mp, fsb);
> __entry->agbno = XFS_FSB_TO_AGBNO(cur->bc_mp, fsb);
> } else {
> - __entry->agno = cur->bc_ag.agno;
> + __entry->agno = cur->bc_ag.pag->pag_agno;
> __entry->agbno = be32_to_cpu(ptr->s);
> }
> __entry->nr_records = nr_records;
> --
> 2.31.1
>