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:

quoted

On Tue, May 11, 2021 at 08:30:22AM -0400, Brian Foster wrote:

quoted

On Thu, May 06, 2021 at 05:20:43PM +1000, Dave Chinner wrote:

quoted

From: Dave Chinner <redacted>

Which will eventually completely replace the agno in it.

Signed-off-by: Dave Chinner <redacted>
---
 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(-)

...

quoted

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.

quoted

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"...

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 <redacted>

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