Re: [PATCH 11/22] xfs: add a perag to the btree cursor

[Dave Chinner <david@fromorbit.com>]

On Tue, May 11, 2021 at 01:51:52PM -0700, Darrick J. Wong wrote:
> On Tue, May 11, 2021 at 08:30:22AM -0400, Brian Foster wrote:
> > On Thu, May 06, 2021 at 05:20:43PM +1000, Dave Chinner wrote:
> > > From: Dave Chinner <dchinner@redhat.com>
> > > 
> > > Which will eventually completely replace the agno in it.
> > > 
> > > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > > ---
> > >  fs/xfs/libxfs/xfs_alloc.c          | 25 +++++++++++++++----------
> > >  fs/xfs/libxfs/xfs_alloc_btree.c    | 13 ++++++++++---
> > >  fs/xfs/libxfs/xfs_alloc_btree.h    |  3 ++-
> > >  fs/xfs/libxfs/xfs_btree.c          |  2 ++
> > >  fs/xfs/libxfs/xfs_btree.h          |  4 +++-
> > >  fs/xfs/libxfs/xfs_ialloc.c         | 16 ++++++++--------
> > >  fs/xfs/libxfs/xfs_ialloc_btree.c   | 15 +++++++++++----
> > >  fs/xfs/libxfs/xfs_ialloc_btree.h   |  7 ++++---
> > >  fs/xfs/libxfs/xfs_refcount.c       |  4 ++--
> > >  fs/xfs/libxfs/xfs_refcount_btree.c | 17 ++++++++++++-----
> > >  fs/xfs/libxfs/xfs_refcount_btree.h |  2 +-
> > >  fs/xfs/libxfs/xfs_rmap.c           |  6 +++---
> > >  fs/xfs/libxfs/xfs_rmap_btree.c     | 17 ++++++++++++-----
> > >  fs/xfs/libxfs/xfs_rmap_btree.h     |  2 +-
> > >  fs/xfs/scrub/agheader_repair.c     | 20 +++++++++++---------
> > >  fs/xfs/scrub/bmap.c                |  2 +-
> > >  fs/xfs/scrub/common.c              | 12 ++++++------
> > >  fs/xfs/scrub/repair.c              |  5 +++--
> > >  fs/xfs/xfs_discard.c               |  2 +-
> > >  fs/xfs/xfs_fsmap.c                 |  6 +++---
> > >  fs/xfs/xfs_reflink.c               |  2 +-
> > >  21 files changed, 112 insertions(+), 70 deletions(-)
> > > 
> > ...
> > > diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
> > > index 0f12b885600d..44044317c0fb 100644
> > > --- a/fs/xfs/libxfs/xfs_btree.c
> > > +++ b/fs/xfs/libxfs/xfs_btree.c
> > > @@ -377,6 +377,8 @@ xfs_btree_del_cursor(
> > >  	       XFS_FORCED_SHUTDOWN(cur->bc_mp));
> > >  	if (unlikely(cur->bc_flags & XFS_BTREE_STAGING))
> > >  		kmem_free(cur->bc_ops);
> > > +	if (!(cur->bc_flags & XFS_BTREE_LONG_PTRS) && cur->bc_ag.pag)
> > > +		xfs_perag_put(cur->bc_ag.pag);
> > 
> > What's the correlation with BTREE_LONG_PTRS?

Only the btrees that index agbnos within a specific AG use the
cur->bc_ag structure to store the agno the btree is rooted in.
These are all btrees that use short pointers.

IOWs, we need an agno to turn the agbno into a full fsbno, daddr,
inum or anything else with global scope. Translation of short
pointers to physical location is necessary just to walk the tree,
while long pointer trees already record physical location of the
blocks within the tree and hence do not need an agno for
translation.

Hence needing the agno is specific, at this point in
time, to a btree containing short pointers.

> maybe this should be:
> 
> 	if (!(cur->bc_flags & XFS_BTREE_ROOT_IN_INODE))
> 		xfs_perag_put(cur->bc_ag.pag);

Given that the only long pointer btree we have is also the only
btree we have rooted in an inode, this is just another way of saying
!BTREE_LONG_PTRS. But "root in inode" is less obvious, because then we
lose the context taht "short pointers need translation via agno to
calculate their physical location in the wider filesystem"...

If we are going to put short ptrs in an inode, then at that point
we need to change the btree cursor, anyway, because then we are
going to need both an inode pointer and something else to turn those
short pointers into global scope pointers for IO....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com