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

[Brian Foster <bfoster@redhat.com>]

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
>