On Wed, May 12, 2021 at 08:49:28AM -0400, Brian Foster wrote: > On Wed, May 12, 2021 at 07:52:50AM +1000, Dave Chinner wrote: > > 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@xxxxxxxxxx> > > > > > > > > > > Which will eventually completely replace the agno in it. > > > > > > > > > > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx> > > > > > --- > > > > > 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"... Ok. Can you add a comment to xfs_btree.h somewhere noting that LONG_PTRS is what we use to switch betwen bc_ag and bc_ino, please? --D > > > > Got it, thanks. The bc_ag field is unioned with bc_ino, the latter of > which is used by XFS_BTREE_ROOT_IN_INODE trees to track inode > information rather than AG information. I think the flag usage just > threw me off at a glance. With that cleared up: > > Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx> > > > 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@xxxxxxxxxxxxx > > >