diff mbox series

[4/5] xfs: factor out a xfs_btree_owner helper

Message ID 20240103203836.608391-5-hch@lst.de (mailing list archive)
State Superseded
Headers show
Series [1/5] xfs: remove the in-memory btree header block | expand

Commit Message

hch@lst.de Jan. 3, 2024, 8:38 p.m. UTC 
Split out a helper to calculate the owner for a given btree instead
of dulicating the logic in two places.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_btree.c     | 52 +++++++++++++++--------------------
 fs/xfs/libxfs/xfs_btree_mem.h |  5 ----
 fs/xfs/scrub/xfbtree.c        | 29 -------------------
 3 files changed, 22 insertions(+), 64 deletions(-)

Comments

Darrick J. Wong Jan. 4, 2024, 1:14 a.m. UTC | #1 
On Wed, Jan 03, 2024 at 09:38:35PM +0100, Christoph Hellwig wrote:
> Split out a helper to calculate the owner for a given btree instead
> of dulicating the logic in two places.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_btree.c     | 52 +++++++++++++++--------------------
>  fs/xfs/libxfs/xfs_btree_mem.h |  5 ----
>  fs/xfs/scrub/xfbtree.c        | 29 -------------------
>  3 files changed, 22 insertions(+), 64 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
> index 3bc8aa6049b9a7..bd51c428f66780 100644
> --- a/fs/xfs/libxfs/xfs_btree.c
> +++ b/fs/xfs/libxfs/xfs_btree.c
> @@ -1298,6 +1298,19 @@ xfs_btree_init_buf(
>  	bp->b_ops = ops->buf_ops;
>  }
>  
> +static uint64_t
> +xfs_btree_owner(
> +	struct xfs_btree_cur	*cur)
> +{
> +#ifdef CONFIG_XFS_BTREE_IN_XFILE
> +	if (cur->bc_flags & XFS_BTREE_IN_XFILE)
> +		return cur->bc_mem.xfbtree->owner;
> +#endif

Hrm.  I guess I never /did/ use xfbtree_owner except for this one file.

> +	if (cur->bc_flags & XFS_BTREE_LONG_PTRS)
> +		return cur->bc_ino.ip->i_ino;
> +	return cur->bc_ag.pag->pag_agno;
> +}
> +
>  void
>  xfs_btree_init_block_cur(
>  	struct xfs_btree_cur	*cur,
> @@ -1305,22 +1318,8 @@ xfs_btree_init_block_cur(
>  	int			level,
>  	int			numrecs)
>  {
> -	__u64			owner;
> -
> -	/*
> -	 * we can pull the owner from the cursor right now as the different
> -	 * owners align directly with the pointer size of the btree. This may
> -	 * change in future, but is safe for current users of the generic btree
> -	 * code.
> -	 */
> -	if (cur->bc_flags & XFS_BTREE_IN_XFILE)
> -		owner = xfbtree_owner(cur);
> -	else if (cur->bc_flags & XFS_BTREE_LONG_PTRS)
> -		owner = cur->bc_ino.ip->i_ino;
> -	else
> -		owner = cur->bc_ag.pag->pag_agno;
> -
> -	xfs_btree_init_buf(cur->bc_mp, bp, cur->bc_ops, level, numrecs, owner);
> +	xfs_btree_init_buf(cur->bc_mp, bp, cur->bc_ops, level, numrecs,
> +			xfs_btree_owner(cur));
>  }
>  
>  /*
> @@ -1875,25 +1874,18 @@ xfs_btree_check_block_owner(
>  	struct xfs_btree_cur	*cur,
>  	struct xfs_btree_block	*block)
>  {
> -	if (!xfs_has_crc(cur->bc_mp))
> +	if (!xfs_has_crc(cur->bc_mp) ||

I wonder, shouldn't this be (bc_flags & XFS_BTREE_CRC_BLOCKS) and not
xfs_has_crc?  They're one and the same, but as the geometry flags are
all getting moved to xfs_btree_ops, we ought to be consistent about what
we check.

--D

> +	    (cur->bc_flags & XFS_BTREE_BMBT_INVALID_OWNER))
>  		return NULL;
>  
> -	if (cur->bc_flags & XFS_BTREE_IN_XFILE)
> -		return xfbtree_check_block_owner(cur, block);
> -
> -	if (!(cur->bc_flags & XFS_BTREE_LONG_PTRS)) {
> -		if (be32_to_cpu(block->bb_u.s.bb_owner) !=
> -						cur->bc_ag.pag->pag_agno)
> +	if (cur->bc_flags & XFS_BTREE_LONG_PTRS) {
> +		if (be64_to_cpu(block->bb_u.l.bb_owner) != xfs_btree_owner(cur))
> +			return __this_address;
> +	} else {
> +		if (be32_to_cpu(block->bb_u.s.bb_owner) != xfs_btree_owner(cur))
>  			return __this_address;
> -		return NULL;
>  	}
>  
> -	if (cur->bc_flags & XFS_BTREE_BMBT_INVALID_OWNER)
> -		return NULL;
> -
> -	if (be64_to_cpu(block->bb_u.l.bb_owner) != cur->bc_ino.ip->i_ino)
> -		return __this_address;
> -
>  	return NULL;
>  }
>  
> diff --git a/fs/xfs/libxfs/xfs_btree_mem.h b/fs/xfs/libxfs/xfs_btree_mem.h
> index eeb3340a22d201..3a5492c2cc26b6 100644
> --- a/fs/xfs/libxfs/xfs_btree_mem.h
> +++ b/fs/xfs/libxfs/xfs_btree_mem.h
> @@ -43,9 +43,6 @@ void xfbtree_init_ptr_from_cur(struct xfs_btree_cur *cur,
>  struct xfs_btree_cur *xfbtree_dup_cursor(struct xfs_btree_cur *cur);
>  bool xfbtree_verify_xfileoff(struct xfs_btree_cur *cur,
>  		unsigned long long xfoff);
> -xfs_failaddr_t xfbtree_check_block_owner(struct xfs_btree_cur *cur,
> -		struct xfs_btree_block *block);
> -unsigned long long xfbtree_owner(struct xfs_btree_cur *cur);
>  xfs_failaddr_t xfbtree_lblock_verify(struct xfs_buf *bp, unsigned int max_recs);
>  xfs_failaddr_t xfbtree_sblock_verify(struct xfs_buf *bp, unsigned int max_recs);
>  unsigned long long xfbtree_buf_to_xfoff(struct xfs_btree_cur *cur,
> @@ -102,8 +99,6 @@ static inline unsigned int xfbtree_bbsize(void)
>  #define xfbtree_alloc_block			NULL
>  #define xfbtree_free_block			NULL
>  #define xfbtree_verify_xfileoff(cur, xfoff)	(false)
> -#define xfbtree_check_block_owner(cur, block)	NULL
> -#define xfbtree_owner(cur)			(0ULL)
>  #define xfbtree_buf_to_xfoff(cur, bp)		(-1)
>  
>  static inline int
> diff --git a/fs/xfs/scrub/xfbtree.c b/fs/xfs/scrub/xfbtree.c
> index 63b69aeadc623d..11dad651508067 100644
> --- a/fs/xfs/scrub/xfbtree.c
> +++ b/fs/xfs/scrub/xfbtree.c
> @@ -165,35 +165,6 @@ xfbtree_dup_cursor(
>  	return ncur;
>  }
>  
> -/* Check the owner of an in-memory btree block. */
> -xfs_failaddr_t
> -xfbtree_check_block_owner(
> -	struct xfs_btree_cur	*cur,
> -	struct xfs_btree_block	*block)
> -{
> -	struct xfbtree		*xfbt = cur->bc_mem.xfbtree;
> -
> -	if (cur->bc_flags & XFS_BTREE_LONG_PTRS) {
> -		if (be64_to_cpu(block->bb_u.l.bb_owner) != xfbt->owner)
> -			return __this_address;
> -
> -		return NULL;
> -	}
> -
> -	if (be32_to_cpu(block->bb_u.s.bb_owner) != xfbt->owner)
> -		return __this_address;
> -
> -	return NULL;
> -}
> -
> -/* Return the owner of this in-memory btree. */
> -unsigned long long
> -xfbtree_owner(
> -	struct xfs_btree_cur	*cur)
> -{
> -	return cur->bc_mem.xfbtree->owner;
> -}
> -
>  /* Return the xfile offset (in blocks) of a btree buffer. */
>  unsigned long long
>  xfbtree_buf_to_xfoff(
> -- 
> 2.39.2
> 
>
hch@lst.de Jan. 4, 2024, 6:28 a.m. UTC | #2 
On Wed, Jan 03, 2024 at 05:14:00PM -0800, Darrick J. Wong wrote:
> > @@ -1875,25 +1874,18 @@ xfs_btree_check_block_owner(
> >  	struct xfs_btree_cur	*cur,
> >  	struct xfs_btree_block	*block)
> >  {
> > -	if (!xfs_has_crc(cur->bc_mp))
> > +	if (!xfs_has_crc(cur->bc_mp) ||
> 
> I wonder, shouldn't this be (bc_flags & XFS_BTREE_CRC_BLOCKS) and not
> xfs_has_crc?  They're one and the same, but as the geometry flags are
> all getting moved to xfs_btree_ops, we ought to be consistent about what
> we check.

Sure.
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index 3bc8aa6049b9a7..bd51c428f66780 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -1298,6 +1298,19 @@  xfs_btree_init_buf(
 	bp->b_ops = ops->buf_ops;
 }
 
+static uint64_t
+xfs_btree_owner(
+	struct xfs_btree_cur	*cur)
+{
+#ifdef CONFIG_XFS_BTREE_IN_XFILE
+	if (cur->bc_flags & XFS_BTREE_IN_XFILE)
+		return cur->bc_mem.xfbtree->owner;
+#endif
+	if (cur->bc_flags & XFS_BTREE_LONG_PTRS)
+		return cur->bc_ino.ip->i_ino;
+	return cur->bc_ag.pag->pag_agno;
+}
+
 void
 xfs_btree_init_block_cur(
 	struct xfs_btree_cur	*cur,
@@ -1305,22 +1318,8 @@  xfs_btree_init_block_cur(
 	int			level,
 	int			numrecs)
 {
-	__u64			owner;
-
-	/*
-	 * we can pull the owner from the cursor right now as the different
-	 * owners align directly with the pointer size of the btree. This may
-	 * change in future, but is safe for current users of the generic btree
-	 * code.
-	 */
-	if (cur->bc_flags & XFS_BTREE_IN_XFILE)
-		owner = xfbtree_owner(cur);
-	else if (cur->bc_flags & XFS_BTREE_LONG_PTRS)
-		owner = cur->bc_ino.ip->i_ino;
-	else
-		owner = cur->bc_ag.pag->pag_agno;
-
-	xfs_btree_init_buf(cur->bc_mp, bp, cur->bc_ops, level, numrecs, owner);
+	xfs_btree_init_buf(cur->bc_mp, bp, cur->bc_ops, level, numrecs,
+			xfs_btree_owner(cur));
 }
 
 /*
@@ -1875,25 +1874,18 @@  xfs_btree_check_block_owner(
 	struct xfs_btree_cur	*cur,
 	struct xfs_btree_block	*block)
 {
-	if (!xfs_has_crc(cur->bc_mp))
+	if (!xfs_has_crc(cur->bc_mp) ||
+	    (cur->bc_flags & XFS_BTREE_BMBT_INVALID_OWNER))
 		return NULL;
 
-	if (cur->bc_flags & XFS_BTREE_IN_XFILE)
-		return xfbtree_check_block_owner(cur, block);
-
-	if (!(cur->bc_flags & XFS_BTREE_LONG_PTRS)) {
-		if (be32_to_cpu(block->bb_u.s.bb_owner) !=
-						cur->bc_ag.pag->pag_agno)
+	if (cur->bc_flags & XFS_BTREE_LONG_PTRS) {
+		if (be64_to_cpu(block->bb_u.l.bb_owner) != xfs_btree_owner(cur))
+			return __this_address;
+	} else {
+		if (be32_to_cpu(block->bb_u.s.bb_owner) != xfs_btree_owner(cur))
 			return __this_address;
-		return NULL;
 	}
 
-	if (cur->bc_flags & XFS_BTREE_BMBT_INVALID_OWNER)
-		return NULL;
-
-	if (be64_to_cpu(block->bb_u.l.bb_owner) != cur->bc_ino.ip->i_ino)
-		return __this_address;
-
 	return NULL;
 }
 
diff --git a/fs/xfs/libxfs/xfs_btree_mem.h b/fs/xfs/libxfs/xfs_btree_mem.h
index eeb3340a22d201..3a5492c2cc26b6 100644
--- a/fs/xfs/libxfs/xfs_btree_mem.h
+++ b/fs/xfs/libxfs/xfs_btree_mem.h
@@ -43,9 +43,6 @@  void xfbtree_init_ptr_from_cur(struct xfs_btree_cur *cur,
 struct xfs_btree_cur *xfbtree_dup_cursor(struct xfs_btree_cur *cur);
 bool xfbtree_verify_xfileoff(struct xfs_btree_cur *cur,
 		unsigned long long xfoff);
-xfs_failaddr_t xfbtree_check_block_owner(struct xfs_btree_cur *cur,
-		struct xfs_btree_block *block);
-unsigned long long xfbtree_owner(struct xfs_btree_cur *cur);
 xfs_failaddr_t xfbtree_lblock_verify(struct xfs_buf *bp, unsigned int max_recs);
 xfs_failaddr_t xfbtree_sblock_verify(struct xfs_buf *bp, unsigned int max_recs);
 unsigned long long xfbtree_buf_to_xfoff(struct xfs_btree_cur *cur,
@@ -102,8 +99,6 @@  static inline unsigned int xfbtree_bbsize(void)
 #define xfbtree_alloc_block			NULL
 #define xfbtree_free_block			NULL
 #define xfbtree_verify_xfileoff(cur, xfoff)	(false)
-#define xfbtree_check_block_owner(cur, block)	NULL
-#define xfbtree_owner(cur)			(0ULL)
 #define xfbtree_buf_to_xfoff(cur, bp)		(-1)
 
 static inline int
diff --git a/fs/xfs/scrub/xfbtree.c b/fs/xfs/scrub/xfbtree.c
index 63b69aeadc623d..11dad651508067 100644
--- a/fs/xfs/scrub/xfbtree.c
+++ b/fs/xfs/scrub/xfbtree.c
@@ -165,35 +165,6 @@  xfbtree_dup_cursor(
 	return ncur;
 }
 
-/* Check the owner of an in-memory btree block. */
-xfs_failaddr_t
-xfbtree_check_block_owner(
-	struct xfs_btree_cur	*cur,
-	struct xfs_btree_block	*block)
-{
-	struct xfbtree		*xfbt = cur->bc_mem.xfbtree;
-
-	if (cur->bc_flags & XFS_BTREE_LONG_PTRS) {
-		if (be64_to_cpu(block->bb_u.l.bb_owner) != xfbt->owner)
-			return __this_address;
-
-		return NULL;
-	}
-
-	if (be32_to_cpu(block->bb_u.s.bb_owner) != xfbt->owner)
-		return __this_address;
-
-	return NULL;
-}
-
-/* Return the owner of this in-memory btree. */
-unsigned long long
-xfbtree_owner(
-	struct xfs_btree_cur	*cur)
-{
-	return cur->bc_mem.xfbtree->owner;
-}
-
 /* Return the xfile offset (in blocks) of a btree buffer. */
 unsigned long long
 xfbtree_buf_to_xfoff(