[PATCH 0/3] xfs: clean up buffer cache disk addressing

[PATCH 0/3] xfs: clean up buffer cache disk addressing

[Dave Chinner]

Over time, we've moved from using bp->b_bn as the physical disk
address for a buffer to using the address of the first buffer map
attached to the buffer. This can be seen by the implementation of
XFS_BUF_ADDR() macro:

#define XFS_BUF_ADDR(bp) ((bp)->b_maps[0].bm_bn)

The bp->b_bn value is now used as the buffer cache index for the
buffer, and it is always set to XFS_BUF_DADDR_NULL for uncached
buffers. Hence code that uses bp->b_bn for the physical address of
the buffer will not do the right thing when passed an uncached
buffer.

This series of patches addresses this problem. It adds a helper
function xfs_buf_daddr() to extract the physical address of the
buffer from it, and replaces all the open coded bp->b_bn accesses
and the XFS_BUF_ADDR() users to use it. It then renames b_bn to
b_index and converts all the internal cache lookup code to use
b_index rather than b_bn.

The result is that all code now uses xfs_buf_daddr() where physical
addresses are required, and the cache index variable isn't named in
a manner that engenders external use of access. The changes end up
being relatively small, and the impact on userspace outside libxfs
is even smaller (only a couple of dozen references to b_bn or
XFS_BUF_ADDR).

Version 1
- based on 5.14-rc4 + for-next + "xfs: rework feature flags"

[PATCH 1/3] xfs: introduce xfs_buf_daddr()

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

Introduce a helper function xfs_buf_daddr() to extract the disk
address of the buffer from the struct xfs_buf. This will replace
direct accesses to bp->b_bn and bp->b_maps[0].bm_bn, as well as
the XFS_BUF_ADDR() macro.

This patch introduces the helper function and replaces all uses of
XFS_BUF_ADDR() as this is just a simple sed replacement.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/libxfs/xfs_alloc_btree.c    |  2 +-
 fs/xfs/libxfs/xfs_attr.c           |  4 ++--
 fs/xfs/libxfs/xfs_bmap.c           |  4 ++--
 fs/xfs/libxfs/xfs_bmap_btree.c     |  2 +-
 fs/xfs/libxfs/xfs_btree.c          | 10 +++++-----
 fs/xfs/libxfs/xfs_ialloc_btree.c   |  2 +-
 fs/xfs/libxfs/xfs_inode_buf.c      |  2 +-
 fs/xfs/libxfs/xfs_refcount_btree.c |  2 +-
 fs/xfs/libxfs/xfs_rmap_btree.c     |  2 +-
 fs/xfs/libxfs/xfs_sb.c             |  2 +-
 fs/xfs/scrub/btree.c               |  4 ++--
 fs/xfs/xfs_attr_inactive.c         |  2 +-
 fs/xfs/xfs_buf.c                   |  2 +-
 fs/xfs/xfs_buf.h                   |  6 +++++-
 fs/xfs/xfs_trans_buf.c             |  2 +-
 15 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_alloc_btree.c b/fs/xfs/libxfs/xfs_alloc_btree.c
index 81a8e1d0cd90..7f69387609d3 100644
--- a/fs/xfs/libxfs/xfs_alloc_btree.c
+++ b/fs/xfs/libxfs/xfs_alloc_btree.c
@@ -87,7 +87,7 @@ xfs_allocbt_free_block(
 	xfs_agblock_t		bno;
 	int			error;
 
-	bno = xfs_daddr_to_agbno(cur->bc_mp, XFS_BUF_ADDR(bp));
+	bno = xfs_daddr_to_agbno(cur->bc_mp, xfs_buf_daddr(bp));
 	error = xfs_alloc_put_freelist(cur->bc_tp, agbp, NULL, bno, 1);
 	if (error)
 		return error;
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 58fac02aad2a..270973d4a4ac 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -1541,7 +1541,7 @@ xfs_attr_fillstate(xfs_da_state_t *state)
 	ASSERT((path->active >= 0) && (path->active < XFS_DA_NODE_MAXDEPTH));
 	for (blk = path->blk, level = 0; level < path->active; blk++, level++) {
 		if (blk->bp) {
-			blk->disk_blkno = XFS_BUF_ADDR(blk->bp);
+			blk->disk_blkno = xfs_buf_daddr(blk->bp);
 			blk->bp = NULL;
 		} else {
 			blk->disk_blkno = 0;
@@ -1556,7 +1556,7 @@ xfs_attr_fillstate(xfs_da_state_t *state)
 	ASSERT((path->active >= 0) && (path->active < XFS_DA_NODE_MAXDEPTH));
 	for (blk = path->blk, level = 0; level < path->active; blk++, level++) {
 		if (blk->bp) {
-			blk->disk_blkno = XFS_BUF_ADDR(blk->bp);
+			blk->disk_blkno = xfs_buf_daddr(blk->bp);
 			blk->bp = NULL;
 		} else {
 			blk->disk_blkno = 0;
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 75354023cea7..d0bfa9a1f549 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -242,7 +242,7 @@ xfs_bmap_get_bp(
 	for (i = 0; i < XFS_BTREE_MAXLEVELS; i++) {
 		if (!cur->bc_bufs[i])
 			break;
-		if (XFS_BUF_ADDR(cur->bc_bufs[i]) == bno)
+		if (xfs_buf_daddr(cur->bc_bufs[i]) == bno)
 			return cur->bc_bufs[i];
 	}
 
@@ -251,7 +251,7 @@ xfs_bmap_get_bp(
 		struct xfs_buf_log_item	*bip = (struct xfs_buf_log_item *)lip;
 
 		if (bip->bli_item.li_type == XFS_LI_BUF &&
-		    XFS_BUF_ADDR(bip->bli_buf) == bno)
+		    xfs_buf_daddr(bip->bli_buf) == bno)
 			return bip->bli_buf;
 	}
 
diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c
index c04b2a697b29..e0ae95c7b379 100644
--- a/fs/xfs/libxfs/xfs_bmap_btree.c
+++ b/fs/xfs/libxfs/xfs_bmap_btree.c
@@ -282,7 +282,7 @@ xfs_bmbt_free_block(
 	struct xfs_mount	*mp = cur->bc_mp;
 	struct xfs_inode	*ip = cur->bc_ino.ip;
 	struct xfs_trans	*tp = cur->bc_tp;
-	xfs_fsblock_t		fsbno = XFS_DADDR_TO_FSB(mp, XFS_BUF_ADDR(bp));
+	xfs_fsblock_t		fsbno = XFS_DADDR_TO_FSB(mp, xfs_buf_daddr(bp));
 	struct xfs_owner_info	oinfo;
 
 	xfs_rmap_ino_bmbt_owner(&oinfo, ip->i_ino, cur->bc_ino.whichfork);
diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index 393c9438d5a7..dc704c2b22cb 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -420,7 +420,7 @@ xfs_btree_dup_cursor(
 		bp = cur->bc_bufs[i];
 		if (bp) {
 			error = xfs_trans_read_buf(mp, tp, mp->m_ddev_targp,
-						   XFS_BUF_ADDR(bp), mp->m_bsize,
+						   xfs_buf_daddr(bp), mp->m_bsize,
 						   0, &bp,
 						   cur->bc_ops->buf_ops);
 			if (error) {
@@ -1192,10 +1192,10 @@ xfs_btree_buf_to_ptr(
 {
 	if (cur->bc_flags & XFS_BTREE_LONG_PTRS)
 		ptr->l = cpu_to_be64(XFS_DADDR_TO_FSB(cur->bc_mp,
-					XFS_BUF_ADDR(bp)));
+					xfs_buf_daddr(bp)));
 	else {
 		ptr->s = cpu_to_be32(xfs_daddr_to_agbno(cur->bc_mp,
-					XFS_BUF_ADDR(bp)));
+					xfs_buf_daddr(bp)));
 	}
 }
 
@@ -1739,7 +1739,7 @@ xfs_btree_lookup_get_block(
 	error = xfs_btree_ptr_to_daddr(cur, pp, &daddr);
 	if (error)
 		return error;
-	if (bp && XFS_BUF_ADDR(bp) == daddr) {
+	if (bp && xfs_buf_daddr(bp) == daddr) {
 		*blkp = XFS_BUF_TO_BLOCK(bp);
 		return 0;
 	}
@@ -4499,7 +4499,7 @@ xfs_btree_sblock_verify(
 		return __this_address;
 
 	/* sibling pointer verification */
-	agno = xfs_daddr_to_agno(mp, XFS_BUF_ADDR(bp));
+	agno = xfs_daddr_to_agno(mp, xfs_buf_daddr(bp));
 	if (block->bb_u.s.bb_leftsib != cpu_to_be32(NULLAGBLOCK) &&
 	    !xfs_verify_agbno(mp, agno, be32_to_cpu(block->bb_u.s.bb_leftsib)))
 		return __this_address;
diff --git a/fs/xfs/libxfs/xfs_ialloc_btree.c b/fs/xfs/libxfs/xfs_ialloc_btree.c
index f1384c280059..65a94741ce16 100644
--- a/fs/xfs/libxfs/xfs_ialloc_btree.c
+++ b/fs/xfs/libxfs/xfs_ialloc_btree.c
@@ -156,7 +156,7 @@ __xfs_inobt_free_block(
 {
 	xfs_inobt_mod_blockcount(cur, -1);
 	return xfs_free_extent(cur->bc_tp,
-			XFS_DADDR_TO_FSB(cur->bc_mp, XFS_BUF_ADDR(bp)), 1,
+			XFS_DADDR_TO_FSB(cur->bc_mp, xfs_buf_daddr(bp)), 1,
 			&XFS_RMAP_OINFO_INOBT, resv);
 }
 
diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index 036f909ff7a6..83ba63b4ace4 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -48,7 +48,7 @@ xfs_inode_buf_verify(
 	/*
 	 * Validate the magic number and version of every inode in the buffer
 	 */
-	agno = xfs_daddr_to_agno(mp, XFS_BUF_ADDR(bp));
+	agno = xfs_daddr_to_agno(mp, xfs_buf_daddr(bp));
 	ni = XFS_BB_TO_FSB(mp, bp->b_length) * mp->m_sb.sb_inopblock;
 	for (i = 0; i < ni; i++) {
 		int		di_ok;
diff --git a/fs/xfs/libxfs/xfs_refcount_btree.c b/fs/xfs/libxfs/xfs_refcount_btree.c
index 31ce9d2d45e1..fd47dc934dae 100644
--- a/fs/xfs/libxfs/xfs_refcount_btree.c
+++ b/fs/xfs/libxfs/xfs_refcount_btree.c
@@ -102,7 +102,7 @@ xfs_refcountbt_free_block(
 	struct xfs_mount	*mp = cur->bc_mp;
 	struct xfs_buf		*agbp = cur->bc_ag.agbp;
 	struct xfs_agf		*agf = agbp->b_addr;
-	xfs_fsblock_t		fsbno = XFS_DADDR_TO_FSB(mp, XFS_BUF_ADDR(bp));
+	xfs_fsblock_t		fsbno = XFS_DADDR_TO_FSB(mp, xfs_buf_daddr(bp));
 	int			error;
 
 	trace_xfs_refcountbt_free_block(cur->bc_mp, cur->bc_ag.pag->pag_agno,
diff --git a/fs/xfs/libxfs/xfs_rmap_btree.c b/fs/xfs/libxfs/xfs_rmap_btree.c
index 921651fb4b1b..f0fe7aa09a43 100644
--- a/fs/xfs/libxfs/xfs_rmap_btree.c
+++ b/fs/xfs/libxfs/xfs_rmap_btree.c
@@ -122,7 +122,7 @@ xfs_rmapbt_free_block(
 	xfs_agblock_t		bno;
 	int			error;
 
-	bno = xfs_daddr_to_agbno(cur->bc_mp, XFS_BUF_ADDR(bp));
+	bno = xfs_daddr_to_agbno(cur->bc_mp, xfs_buf_daddr(bp));
 	trace_xfs_rmapbt_free_block(cur->bc_mp, pag->pag_agno,
 			bno, 1);
 	be32_add_cpu(&agf->agf_rmap_blocks, -1);
diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index 197093acb828..b1c5ec1bd200 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -192,7 +192,7 @@ xfs_validate_sb_write(
 	 * secondary superblocks, so allow this usage to continue because
 	 * we never read counters from such superblocks.
 	 */
-	if (XFS_BUF_ADDR(bp) == XFS_SB_DADDR && !sbp->sb_inprogress &&
+	if (xfs_buf_daddr(bp) == XFS_SB_DADDR && !sbp->sb_inprogress &&
 	    (sbp->sb_fdblocks > sbp->sb_dblocks ||
 	     !xfs_verify_icount(mp, sbp->sb_icount) ||
 	     sbp->sb_ifree > sbp->sb_icount)) {
diff --git a/fs/xfs/scrub/btree.c b/fs/xfs/scrub/btree.c
index c044e0a8da7f..fd832f103fa4 100644
--- a/fs/xfs/scrub/btree.c
+++ b/fs/xfs/scrub/btree.c
@@ -435,12 +435,12 @@ xchk_btree_check_owner(
 		if (!co)
 			return -ENOMEM;
 		co->level = level;
-		co->daddr = XFS_BUF_ADDR(bp);
+		co->daddr = xfs_buf_daddr(bp);
 		list_add_tail(&co->list, &bs->to_check);
 		return 0;
 	}
 
-	return xchk_btree_check_block_owner(bs, level, XFS_BUF_ADDR(bp));
+	return xchk_btree_check_block_owner(bs, level, xfs_buf_daddr(bp));
 }
 
 /* Decide if we want to check minrecs of a btree block in the inode root. */
diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c
index aaa7e66c42d7..d8fdde206867 100644
--- a/fs/xfs/xfs_attr_inactive.c
+++ b/fs/xfs/xfs_attr_inactive.c
@@ -177,7 +177,7 @@ xfs_attr3_node_inactive(
 			return error;
 
 		/* save for re-read later */
-		child_blkno = XFS_BUF_ADDR(child_bp);
+		child_blkno = xfs_buf_daddr(child_bp);
 
 		/*
 		 * Invalidate the subtree, however we have to.
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 82dd9bfa4265..c1bb6e41595b 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -1335,7 +1335,7 @@ xfs_buf_ioerror_alert(
 {
 	xfs_buf_alert_ratelimited(bp, "XFS: metadata IO error",
 		"metadata I/O error in \"%pS\" at daddr 0x%llx len %d error %d",
-				  func, (uint64_t)XFS_BUF_ADDR(bp),
+				  func, (uint64_t)xfs_buf_daddr(bp),
 				  bp->b_length, -bp->b_error);
 }
 
diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index 37c9004f11de..6db2fba44b46 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -311,9 +311,13 @@ extern void xfs_buf_terminate(void);
  * In future, uncached buffers will pass the block number directly to the io
  * request function and hence these macros will go away at that point.
  */
-#define XFS_BUF_ADDR(bp)		((bp)->b_maps[0].bm_bn)
 #define XFS_BUF_SET_ADDR(bp, bno)	((bp)->b_maps[0].bm_bn = (xfs_daddr_t)(bno))
 
+static inline xfs_daddr_t xfs_buf_daddr(struct xfs_buf *bp)
+{
+	return bp->b_maps[0].bm_bn;
+}
+
 void xfs_buf_set_ref(struct xfs_buf *bp, int lru_ref);
 
 /*
diff --git a/fs/xfs/xfs_trans_buf.c b/fs/xfs/xfs_trans_buf.c
index 4ff274ce31c4..6549e50d852c 100644
--- a/fs/xfs/xfs_trans_buf.c
+++ b/fs/xfs/xfs_trans_buf.c
@@ -38,7 +38,7 @@ xfs_trans_buf_item_match(
 		blip = (struct xfs_buf_log_item *)lip;
 		if (blip->bli_item.li_type == XFS_LI_BUF &&
 		    blip->bli_buf->b_target == target &&
-		    XFS_BUF_ADDR(blip->bli_buf) == map[0].bm_bn &&
+		    xfs_buf_daddr(blip->bli_buf) == map[0].bm_bn &&
 		    blip->bli_buf->b_length == len) {
 			ASSERT(blip->bli_buf->b_map_count == nmaps);
 			return blip->bli_buf;
-- 
2.31.1

Re: [PATCH 1/3] xfs: introduce xfs_buf_daddr()

[Darrick J. Wong]

On Tue, Aug 10, 2021 at 03:28:49PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Introduce a helper function xfs_buf_daddr() to extract the disk
> address of the buffer from the struct xfs_buf. This will replace
> direct accesses to bp->b_bn and bp->b_maps[0].bm_bn, as well as
> the XFS_BUF_ADDR() macro.
> 
> This patch introduces the helper function and replaces all uses of
> XFS_BUF_ADDR() as this is just a simple sed replacement.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

I like the new name!

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_alloc_btree.c    |  2 +-
>  fs/xfs/libxfs/xfs_attr.c           |  4 ++--
>  fs/xfs/libxfs/xfs_bmap.c           |  4 ++--
>  fs/xfs/libxfs/xfs_bmap_btree.c     |  2 +-
>  fs/xfs/libxfs/xfs_btree.c          | 10 +++++-----
>  fs/xfs/libxfs/xfs_ialloc_btree.c   |  2 +-
>  fs/xfs/libxfs/xfs_inode_buf.c      |  2 +-
>  fs/xfs/libxfs/xfs_refcount_btree.c |  2 +-
>  fs/xfs/libxfs/xfs_rmap_btree.c     |  2 +-
>  fs/xfs/libxfs/xfs_sb.c             |  2 +-
>  fs/xfs/scrub/btree.c               |  4 ++--
>  fs/xfs/xfs_attr_inactive.c         |  2 +-
>  fs/xfs/xfs_buf.c                   |  2 +-
>  fs/xfs/xfs_buf.h                   |  6 +++++-
>  fs/xfs/xfs_trans_buf.c             |  2 +-
>  15 files changed, 26 insertions(+), 22 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_alloc_btree.c b/fs/xfs/libxfs/xfs_alloc_btree.c
> index 81a8e1d0cd90..7f69387609d3 100644
> --- a/fs/xfs/libxfs/xfs_alloc_btree.c
> +++ b/fs/xfs/libxfs/xfs_alloc_btree.c
> @@ -87,7 +87,7 @@ xfs_allocbt_free_block(
>  	xfs_agblock_t		bno;
>  	int			error;
>  
> -	bno = xfs_daddr_to_agbno(cur->bc_mp, XFS_BUF_ADDR(bp));
> +	bno = xfs_daddr_to_agbno(cur->bc_mp, xfs_buf_daddr(bp));
>  	error = xfs_alloc_put_freelist(cur->bc_tp, agbp, NULL, bno, 1);
>  	if (error)
>  		return error;
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 58fac02aad2a..270973d4a4ac 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -1541,7 +1541,7 @@ xfs_attr_fillstate(xfs_da_state_t *state)
>  	ASSERT((path->active >= 0) && (path->active < XFS_DA_NODE_MAXDEPTH));
>  	for (blk = path->blk, level = 0; level < path->active; blk++, level++) {
>  		if (blk->bp) {
> -			blk->disk_blkno = XFS_BUF_ADDR(blk->bp);
> +			blk->disk_blkno = xfs_buf_daddr(blk->bp);
>  			blk->bp = NULL;
>  		} else {
>  			blk->disk_blkno = 0;
> @@ -1556,7 +1556,7 @@ xfs_attr_fillstate(xfs_da_state_t *state)
>  	ASSERT((path->active >= 0) && (path->active < XFS_DA_NODE_MAXDEPTH));
>  	for (blk = path->blk, level = 0; level < path->active; blk++, level++) {
>  		if (blk->bp) {
> -			blk->disk_blkno = XFS_BUF_ADDR(blk->bp);
> +			blk->disk_blkno = xfs_buf_daddr(blk->bp);
>  			blk->bp = NULL;
>  		} else {
>  			blk->disk_blkno = 0;
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 75354023cea7..d0bfa9a1f549 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -242,7 +242,7 @@ xfs_bmap_get_bp(
>  	for (i = 0; i < XFS_BTREE_MAXLEVELS; i++) {
>  		if (!cur->bc_bufs[i])
>  			break;
> -		if (XFS_BUF_ADDR(cur->bc_bufs[i]) == bno)
> +		if (xfs_buf_daddr(cur->bc_bufs[i]) == bno)
>  			return cur->bc_bufs[i];
>  	}
>  
> @@ -251,7 +251,7 @@ xfs_bmap_get_bp(
>  		struct xfs_buf_log_item	*bip = (struct xfs_buf_log_item *)lip;
>  
>  		if (bip->bli_item.li_type == XFS_LI_BUF &&
> -		    XFS_BUF_ADDR(bip->bli_buf) == bno)
> +		    xfs_buf_daddr(bip->bli_buf) == bno)
>  			return bip->bli_buf;
>  	}
>  
> diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c
> index c04b2a697b29..e0ae95c7b379 100644
> --- a/fs/xfs/libxfs/xfs_bmap_btree.c
> +++ b/fs/xfs/libxfs/xfs_bmap_btree.c
> @@ -282,7 +282,7 @@ xfs_bmbt_free_block(
>  	struct xfs_mount	*mp = cur->bc_mp;
>  	struct xfs_inode	*ip = cur->bc_ino.ip;
>  	struct xfs_trans	*tp = cur->bc_tp;
> -	xfs_fsblock_t		fsbno = XFS_DADDR_TO_FSB(mp, XFS_BUF_ADDR(bp));
> +	xfs_fsblock_t		fsbno = XFS_DADDR_TO_FSB(mp, xfs_buf_daddr(bp));
>  	struct xfs_owner_info	oinfo;
>  
>  	xfs_rmap_ino_bmbt_owner(&oinfo, ip->i_ino, cur->bc_ino.whichfork);
> diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
> index 393c9438d5a7..dc704c2b22cb 100644
> --- a/fs/xfs/libxfs/xfs_btree.c
> +++ b/fs/xfs/libxfs/xfs_btree.c
> @@ -420,7 +420,7 @@ xfs_btree_dup_cursor(
>  		bp = cur->bc_bufs[i];
>  		if (bp) {
>  			error = xfs_trans_read_buf(mp, tp, mp->m_ddev_targp,
> -						   XFS_BUF_ADDR(bp), mp->m_bsize,
> +						   xfs_buf_daddr(bp), mp->m_bsize,
>  						   0, &bp,
>  						   cur->bc_ops->buf_ops);
>  			if (error) {
> @@ -1192,10 +1192,10 @@ xfs_btree_buf_to_ptr(
>  {
>  	if (cur->bc_flags & XFS_BTREE_LONG_PTRS)
>  		ptr->l = cpu_to_be64(XFS_DADDR_TO_FSB(cur->bc_mp,
> -					XFS_BUF_ADDR(bp)));
> +					xfs_buf_daddr(bp)));
>  	else {
>  		ptr->s = cpu_to_be32(xfs_daddr_to_agbno(cur->bc_mp,
> -					XFS_BUF_ADDR(bp)));
> +					xfs_buf_daddr(bp)));
>  	}
>  }
>  
> @@ -1739,7 +1739,7 @@ xfs_btree_lookup_get_block(
>  	error = xfs_btree_ptr_to_daddr(cur, pp, &daddr);
>  	if (error)
>  		return error;
> -	if (bp && XFS_BUF_ADDR(bp) == daddr) {
> +	if (bp && xfs_buf_daddr(bp) == daddr) {
>  		*blkp = XFS_BUF_TO_BLOCK(bp);
>  		return 0;
>  	}
> @@ -4499,7 +4499,7 @@ xfs_btree_sblock_verify(
>  		return __this_address;
>  
>  	/* sibling pointer verification */
> -	agno = xfs_daddr_to_agno(mp, XFS_BUF_ADDR(bp));
> +	agno = xfs_daddr_to_agno(mp, xfs_buf_daddr(bp));
>  	if (block->bb_u.s.bb_leftsib != cpu_to_be32(NULLAGBLOCK) &&
>  	    !xfs_verify_agbno(mp, agno, be32_to_cpu(block->bb_u.s.bb_leftsib)))
>  		return __this_address;
> diff --git a/fs/xfs/libxfs/xfs_ialloc_btree.c b/fs/xfs/libxfs/xfs_ialloc_btree.c
> index f1384c280059..65a94741ce16 100644
> --- a/fs/xfs/libxfs/xfs_ialloc_btree.c
> +++ b/fs/xfs/libxfs/xfs_ialloc_btree.c
> @@ -156,7 +156,7 @@ __xfs_inobt_free_block(
>  {
>  	xfs_inobt_mod_blockcount(cur, -1);
>  	return xfs_free_extent(cur->bc_tp,
> -			XFS_DADDR_TO_FSB(cur->bc_mp, XFS_BUF_ADDR(bp)), 1,
> +			XFS_DADDR_TO_FSB(cur->bc_mp, xfs_buf_daddr(bp)), 1,
>  			&XFS_RMAP_OINFO_INOBT, resv);
>  }
>  
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index 036f909ff7a6..83ba63b4ace4 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -48,7 +48,7 @@ xfs_inode_buf_verify(
>  	/*
>  	 * Validate the magic number and version of every inode in the buffer
>  	 */
> -	agno = xfs_daddr_to_agno(mp, XFS_BUF_ADDR(bp));
> +	agno = xfs_daddr_to_agno(mp, xfs_buf_daddr(bp));
>  	ni = XFS_BB_TO_FSB(mp, bp->b_length) * mp->m_sb.sb_inopblock;
>  	for (i = 0; i < ni; i++) {
>  		int		di_ok;
> diff --git a/fs/xfs/libxfs/xfs_refcount_btree.c b/fs/xfs/libxfs/xfs_refcount_btree.c
> index 31ce9d2d45e1..fd47dc934dae 100644
> --- a/fs/xfs/libxfs/xfs_refcount_btree.c
> +++ b/fs/xfs/libxfs/xfs_refcount_btree.c
> @@ -102,7 +102,7 @@ xfs_refcountbt_free_block(
>  	struct xfs_mount	*mp = cur->bc_mp;
>  	struct xfs_buf		*agbp = cur->bc_ag.agbp;
>  	struct xfs_agf		*agf = agbp->b_addr;
> -	xfs_fsblock_t		fsbno = XFS_DADDR_TO_FSB(mp, XFS_BUF_ADDR(bp));
> +	xfs_fsblock_t		fsbno = XFS_DADDR_TO_FSB(mp, xfs_buf_daddr(bp));
>  	int			error;
>  
>  	trace_xfs_refcountbt_free_block(cur->bc_mp, cur->bc_ag.pag->pag_agno,
> diff --git a/fs/xfs/libxfs/xfs_rmap_btree.c b/fs/xfs/libxfs/xfs_rmap_btree.c
> index 921651fb4b1b..f0fe7aa09a43 100644
> --- a/fs/xfs/libxfs/xfs_rmap_btree.c
> +++ b/fs/xfs/libxfs/xfs_rmap_btree.c
> @@ -122,7 +122,7 @@ xfs_rmapbt_free_block(
>  	xfs_agblock_t		bno;
>  	int			error;
>  
> -	bno = xfs_daddr_to_agbno(cur->bc_mp, XFS_BUF_ADDR(bp));
> +	bno = xfs_daddr_to_agbno(cur->bc_mp, xfs_buf_daddr(bp));
>  	trace_xfs_rmapbt_free_block(cur->bc_mp, pag->pag_agno,
>  			bno, 1);
>  	be32_add_cpu(&agf->agf_rmap_blocks, -1);
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index 197093acb828..b1c5ec1bd200 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -192,7 +192,7 @@ xfs_validate_sb_write(
>  	 * secondary superblocks, so allow this usage to continue because
>  	 * we never read counters from such superblocks.
>  	 */
> -	if (XFS_BUF_ADDR(bp) == XFS_SB_DADDR && !sbp->sb_inprogress &&
> +	if (xfs_buf_daddr(bp) == XFS_SB_DADDR && !sbp->sb_inprogress &&
>  	    (sbp->sb_fdblocks > sbp->sb_dblocks ||
>  	     !xfs_verify_icount(mp, sbp->sb_icount) ||
>  	     sbp->sb_ifree > sbp->sb_icount)) {
> diff --git a/fs/xfs/scrub/btree.c b/fs/xfs/scrub/btree.c
> index c044e0a8da7f..fd832f103fa4 100644
> --- a/fs/xfs/scrub/btree.c
> +++ b/fs/xfs/scrub/btree.c
> @@ -435,12 +435,12 @@ xchk_btree_check_owner(
>  		if (!co)
>  			return -ENOMEM;
>  		co->level = level;
> -		co->daddr = XFS_BUF_ADDR(bp);
> +		co->daddr = xfs_buf_daddr(bp);
>  		list_add_tail(&co->list, &bs->to_check);
>  		return 0;
>  	}
>  
> -	return xchk_btree_check_block_owner(bs, level, XFS_BUF_ADDR(bp));
> +	return xchk_btree_check_block_owner(bs, level, xfs_buf_daddr(bp));
>  }
>  
>  /* Decide if we want to check minrecs of a btree block in the inode root. */
> diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c
> index aaa7e66c42d7..d8fdde206867 100644
> --- a/fs/xfs/xfs_attr_inactive.c
> +++ b/fs/xfs/xfs_attr_inactive.c
> @@ -177,7 +177,7 @@ xfs_attr3_node_inactive(
>  			return error;
>  
>  		/* save for re-read later */
> -		child_blkno = XFS_BUF_ADDR(child_bp);
> +		child_blkno = xfs_buf_daddr(child_bp);
>  
>  		/*
>  		 * Invalidate the subtree, however we have to.
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 82dd9bfa4265..c1bb6e41595b 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -1335,7 +1335,7 @@ xfs_buf_ioerror_alert(
>  {
>  	xfs_buf_alert_ratelimited(bp, "XFS: metadata IO error",
>  		"metadata I/O error in \"%pS\" at daddr 0x%llx len %d error %d",
> -				  func, (uint64_t)XFS_BUF_ADDR(bp),
> +				  func, (uint64_t)xfs_buf_daddr(bp),
>  				  bp->b_length, -bp->b_error);
>  }
>  
> diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
> index 37c9004f11de..6db2fba44b46 100644
> --- a/fs/xfs/xfs_buf.h
> +++ b/fs/xfs/xfs_buf.h
> @@ -311,9 +311,13 @@ extern void xfs_buf_terminate(void);
>   * In future, uncached buffers will pass the block number directly to the io
>   * request function and hence these macros will go away at that point.
>   */
> -#define XFS_BUF_ADDR(bp)		((bp)->b_maps[0].bm_bn)
>  #define XFS_BUF_SET_ADDR(bp, bno)	((bp)->b_maps[0].bm_bn = (xfs_daddr_t)(bno))
>  
> +static inline xfs_daddr_t xfs_buf_daddr(struct xfs_buf *bp)
> +{
> +	return bp->b_maps[0].bm_bn;
> +}
> +
>  void xfs_buf_set_ref(struct xfs_buf *bp, int lru_ref);
>  
>  /*
> diff --git a/fs/xfs/xfs_trans_buf.c b/fs/xfs/xfs_trans_buf.c
> index 4ff274ce31c4..6549e50d852c 100644
> --- a/fs/xfs/xfs_trans_buf.c
> +++ b/fs/xfs/xfs_trans_buf.c
> @@ -38,7 +38,7 @@ xfs_trans_buf_item_match(
>  		blip = (struct xfs_buf_log_item *)lip;
>  		if (blip->bli_item.li_type == XFS_LI_BUF &&
>  		    blip->bli_buf->b_target == target &&
> -		    XFS_BUF_ADDR(blip->bli_buf) == map[0].bm_bn &&
> +		    xfs_buf_daddr(blip->bli_buf) == map[0].bm_bn &&
>  		    blip->bli_buf->b_length == len) {
>  			ASSERT(blip->bli_buf->b_map_count == nmaps);
>  			return blip->bli_buf;
> -- 
> 2.31.1
> 

Re: [PATCH 1/3] xfs: introduce xfs_buf_daddr()

[Christoph Hellwig]

On Tue, Aug 10, 2021 at 03:28:49PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Introduce a helper function xfs_buf_daddr() to extract the disk
> address of the buffer from the struct xfs_buf. This will replace
> direct accesses to bp->b_bn and bp->b_maps[0].bm_bn, as well as
> the XFS_BUF_ADDR() macro.
> 
> This patch introduces the helper function and replaces all uses of
> XFS_BUF_ADDR() as this is just a simple sed replacement.

The end result looks sane, but I would have preferred to do one patch
that just does the script rename of XFS_BUF_ADDR, and one ore more to
clean up the rest.

Re: [PATCH 1/3] xfs: introduce xfs_buf_daddr()

[Christoph Hellwig]

On Thu, Aug 12, 2021 at 09:10:56AM +0100, Christoph Hellwig wrote:
> On Tue, Aug 10, 2021 at 03:28:49PM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > Introduce a helper function xfs_buf_daddr() to extract the disk
> > address of the buffer from the struct xfs_buf. This will replace
> > direct accesses to bp->b_bn and bp->b_maps[0].bm_bn, as well as
> > the XFS_BUF_ADDR() macro.
> > 
> > This patch introduces the helper function and replaces all uses of
> > XFS_BUF_ADDR() as this is just a simple sed replacement.
> 
> The end result looks sane, but I would have preferred to do one patch
> that just does the script rename of XFS_BUF_ADDR, and one ore more to
> clean up the rest.

Which is exactly what this patch does - I should have looked at it
more carefully and not just the applied series.

So maybe tweak the above commit message bit, otherwise:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 2/3] xfs: convert bp->b_bn references to xfs_buf_daddr()

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

Stop directly referencing b_bn in code outside the buffer cache, as
b_bn is supposed to be used only as an internal cache index.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/libxfs/xfs_ag.c             |  1 -
 fs/xfs/libxfs/xfs_attr_leaf.c      |  4 ++--
 fs/xfs/libxfs/xfs_attr_remote.c    |  8 ++++----
 fs/xfs/libxfs/xfs_bmap.c           |  2 +-
 fs/xfs/libxfs/xfs_btree.c          | 25 +++++++++++++------------
 fs/xfs/libxfs/xfs_da_btree.c       |  8 ++++----
 fs/xfs/libxfs/xfs_dir2_block.c     |  4 ++--
 fs/xfs/libxfs/xfs_dir2_data.c      |  4 ++--
 fs/xfs/libxfs/xfs_dir2_leaf.c      |  4 ++--
 fs/xfs/libxfs/xfs_dir2_node.c      |  6 +++---
 fs/xfs/libxfs/xfs_inode_buf.c      |  2 +-
 fs/xfs/libxfs/xfs_sb.c             |  2 +-
 fs/xfs/libxfs/xfs_symlink_remote.c |  4 ++--
 fs/xfs/scrub/bitmap.c              |  4 ++--
 fs/xfs/scrub/common.c              |  8 ++++----
 fs/xfs/scrub/trace.c               |  8 ++++----
 fs/xfs/xfs_attr_inactive.c         |  4 ++--
 fs/xfs/xfs_buf_item.c              |  2 +-
 fs/xfs/xfs_buf_item_recover.c      |  2 +-
 fs/xfs/xfs_error.c                 |  4 ++--
 fs/xfs/xfs_trace.h                 | 13 +++++--------
 21 files changed, 58 insertions(+), 61 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
index 92033c4672a4..005abfd9fd34 100644
--- a/fs/xfs/libxfs/xfs_ag.c
+++ b/fs/xfs/libxfs/xfs_ag.c
@@ -313,7 +313,6 @@ xfs_get_aghdr_buf(
 	if (error)
 		return error;
 
-	bp->b_bn = blkno;
 	bp->b_maps[0].bm_bn = blkno;
 	bp->b_ops = ops;
 
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index caaecb11df93..e893c96b5a56 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -1205,7 +1205,7 @@ xfs_attr3_leaf_to_node(
 	memcpy(bp2->b_addr, bp1->b_addr, args->geo->blksize);
 	if (xfs_has_crc(mp)) {
 		struct xfs_da3_blkinfo *hdr3 = bp2->b_addr;
-		hdr3->blkno = cpu_to_be64(bp2->b_bn);
+		hdr3->blkno = cpu_to_be64(xfs_buf_daddr(bp2));
 	}
 	xfs_trans_log_buf(args->trans, bp2, 0, args->geo->blksize - 1);
 
@@ -1273,7 +1273,7 @@ xfs_attr3_leaf_create(
 
 		ichdr.magic = XFS_ATTR3_LEAF_MAGIC;
 
-		hdr3->blkno = cpu_to_be64(bp->b_bn);
+		hdr3->blkno = cpu_to_be64(xfs_buf_daddr(bp));
 		hdr3->owner = cpu_to_be64(dp->i_ino);
 		uuid_copy(&hdr3->uuid, &mp->m_sb.sb_meta_uuid);
 
diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
index 8429395ad5f1..83b95be9ded8 100644
--- a/fs/xfs/libxfs/xfs_attr_remote.c
+++ b/fs/xfs/libxfs/xfs_attr_remote.c
@@ -130,7 +130,7 @@ __xfs_attr3_rmt_read_verify(
 		return 0;
 
 	ptr = bp->b_addr;
-	bno = bp->b_bn;
+	bno = xfs_buf_daddr(bp);
 	len = BBTOB(bp->b_length);
 	ASSERT(len >= blksize);
 
@@ -195,7 +195,7 @@ xfs_attr3_rmt_write_verify(
 		return;
 
 	ptr = bp->b_addr;
-	bno = bp->b_bn;
+	bno = xfs_buf_daddr(bp);
 	len = BBTOB(bp->b_length);
 	ASSERT(len >= blksize);
 
@@ -284,7 +284,7 @@ xfs_attr_rmtval_copyout(
 	uint8_t		**dst)
 {
 	char		*src = bp->b_addr;
-	xfs_daddr_t	bno = bp->b_bn;
+	xfs_daddr_t	bno = xfs_buf_daddr(bp);
 	int		len = BBTOB(bp->b_length);
 	int		blksize = mp->m_attr_geo->blksize;
 
@@ -332,7 +332,7 @@ xfs_attr_rmtval_copyin(
 	uint8_t		**src)
 {
 	char		*dst = bp->b_addr;
-	xfs_daddr_t	bno = bp->b_bn;
+	xfs_daddr_t	bno = xfs_buf_daddr(bp);
 	int		len = BBTOB(bp->b_length);
 	int		blksize = mp->m_attr_geo->blksize;
 
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index d0bfa9a1f549..b48230f1a361 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -739,7 +739,7 @@ xfs_bmap_extents_to_btree(
 	 */
 	abp->b_ops = &xfs_bmbt_buf_ops;
 	ablock = XFS_BUF_TO_BLOCK(abp);
-	xfs_btree_init_block_int(mp, ablock, abp->b_bn,
+	xfs_btree_init_block_int(mp, ablock, xfs_buf_daddr(abp),
 				XFS_BTNUM_BMAP, 0, 0, ip->i_ino,
 				XFS_BTREE_LONG_PTRS);
 
diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index dc704c2b22cb..00f631b1d4e9 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -70,7 +70,7 @@ __xfs_btree_check_lblock(
 		if (!uuid_equal(&block->bb_u.l.bb_uuid, &mp->m_sb.sb_meta_uuid))
 			return __this_address;
 		if (block->bb_u.l.bb_blkno !=
-		    cpu_to_be64(bp ? bp->b_bn : XFS_BUF_DADDR_NULL))
+		    cpu_to_be64(bp ? xfs_buf_daddr(bp) : XFS_BUF_DADDR_NULL))
 			return __this_address;
 		if (block->bb_u.l.bb_pad != cpu_to_be32(0))
 			return __this_address;
@@ -135,7 +135,7 @@ __xfs_btree_check_sblock(
 		if (!uuid_equal(&block->bb_u.s.bb_uuid, &mp->m_sb.sb_meta_uuid))
 			return __this_address;
 		if (block->bb_u.s.bb_blkno !=
-		    cpu_to_be64(bp ? bp->b_bn : XFS_BUF_DADDR_NULL))
+		    cpu_to_be64(bp ? xfs_buf_daddr(bp) : XFS_BUF_DADDR_NULL))
 			return __this_address;
 	}
 
@@ -1131,7 +1131,7 @@ xfs_btree_init_block(
 	__u16		numrecs,
 	__u64		owner)
 {
-	xfs_btree_init_block_int(mp, XFS_BUF_TO_BLOCK(bp), bp->b_bn,
+	xfs_btree_init_block_int(mp, XFS_BUF_TO_BLOCK(bp), xfs_buf_daddr(bp),
 				 btnum, level, numrecs, owner, 0);
 }
 
@@ -1155,9 +1155,9 @@ xfs_btree_init_block_cur(
 	else
 		owner = cur->bc_ag.pag->pag_agno;
 
-	xfs_btree_init_block_int(cur->bc_mp, XFS_BUF_TO_BLOCK(bp), bp->b_bn,
-				 cur->bc_btnum, level, numrecs,
-				 owner, cur->bc_flags);
+	xfs_btree_init_block_int(cur->bc_mp, XFS_BUF_TO_BLOCK(bp),
+				xfs_buf_daddr(bp), cur->bc_btnum, level,
+				numrecs, owner, cur->bc_flags);
 }
 
 /*
@@ -2923,10 +2923,11 @@ xfs_btree_new_iroot(
 	 */
 	memcpy(cblock, block, xfs_btree_block_len(cur));
 	if (cur->bc_flags & XFS_BTREE_CRC_BLOCKS) {
+		xfs_daddr_t bno = cpu_to_be64(xfs_buf_daddr(cbp));
 		if (cur->bc_flags & XFS_BTREE_LONG_PTRS)
-			cblock->bb_u.l.bb_blkno = cpu_to_be64(cbp->b_bn);
+			cblock->bb_u.l.bb_blkno = bno;
 		else
-			cblock->bb_u.s.bb_blkno = cpu_to_be64(cbp->b_bn);
+			cblock->bb_u.s.bb_blkno = bno;
 	}
 
 	be16_add_cpu(&block->bb_level, 1);
@@ -3225,7 +3226,7 @@ xfs_btree_insrec(
 
 	/* Get pointers to the btree buffer and block. */
 	block = xfs_btree_get_block(cur, level, &bp);
-	old_bn = bp ? bp->b_bn : XFS_BUF_DADDR_NULL;
+	old_bn = bp ? xfs_buf_daddr(bp) : XFS_BUF_DADDR_NULL;
 	numrecs = xfs_btree_get_numrecs(block);
 
 #ifdef DEBUG
@@ -3341,7 +3342,7 @@ xfs_btree_insrec(
 	 * some records into the new tree block), so use the regular key
 	 * update mechanism.
 	 */
-	if (bp && bp->b_bn != old_bn) {
+	if (bp && xfs_buf_daddr(bp) != old_bn) {
 		xfs_btree_get_keys(cur, block, lkey);
 	} else if (xfs_btree_needs_key_update(cur, optr)) {
 		error = xfs_btree_update_keys(cur, level);
@@ -4422,7 +4423,7 @@ xfs_btree_lblock_v5hdr_verify(
 		return __this_address;
 	if (!uuid_equal(&block->bb_u.l.bb_uuid, &mp->m_sb.sb_meta_uuid))
 		return __this_address;
-	if (block->bb_u.l.bb_blkno != cpu_to_be64(bp->b_bn))
+	if (block->bb_u.l.bb_blkno != cpu_to_be64(xfs_buf_daddr(bp)))
 		return __this_address;
 	if (owner != XFS_RMAP_OWN_UNKNOWN &&
 	    be64_to_cpu(block->bb_u.l.bb_owner) != owner)
@@ -4472,7 +4473,7 @@ xfs_btree_sblock_v5hdr_verify(
 		return __this_address;
 	if (!uuid_equal(&block->bb_u.s.bb_uuid, &mp->m_sb.sb_meta_uuid))
 		return __this_address;
-	if (block->bb_u.s.bb_blkno != cpu_to_be64(bp->b_bn))
+	if (block->bb_u.s.bb_blkno != cpu_to_be64(xfs_buf_daddr(bp)))
 		return __this_address;
 	if (pag && be32_to_cpu(block->bb_u.s.bb_owner) != pag->pag_agno)
 		return __this_address;
diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index 99f81f6bb306..c062e2c85178 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -194,7 +194,7 @@ xfs_da3_blkinfo_verify(
 	if (xfs_has_crc(mp)) {
 		if (!uuid_equal(&hdr3->uuid, &mp->m_sb.sb_meta_uuid))
 			return __this_address;
-		if (be64_to_cpu(hdr3->blkno) != bp->b_bn)
+		if (be64_to_cpu(hdr3->blkno) != xfs_buf_daddr(bp))
 			return __this_address;
 		if (!xfs_log_check_lsn(mp, be64_to_cpu(hdr3->lsn)))
 			return __this_address;
@@ -447,7 +447,7 @@ xfs_da3_node_create(
 
 		memset(hdr3, 0, sizeof(struct xfs_da3_node_hdr));
 		ichdr.magic = XFS_DA3_NODE_MAGIC;
-		hdr3->info.blkno = cpu_to_be64(bp->b_bn);
+		hdr3->info.blkno = cpu_to_be64(xfs_buf_daddr(bp));
 		hdr3->info.owner = cpu_to_be64(args->dp->i_ino);
 		uuid_copy(&hdr3->info.uuid, &mp->m_sb.sb_meta_uuid);
 	} else {
@@ -711,7 +711,7 @@ xfs_da3_root_split(
 	    oldroot->hdr.info.magic == cpu_to_be16(XFS_DIR3_LEAFN_MAGIC)) {
 		struct xfs_da3_intnode *node3 = (struct xfs_da3_intnode *)node;
 
-		node3->hdr.info.blkno = cpu_to_be64(bp->b_bn);
+		node3->hdr.info.blkno = cpu_to_be64(xfs_buf_daddr(bp));
 	}
 	xfs_trans_log_buf(tp, bp, 0, size - 1);
 
@@ -1219,7 +1219,7 @@ xfs_da3_root_join(
 	xfs_trans_buf_copy_type(root_blk->bp, bp);
 	if (oldroothdr.magic == XFS_DA3_NODE_MAGIC) {
 		struct xfs_da3_blkinfo *da3 = root_blk->bp->b_addr;
-		da3->blkno = cpu_to_be64(root_blk->bp->b_bn);
+		da3->blkno = cpu_to_be64(xfs_buf_daddr(root_blk->bp));
 	}
 	xfs_trans_log_buf(args->trans, root_blk->bp, 0,
 			  args->geo->blksize - 1);
diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
index 41e406067f91..df0869bba275 100644
--- a/fs/xfs/libxfs/xfs_dir2_block.c
+++ b/fs/xfs/libxfs/xfs_dir2_block.c
@@ -56,7 +56,7 @@ xfs_dir3_block_verify(
 	if (xfs_has_crc(mp)) {
 		if (!uuid_equal(&hdr3->uuid, &mp->m_sb.sb_meta_uuid))
 			return __this_address;
-		if (be64_to_cpu(hdr3->blkno) != bp->b_bn)
+		if (be64_to_cpu(hdr3->blkno) != xfs_buf_daddr(bp))
 			return __this_address;
 		if (!xfs_log_check_lsn(mp, be64_to_cpu(hdr3->lsn)))
 			return __this_address;
@@ -174,7 +174,7 @@ xfs_dir3_block_init(
 	if (xfs_has_crc(mp)) {
 		memset(hdr3, 0, sizeof(*hdr3));
 		hdr3->magic = cpu_to_be32(XFS_DIR3_BLOCK_MAGIC);
-		hdr3->blkno = cpu_to_be64(bp->b_bn);
+		hdr3->blkno = cpu_to_be64(xfs_buf_daddr(bp));
 		hdr3->owner = cpu_to_be64(dp->i_ino);
 		uuid_copy(&hdr3->uuid, &mp->m_sb.sb_meta_uuid);
 		return;
diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c
index c90180f2ba5c..dbcf58979a59 100644
--- a/fs/xfs/libxfs/xfs_dir2_data.c
+++ b/fs/xfs/libxfs/xfs_dir2_data.c
@@ -300,7 +300,7 @@ xfs_dir3_data_verify(
 	if (xfs_has_crc(mp)) {
 		if (!uuid_equal(&hdr3->uuid, &mp->m_sb.sb_meta_uuid))
 			return __this_address;
-		if (be64_to_cpu(hdr3->blkno) != bp->b_bn)
+		if (be64_to_cpu(hdr3->blkno) != xfs_buf_daddr(bp))
 			return __this_address;
 		if (!xfs_log_check_lsn(mp, be64_to_cpu(hdr3->lsn)))
 			return __this_address;
@@ -722,7 +722,7 @@ xfs_dir3_data_init(
 
 		memset(hdr3, 0, sizeof(*hdr3));
 		hdr3->magic = cpu_to_be32(XFS_DIR3_DATA_MAGIC);
-		hdr3->blkno = cpu_to_be64(bp->b_bn);
+		hdr3->blkno = cpu_to_be64(xfs_buf_daddr(bp));
 		hdr3->owner = cpu_to_be64(dp->i_ino);
 		uuid_copy(&hdr3->uuid, &mp->m_sb.sb_meta_uuid);
 
diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
index d03db9cde271..d9b66306a9a7 100644
--- a/fs/xfs/libxfs/xfs_dir2_leaf.c
+++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
@@ -108,7 +108,7 @@ xfs_dir3_leaf1_check(
 
 	if (leafhdr.magic == XFS_DIR3_LEAF1_MAGIC) {
 		struct xfs_dir3_leaf_hdr *leaf3 = bp->b_addr;
-		if (be64_to_cpu(leaf3->info.blkno) != bp->b_bn)
+		if (be64_to_cpu(leaf3->info.blkno) != xfs_buf_daddr(bp))
 			return __this_address;
 	} else if (leafhdr.magic != XFS_DIR2_LEAF1_MAGIC)
 		return __this_address;
@@ -316,7 +316,7 @@ xfs_dir3_leaf_init(
 		leaf3->info.hdr.magic = (type == XFS_DIR2_LEAF1_MAGIC)
 					 ? cpu_to_be16(XFS_DIR3_LEAF1_MAGIC)
 					 : cpu_to_be16(XFS_DIR3_LEAFN_MAGIC);
-		leaf3->info.blkno = cpu_to_be64(bp->b_bn);
+		leaf3->info.blkno = cpu_to_be64(xfs_buf_daddr(bp));
 		leaf3->info.owner = cpu_to_be64(owner);
 		uuid_copy(&leaf3->info.uuid, &mp->m_sb.sb_meta_uuid);
 	} else {
diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
index fbd2de8b3cf2..7a03aeb9f4c9 100644
--- a/fs/xfs/libxfs/xfs_dir2_node.c
+++ b/fs/xfs/libxfs/xfs_dir2_node.c
@@ -68,7 +68,7 @@ xfs_dir3_leafn_check(
 
 	if (leafhdr.magic == XFS_DIR3_LEAFN_MAGIC) {
 		struct xfs_dir3_leaf_hdr *leaf3 = bp->b_addr;
-		if (be64_to_cpu(leaf3->info.blkno) != bp->b_bn)
+		if (be64_to_cpu(leaf3->info.blkno) != xfs_buf_daddr(bp))
 			return __this_address;
 	} else if (leafhdr.magic != XFS_DIR2_LEAFN_MAGIC)
 		return __this_address;
@@ -110,7 +110,7 @@ xfs_dir3_free_verify(
 
 		if (!uuid_equal(&hdr3->uuid, &mp->m_sb.sb_meta_uuid))
 			return __this_address;
-		if (be64_to_cpu(hdr3->blkno) != bp->b_bn)
+		if (be64_to_cpu(hdr3->blkno) != xfs_buf_daddr(bp))
 			return __this_address;
 		if (!xfs_log_check_lsn(mp, be64_to_cpu(hdr3->lsn)))
 			return __this_address;
@@ -346,7 +346,7 @@ xfs_dir3_free_get_buf(
 
 		hdr.magic = XFS_DIR3_FREE_MAGIC;
 
-		hdr3->hdr.blkno = cpu_to_be64(bp->b_bn);
+		hdr3->hdr.blkno = cpu_to_be64(xfs_buf_daddr(bp));
 		hdr3->hdr.owner = cpu_to_be64(dp->i_ino);
 		uuid_copy(&hdr3->hdr.uuid, &mp->m_sb.sb_meta_uuid);
 	} else
diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
index 83ba63b4ace4..3932b4ebf903 100644
--- a/fs/xfs/libxfs/xfs_inode_buf.c
+++ b/fs/xfs/libxfs/xfs_inode_buf.c
@@ -71,7 +71,7 @@ xfs_inode_buf_verify(
 #ifdef DEBUG
 			xfs_alert(mp,
 				"bad inode magic/vsn daddr %lld #%d (magic=%x)",
-				(unsigned long long)bp->b_bn, i,
+				(unsigned long long)xfs_buf_daddr(bp), i,
 				be16_to_cpu(dip->di_magic));
 #endif
 			xfs_buf_verifier_error(bp, -EFSCORRUPTED,
diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index b1c5ec1bd200..e58349be78bd 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -747,7 +747,7 @@ xfs_sb_read_verify(
 
 		if (!xfs_buf_verify_cksum(bp, XFS_SB_CRC_OFF)) {
 			/* Only fail bad secondaries on a known V5 filesystem */
-			if (bp->b_maps[0].bm_bn == XFS_SB_DADDR ||
+			if (xfs_buf_daddr(bp) == XFS_SB_DADDR ||
 			    xfs_has_crc(mp)) {
 				error = -EFSBADCRC;
 				goto out_error;
diff --git a/fs/xfs/libxfs/xfs_symlink_remote.c b/fs/xfs/libxfs/xfs_symlink_remote.c
index 98b2b6804657..f0b38f4aba80 100644
--- a/fs/xfs/libxfs/xfs_symlink_remote.c
+++ b/fs/xfs/libxfs/xfs_symlink_remote.c
@@ -51,7 +51,7 @@ xfs_symlink_hdr_set(
 	dsl->sl_bytes = cpu_to_be32(size);
 	uuid_copy(&dsl->sl_uuid, &mp->m_sb.sb_meta_uuid);
 	dsl->sl_owner = cpu_to_be64(ino);
-	dsl->sl_blkno = cpu_to_be64(bp->b_bn);
+	dsl->sl_blkno = cpu_to_be64(xfs_buf_daddr(bp));
 	bp->b_ops = &xfs_symlink_buf_ops;
 
 	return sizeof(struct xfs_dsymlink_hdr);
@@ -95,7 +95,7 @@ xfs_symlink_verify(
 		return __this_address;
 	if (!uuid_equal(&dsl->sl_uuid, &mp->m_sb.sb_meta_uuid))
 		return __this_address;
-	if (bp->b_bn != be64_to_cpu(dsl->sl_blkno))
+	if (xfs_buf_daddr(bp) != be64_to_cpu(dsl->sl_blkno))
 		return __this_address;
 	if (be32_to_cpu(dsl->sl_offset) +
 				be32_to_cpu(dsl->sl_bytes) >= XFS_SYMLINK_MAXLEN)
diff --git a/fs/xfs/scrub/bitmap.c b/fs/xfs/scrub/bitmap.c
index 813b5f219113..d6d24c866bc4 100644
--- a/fs/xfs/scrub/bitmap.c
+++ b/fs/xfs/scrub/bitmap.c
@@ -260,7 +260,7 @@ xbitmap_set_btcur_path(
 		xfs_btree_get_block(cur, i, &bp);
 		if (!bp)
 			continue;
-		fsb = XFS_DADDR_TO_FSB(cur->bc_mp, bp->b_bn);
+		fsb = XFS_DADDR_TO_FSB(cur->bc_mp, xfs_buf_daddr(bp));
 		error = xbitmap_set(bitmap, fsb, 1);
 		if (error)
 			return error;
@@ -284,7 +284,7 @@ xbitmap_collect_btblock(
 	if (!bp)
 		return 0;
 
-	fsbno = XFS_DADDR_TO_FSB(cur->bc_mp, bp->b_bn);
+	fsbno = XFS_DADDR_TO_FSB(cur->bc_mp, xfs_buf_daddr(bp));
 	return xbitmap_set(bitmap, fsbno, 1);
 }
 
diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c
index 74a442bc7e14..38f8852d9837 100644
--- a/fs/xfs/scrub/common.c
+++ b/fs/xfs/scrub/common.c
@@ -186,7 +186,7 @@ xchk_block_set_preen(
 	struct xfs_buf		*bp)
 {
 	sc->sm->sm_flags |= XFS_SCRUB_OFLAG_PREEN;
-	trace_xchk_block_preen(sc, bp->b_bn, __return_address);
+	trace_xchk_block_preen(sc, xfs_buf_daddr(bp), __return_address);
 }
 
 /*
@@ -219,7 +219,7 @@ xchk_block_set_corrupt(
 	struct xfs_buf		*bp)
 {
 	sc->sm->sm_flags |= XFS_SCRUB_OFLAG_CORRUPT;
-	trace_xchk_block_error(sc, bp->b_bn, __return_address);
+	trace_xchk_block_error(sc, xfs_buf_daddr(bp), __return_address);
 }
 
 /* Record a corruption while cross-referencing. */
@@ -229,7 +229,7 @@ xchk_block_xref_set_corrupt(
 	struct xfs_buf		*bp)
 {
 	sc->sm->sm_flags |= XFS_SCRUB_OFLAG_XCORRUPT;
-	trace_xchk_block_error(sc, bp->b_bn, __return_address);
+	trace_xchk_block_error(sc, xfs_buf_daddr(bp), __return_address);
 }
 
 /*
@@ -787,7 +787,7 @@ xchk_buffer_recheck(
 	if (!fa)
 		return;
 	sc->sm->sm_flags |= XFS_SCRUB_OFLAG_CORRUPT;
-	trace_xchk_block_error(sc, bp->b_bn, fa);
+	trace_xchk_block_error(sc, xfs_buf_daddr(bp), fa);
 }
 
 /*
diff --git a/fs/xfs/scrub/trace.c b/fs/xfs/scrub/trace.c
index 03882a605a3c..c0ef53fe6611 100644
--- a/fs/xfs/scrub/trace.c
+++ b/fs/xfs/scrub/trace.c
@@ -22,11 +22,11 @@ xchk_btree_cur_fsbno(
 	int			level)
 {
 	if (level < cur->bc_nlevels && cur->bc_bufs[level])
-		return XFS_DADDR_TO_FSB(cur->bc_mp, cur->bc_bufs[level]->b_bn);
-	else if (level == cur->bc_nlevels - 1 &&
-		 cur->bc_flags & XFS_BTREE_LONG_PTRS)
+		return XFS_DADDR_TO_FSB(cur->bc_mp,
+				xfs_buf_daddr(cur->bc_bufs[level]));
+	if (level == cur->bc_nlevels - 1 && cur->bc_flags & XFS_BTREE_LONG_PTRS)
 		return XFS_INO_TO_FSB(cur->bc_mp, cur->bc_ino.ip->i_ino);
-	else if (!(cur->bc_flags & XFS_BTREE_LONG_PTRS))
+	if (!(cur->bc_flags & XFS_BTREE_LONG_PTRS))
 		return XFS_AGB_TO_FSB(cur->bc_mp, cur->bc_ag.pag->pag_agno, 0);
 	return NULLFSBLOCK;
 }
diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c
index d8fdde206867..2b5da6218977 100644
--- a/fs/xfs/xfs_attr_inactive.c
+++ b/fs/xfs/xfs_attr_inactive.c
@@ -151,7 +151,7 @@ xfs_attr3_node_inactive(
 	}
 
 	xfs_da3_node_hdr_from_disk(dp->i_mount, &ichdr, bp->b_addr);
-	parent_blkno = bp->b_bn;
+	parent_blkno = xfs_buf_daddr(bp);
 	if (!ichdr.count) {
 		xfs_trans_brelse(*trans, bp);
 		return 0;
@@ -271,7 +271,7 @@ xfs_attr3_root_inactive(
 	error = xfs_da3_node_read(*trans, dp, 0, &bp, XFS_ATTR_FORK);
 	if (error)
 		return error;
-	blkno = bp->b_bn;
+	blkno = xfs_buf_daddr(bp);
 
 	/*
 	 * Invalidate the tree, even if the "tree" is only a single leaf block.
diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index d81b0c5e6e9c..b1ab100c09e1 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -581,7 +581,7 @@ xfs_buf_item_push(
 	if (bp->b_flags & XBF_WRITE_FAIL) {
 		xfs_buf_alert_ratelimited(bp, "XFS: Failing async write",
 	    "Failing async write on buffer block 0x%llx. Retrying async write.",
-					  (long long)bp->b_bn);
+					  (long long)xfs_buf_daddr(bp));
 	}
 
 	if (!xfs_buf_delwri_queue(bp, buffer_list))
diff --git a/fs/xfs/xfs_buf_item_recover.c b/fs/xfs/xfs_buf_item_recover.c
index a14f7039d346..a476c7ef5d53 100644
--- a/fs/xfs/xfs_buf_item_recover.c
+++ b/fs/xfs/xfs_buf_item_recover.c
@@ -497,7 +497,7 @@ xlog_recover_do_reg_buffer(
 			if (fa) {
 				xfs_alert(mp,
 	"dquot corrupt at %pS trying to replay into block 0x%llx",
-					fa, bp->b_bn);
+					fa, xfs_buf_daddr(bp));
 				goto next;
 			}
 		}
diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index ce3bc1b291a1..81c445e9489b 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -371,7 +371,7 @@ xfs_buf_corruption_error(
 
 	xfs_alert_tag(mp, XFS_PTAG_VERIFIER_ERROR,
 		  "Metadata corruption detected at %pS, %s block 0x%llx",
-		  fa, bp->b_ops->name, bp->b_bn);
+		  fa, bp->b_ops->name, xfs_buf_daddr(bp));
 
 	xfs_alert(mp, "Unmount and run xfs_repair");
 
@@ -402,7 +402,7 @@ xfs_buf_verifier_error(
 	xfs_alert_tag(mp, XFS_PTAG_VERIFIER_ERROR,
 		  "Metadata %s detected at %pS, %s block 0x%llx %s",
 		  bp->b_error == -EFSBADCRC ? "CRC error" : "corruption",
-		  fa, bp->b_ops->name, bp->b_bn, name);
+		  fa, bp->b_ops->name, xfs_buf_daddr(bp), name);
 
 	xfs_alert(mp, "Unmount and run xfs_repair");
 
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 5bb974c468e8..2fc51985a59e 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -386,10 +386,7 @@ DECLARE_EVENT_CLASS(xfs_buf_class,
 	),
 	TP_fast_assign(
 		__entry->dev = bp->b_target->bt_dev;
-		if (bp->b_bn == XFS_BUF_DADDR_NULL)
-			__entry->bno = bp->b_maps[0].bm_bn;
-		else
-			__entry->bno = bp->b_bn;
+		__entry->bno = xfs_buf_daddr(bp);
 		__entry->nblks = bp->b_length;
 		__entry->hold = atomic_read(&bp->b_hold);
 		__entry->pincount = atomic_read(&bp->b_pin_count);
@@ -457,7 +454,7 @@ DECLARE_EVENT_CLASS(xfs_buf_flags_class,
 	),
 	TP_fast_assign(
 		__entry->dev = bp->b_target->bt_dev;
-		__entry->bno = bp->b_bn;
+		__entry->bno = xfs_buf_daddr(bp);
 		__entry->buffer_length = BBTOB(bp->b_length);
 		__entry->flags = flags;
 		__entry->hold = atomic_read(&bp->b_hold);
@@ -501,7 +498,7 @@ TRACE_EVENT(xfs_buf_ioerror,
 	),
 	TP_fast_assign(
 		__entry->dev = bp->b_target->bt_dev;
-		__entry->bno = bp->b_bn;
+		__entry->bno = xfs_buf_daddr(bp);
 		__entry->buffer_length = BBTOB(bp->b_length);
 		__entry->hold = atomic_read(&bp->b_hold);
 		__entry->pincount = atomic_read(&bp->b_pin_count);
@@ -544,7 +541,7 @@ DECLARE_EVENT_CLASS(xfs_buf_item_class,
 		__entry->bli_flags = bip->bli_flags;
 		__entry->bli_recur = bip->bli_recur;
 		__entry->bli_refcount = atomic_read(&bip->bli_refcount);
-		__entry->buf_bno = bip->bli_buf->b_bn;
+		__entry->buf_bno = xfs_buf_daddr(bip->bli_buf);
 		__entry->buf_len = BBTOB(bip->bli_buf->b_length);
 		__entry->buf_flags = bip->bli_buf->b_flags;
 		__entry->buf_hold = atomic_read(&bip->bli_buf->b_hold);
@@ -2435,7 +2432,7 @@ DECLARE_EVENT_CLASS(xfs_btree_cur_class,
 		__entry->level = level;
 		__entry->nlevels = cur->bc_nlevels;
 		__entry->ptr = cur->bc_ptrs[level];
-		__entry->daddr = bp ? bp->b_bn : -1;
+		__entry->daddr = bp ? xfs_buf_daddr(bp) : -1;
 	),
 	TP_printk("dev %d:%d btree %s level %d/%d ptr %d daddr 0x%llx",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
-- 
2.31.1

Re: [PATCH 2/3] xfs: convert bp->b_bn references to xfs_buf_daddr()

[Darrick J. Wong]

On Tue, Aug 10, 2021 at 03:28:50PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Stop directly referencing b_bn in code outside the buffer cache, as
> b_bn is supposed to be used only as an internal cache index.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  fs/xfs/libxfs/xfs_ag.c             |  1 -
>  fs/xfs/libxfs/xfs_attr_leaf.c      |  4 ++--
>  fs/xfs/libxfs/xfs_attr_remote.c    |  8 ++++----
>  fs/xfs/libxfs/xfs_bmap.c           |  2 +-
>  fs/xfs/libxfs/xfs_btree.c          | 25 +++++++++++++------------
>  fs/xfs/libxfs/xfs_da_btree.c       |  8 ++++----
>  fs/xfs/libxfs/xfs_dir2_block.c     |  4 ++--
>  fs/xfs/libxfs/xfs_dir2_data.c      |  4 ++--
>  fs/xfs/libxfs/xfs_dir2_leaf.c      |  4 ++--
>  fs/xfs/libxfs/xfs_dir2_node.c      |  6 +++---
>  fs/xfs/libxfs/xfs_inode_buf.c      |  2 +-
>  fs/xfs/libxfs/xfs_sb.c             |  2 +-
>  fs/xfs/libxfs/xfs_symlink_remote.c |  4 ++--
>  fs/xfs/scrub/bitmap.c              |  4 ++--
>  fs/xfs/scrub/common.c              |  8 ++++----
>  fs/xfs/scrub/trace.c               |  8 ++++----
>  fs/xfs/xfs_attr_inactive.c         |  4 ++--
>  fs/xfs/xfs_buf_item.c              |  2 +-
>  fs/xfs/xfs_buf_item_recover.c      |  2 +-
>  fs/xfs/xfs_error.c                 |  4 ++--
>  fs/xfs/xfs_trace.h                 | 13 +++++--------
>  21 files changed, 58 insertions(+), 61 deletions(-)
> 

<snip>

> diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
> index dc704c2b22cb..00f631b1d4e9 100644
> --- a/fs/xfs/libxfs/xfs_btree.c
> +++ b/fs/xfs/libxfs/xfs_btree.c

<snip>

> @@ -2923,10 +2923,11 @@ xfs_btree_new_iroot(
>  	 */
>  	memcpy(cblock, block, xfs_btree_block_len(cur));
>  	if (cur->bc_flags & XFS_BTREE_CRC_BLOCKS) {
> +		xfs_daddr_t bno = cpu_to_be64(xfs_buf_daddr(cbp));

Shouldn't the type of @bno be __be64?

--D

>  		if (cur->bc_flags & XFS_BTREE_LONG_PTRS)
> -			cblock->bb_u.l.bb_blkno = cpu_to_be64(cbp->b_bn);
> +			cblock->bb_u.l.bb_blkno = bno;
>  		else
> -			cblock->bb_u.s.bb_blkno = cpu_to_be64(cbp->b_bn);
> +			cblock->bb_u.s.bb_blkno = bno;
>  	}
>  
>  	be16_add_cpu(&block->bb_level, 1);
> @@ -3225,7 +3226,7 @@ xfs_btree_insrec(
>  
>  	/* Get pointers to the btree buffer and block. */
>  	block = xfs_btree_get_block(cur, level, &bp);
> -	old_bn = bp ? bp->b_bn : XFS_BUF_DADDR_NULL;
> +	old_bn = bp ? xfs_buf_daddr(bp) : XFS_BUF_DADDR_NULL;
>  	numrecs = xfs_btree_get_numrecs(block);
>  
>  #ifdef DEBUG
> @@ -3341,7 +3342,7 @@ xfs_btree_insrec(
>  	 * some records into the new tree block), so use the regular key
>  	 * update mechanism.
>  	 */
> -	if (bp && bp->b_bn != old_bn) {
> +	if (bp && xfs_buf_daddr(bp) != old_bn) {
>  		xfs_btree_get_keys(cur, block, lkey);
>  	} else if (xfs_btree_needs_key_update(cur, optr)) {
>  		error = xfs_btree_update_keys(cur, level);
> @@ -4422,7 +4423,7 @@ xfs_btree_lblock_v5hdr_verify(
>  		return __this_address;
>  	if (!uuid_equal(&block->bb_u.l.bb_uuid, &mp->m_sb.sb_meta_uuid))
>  		return __this_address;
> -	if (block->bb_u.l.bb_blkno != cpu_to_be64(bp->b_bn))
> +	if (block->bb_u.l.bb_blkno != cpu_to_be64(xfs_buf_daddr(bp)))
>  		return __this_address;
>  	if (owner != XFS_RMAP_OWN_UNKNOWN &&
>  	    be64_to_cpu(block->bb_u.l.bb_owner) != owner)
> @@ -4472,7 +4473,7 @@ xfs_btree_sblock_v5hdr_verify(
>  		return __this_address;
>  	if (!uuid_equal(&block->bb_u.s.bb_uuid, &mp->m_sb.sb_meta_uuid))
>  		return __this_address;
> -	if (block->bb_u.s.bb_blkno != cpu_to_be64(bp->b_bn))
> +	if (block->bb_u.s.bb_blkno != cpu_to_be64(xfs_buf_daddr(bp)))
>  		return __this_address;
>  	if (pag && be32_to_cpu(block->bb_u.s.bb_owner) != pag->pag_agno)
>  		return __this_address;
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index 99f81f6bb306..c062e2c85178 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -194,7 +194,7 @@ xfs_da3_blkinfo_verify(
>  	if (xfs_has_crc(mp)) {
>  		if (!uuid_equal(&hdr3->uuid, &mp->m_sb.sb_meta_uuid))
>  			return __this_address;
> -		if (be64_to_cpu(hdr3->blkno) != bp->b_bn)
> +		if (be64_to_cpu(hdr3->blkno) != xfs_buf_daddr(bp))
>  			return __this_address;
>  		if (!xfs_log_check_lsn(mp, be64_to_cpu(hdr3->lsn)))
>  			return __this_address;
> @@ -447,7 +447,7 @@ xfs_da3_node_create(
>  
>  		memset(hdr3, 0, sizeof(struct xfs_da3_node_hdr));
>  		ichdr.magic = XFS_DA3_NODE_MAGIC;
> -		hdr3->info.blkno = cpu_to_be64(bp->b_bn);
> +		hdr3->info.blkno = cpu_to_be64(xfs_buf_daddr(bp));
>  		hdr3->info.owner = cpu_to_be64(args->dp->i_ino);
>  		uuid_copy(&hdr3->info.uuid, &mp->m_sb.sb_meta_uuid);
>  	} else {
> @@ -711,7 +711,7 @@ xfs_da3_root_split(
>  	    oldroot->hdr.info.magic == cpu_to_be16(XFS_DIR3_LEAFN_MAGIC)) {
>  		struct xfs_da3_intnode *node3 = (struct xfs_da3_intnode *)node;
>  
> -		node3->hdr.info.blkno = cpu_to_be64(bp->b_bn);
> +		node3->hdr.info.blkno = cpu_to_be64(xfs_buf_daddr(bp));
>  	}
>  	xfs_trans_log_buf(tp, bp, 0, size - 1);
>  
> @@ -1219,7 +1219,7 @@ xfs_da3_root_join(
>  	xfs_trans_buf_copy_type(root_blk->bp, bp);
>  	if (oldroothdr.magic == XFS_DA3_NODE_MAGIC) {
>  		struct xfs_da3_blkinfo *da3 = root_blk->bp->b_addr;
> -		da3->blkno = cpu_to_be64(root_blk->bp->b_bn);
> +		da3->blkno = cpu_to_be64(xfs_buf_daddr(root_blk->bp));
>  	}
>  	xfs_trans_log_buf(args->trans, root_blk->bp, 0,
>  			  args->geo->blksize - 1);
> diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
> index 41e406067f91..df0869bba275 100644
> --- a/fs/xfs/libxfs/xfs_dir2_block.c
> +++ b/fs/xfs/libxfs/xfs_dir2_block.c
> @@ -56,7 +56,7 @@ xfs_dir3_block_verify(
>  	if (xfs_has_crc(mp)) {
>  		if (!uuid_equal(&hdr3->uuid, &mp->m_sb.sb_meta_uuid))
>  			return __this_address;
> -		if (be64_to_cpu(hdr3->blkno) != bp->b_bn)
> +		if (be64_to_cpu(hdr3->blkno) != xfs_buf_daddr(bp))
>  			return __this_address;
>  		if (!xfs_log_check_lsn(mp, be64_to_cpu(hdr3->lsn)))
>  			return __this_address;
> @@ -174,7 +174,7 @@ xfs_dir3_block_init(
>  	if (xfs_has_crc(mp)) {
>  		memset(hdr3, 0, sizeof(*hdr3));
>  		hdr3->magic = cpu_to_be32(XFS_DIR3_BLOCK_MAGIC);
> -		hdr3->blkno = cpu_to_be64(bp->b_bn);
> +		hdr3->blkno = cpu_to_be64(xfs_buf_daddr(bp));
>  		hdr3->owner = cpu_to_be64(dp->i_ino);
>  		uuid_copy(&hdr3->uuid, &mp->m_sb.sb_meta_uuid);
>  		return;
> diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c
> index c90180f2ba5c..dbcf58979a59 100644
> --- a/fs/xfs/libxfs/xfs_dir2_data.c
> +++ b/fs/xfs/libxfs/xfs_dir2_data.c
> @@ -300,7 +300,7 @@ xfs_dir3_data_verify(
>  	if (xfs_has_crc(mp)) {
>  		if (!uuid_equal(&hdr3->uuid, &mp->m_sb.sb_meta_uuid))
>  			return __this_address;
> -		if (be64_to_cpu(hdr3->blkno) != bp->b_bn)
> +		if (be64_to_cpu(hdr3->blkno) != xfs_buf_daddr(bp))
>  			return __this_address;
>  		if (!xfs_log_check_lsn(mp, be64_to_cpu(hdr3->lsn)))
>  			return __this_address;
> @@ -722,7 +722,7 @@ xfs_dir3_data_init(
>  
>  		memset(hdr3, 0, sizeof(*hdr3));
>  		hdr3->magic = cpu_to_be32(XFS_DIR3_DATA_MAGIC);
> -		hdr3->blkno = cpu_to_be64(bp->b_bn);
> +		hdr3->blkno = cpu_to_be64(xfs_buf_daddr(bp));
>  		hdr3->owner = cpu_to_be64(dp->i_ino);
>  		uuid_copy(&hdr3->uuid, &mp->m_sb.sb_meta_uuid);
>  
> diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
> index d03db9cde271..d9b66306a9a7 100644
> --- a/fs/xfs/libxfs/xfs_dir2_leaf.c
> +++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
> @@ -108,7 +108,7 @@ xfs_dir3_leaf1_check(
>  
>  	if (leafhdr.magic == XFS_DIR3_LEAF1_MAGIC) {
>  		struct xfs_dir3_leaf_hdr *leaf3 = bp->b_addr;
> -		if (be64_to_cpu(leaf3->info.blkno) != bp->b_bn)
> +		if (be64_to_cpu(leaf3->info.blkno) != xfs_buf_daddr(bp))
>  			return __this_address;
>  	} else if (leafhdr.magic != XFS_DIR2_LEAF1_MAGIC)
>  		return __this_address;
> @@ -316,7 +316,7 @@ xfs_dir3_leaf_init(
>  		leaf3->info.hdr.magic = (type == XFS_DIR2_LEAF1_MAGIC)
>  					 ? cpu_to_be16(XFS_DIR3_LEAF1_MAGIC)
>  					 : cpu_to_be16(XFS_DIR3_LEAFN_MAGIC);
> -		leaf3->info.blkno = cpu_to_be64(bp->b_bn);
> +		leaf3->info.blkno = cpu_to_be64(xfs_buf_daddr(bp));
>  		leaf3->info.owner = cpu_to_be64(owner);
>  		uuid_copy(&leaf3->info.uuid, &mp->m_sb.sb_meta_uuid);
>  	} else {
> diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
> index fbd2de8b3cf2..7a03aeb9f4c9 100644
> --- a/fs/xfs/libxfs/xfs_dir2_node.c
> +++ b/fs/xfs/libxfs/xfs_dir2_node.c
> @@ -68,7 +68,7 @@ xfs_dir3_leafn_check(
>  
>  	if (leafhdr.magic == XFS_DIR3_LEAFN_MAGIC) {
>  		struct xfs_dir3_leaf_hdr *leaf3 = bp->b_addr;
> -		if (be64_to_cpu(leaf3->info.blkno) != bp->b_bn)
> +		if (be64_to_cpu(leaf3->info.blkno) != xfs_buf_daddr(bp))
>  			return __this_address;
>  	} else if (leafhdr.magic != XFS_DIR2_LEAFN_MAGIC)
>  		return __this_address;
> @@ -110,7 +110,7 @@ xfs_dir3_free_verify(
>  
>  		if (!uuid_equal(&hdr3->uuid, &mp->m_sb.sb_meta_uuid))
>  			return __this_address;
> -		if (be64_to_cpu(hdr3->blkno) != bp->b_bn)
> +		if (be64_to_cpu(hdr3->blkno) != xfs_buf_daddr(bp))
>  			return __this_address;
>  		if (!xfs_log_check_lsn(mp, be64_to_cpu(hdr3->lsn)))
>  			return __this_address;
> @@ -346,7 +346,7 @@ xfs_dir3_free_get_buf(
>  
>  		hdr.magic = XFS_DIR3_FREE_MAGIC;
>  
> -		hdr3->hdr.blkno = cpu_to_be64(bp->b_bn);
> +		hdr3->hdr.blkno = cpu_to_be64(xfs_buf_daddr(bp));
>  		hdr3->hdr.owner = cpu_to_be64(dp->i_ino);
>  		uuid_copy(&hdr3->hdr.uuid, &mp->m_sb.sb_meta_uuid);
>  	} else
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index 83ba63b4ace4..3932b4ebf903 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -71,7 +71,7 @@ xfs_inode_buf_verify(
>  #ifdef DEBUG
>  			xfs_alert(mp,
>  				"bad inode magic/vsn daddr %lld #%d (magic=%x)",
> -				(unsigned long long)bp->b_bn, i,
> +				(unsigned long long)xfs_buf_daddr(bp), i,
>  				be16_to_cpu(dip->di_magic));
>  #endif
>  			xfs_buf_verifier_error(bp, -EFSCORRUPTED,
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index b1c5ec1bd200..e58349be78bd 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -747,7 +747,7 @@ xfs_sb_read_verify(
>  
>  		if (!xfs_buf_verify_cksum(bp, XFS_SB_CRC_OFF)) {
>  			/* Only fail bad secondaries on a known V5 filesystem */
> -			if (bp->b_maps[0].bm_bn == XFS_SB_DADDR ||
> +			if (xfs_buf_daddr(bp) == XFS_SB_DADDR ||
>  			    xfs_has_crc(mp)) {
>  				error = -EFSBADCRC;
>  				goto out_error;
> diff --git a/fs/xfs/libxfs/xfs_symlink_remote.c b/fs/xfs/libxfs/xfs_symlink_remote.c
> index 98b2b6804657..f0b38f4aba80 100644
> --- a/fs/xfs/libxfs/xfs_symlink_remote.c
> +++ b/fs/xfs/libxfs/xfs_symlink_remote.c
> @@ -51,7 +51,7 @@ xfs_symlink_hdr_set(
>  	dsl->sl_bytes = cpu_to_be32(size);
>  	uuid_copy(&dsl->sl_uuid, &mp->m_sb.sb_meta_uuid);
>  	dsl->sl_owner = cpu_to_be64(ino);
> -	dsl->sl_blkno = cpu_to_be64(bp->b_bn);
> +	dsl->sl_blkno = cpu_to_be64(xfs_buf_daddr(bp));
>  	bp->b_ops = &xfs_symlink_buf_ops;
>  
>  	return sizeof(struct xfs_dsymlink_hdr);
> @@ -95,7 +95,7 @@ xfs_symlink_verify(
>  		return __this_address;
>  	if (!uuid_equal(&dsl->sl_uuid, &mp->m_sb.sb_meta_uuid))
>  		return __this_address;
> -	if (bp->b_bn != be64_to_cpu(dsl->sl_blkno))
> +	if (xfs_buf_daddr(bp) != be64_to_cpu(dsl->sl_blkno))
>  		return __this_address;
>  	if (be32_to_cpu(dsl->sl_offset) +
>  				be32_to_cpu(dsl->sl_bytes) >= XFS_SYMLINK_MAXLEN)
> diff --git a/fs/xfs/scrub/bitmap.c b/fs/xfs/scrub/bitmap.c
> index 813b5f219113..d6d24c866bc4 100644
> --- a/fs/xfs/scrub/bitmap.c
> +++ b/fs/xfs/scrub/bitmap.c
> @@ -260,7 +260,7 @@ xbitmap_set_btcur_path(
>  		xfs_btree_get_block(cur, i, &bp);
>  		if (!bp)
>  			continue;
> -		fsb = XFS_DADDR_TO_FSB(cur->bc_mp, bp->b_bn);
> +		fsb = XFS_DADDR_TO_FSB(cur->bc_mp, xfs_buf_daddr(bp));
>  		error = xbitmap_set(bitmap, fsb, 1);
>  		if (error)
>  			return error;
> @@ -284,7 +284,7 @@ xbitmap_collect_btblock(
>  	if (!bp)
>  		return 0;
>  
> -	fsbno = XFS_DADDR_TO_FSB(cur->bc_mp, bp->b_bn);
> +	fsbno = XFS_DADDR_TO_FSB(cur->bc_mp, xfs_buf_daddr(bp));
>  	return xbitmap_set(bitmap, fsbno, 1);
>  }
>  
> diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c
> index 74a442bc7e14..38f8852d9837 100644
> --- a/fs/xfs/scrub/common.c
> +++ b/fs/xfs/scrub/common.c
> @@ -186,7 +186,7 @@ xchk_block_set_preen(
>  	struct xfs_buf		*bp)
>  {
>  	sc->sm->sm_flags |= XFS_SCRUB_OFLAG_PREEN;
> -	trace_xchk_block_preen(sc, bp->b_bn, __return_address);
> +	trace_xchk_block_preen(sc, xfs_buf_daddr(bp), __return_address);
>  }
>  
>  /*
> @@ -219,7 +219,7 @@ xchk_block_set_corrupt(
>  	struct xfs_buf		*bp)
>  {
>  	sc->sm->sm_flags |= XFS_SCRUB_OFLAG_CORRUPT;
> -	trace_xchk_block_error(sc, bp->b_bn, __return_address);
> +	trace_xchk_block_error(sc, xfs_buf_daddr(bp), __return_address);
>  }
>  
>  /* Record a corruption while cross-referencing. */
> @@ -229,7 +229,7 @@ xchk_block_xref_set_corrupt(
>  	struct xfs_buf		*bp)
>  {
>  	sc->sm->sm_flags |= XFS_SCRUB_OFLAG_XCORRUPT;
> -	trace_xchk_block_error(sc, bp->b_bn, __return_address);
> +	trace_xchk_block_error(sc, xfs_buf_daddr(bp), __return_address);
>  }
>  
>  /*
> @@ -787,7 +787,7 @@ xchk_buffer_recheck(
>  	if (!fa)
>  		return;
>  	sc->sm->sm_flags |= XFS_SCRUB_OFLAG_CORRUPT;
> -	trace_xchk_block_error(sc, bp->b_bn, fa);
> +	trace_xchk_block_error(sc, xfs_buf_daddr(bp), fa);
>  }
>  
>  /*
> diff --git a/fs/xfs/scrub/trace.c b/fs/xfs/scrub/trace.c
> index 03882a605a3c..c0ef53fe6611 100644
> --- a/fs/xfs/scrub/trace.c
> +++ b/fs/xfs/scrub/trace.c
> @@ -22,11 +22,11 @@ xchk_btree_cur_fsbno(
>  	int			level)
>  {
>  	if (level < cur->bc_nlevels && cur->bc_bufs[level])
> -		return XFS_DADDR_TO_FSB(cur->bc_mp, cur->bc_bufs[level]->b_bn);
> -	else if (level == cur->bc_nlevels - 1 &&
> -		 cur->bc_flags & XFS_BTREE_LONG_PTRS)
> +		return XFS_DADDR_TO_FSB(cur->bc_mp,
> +				xfs_buf_daddr(cur->bc_bufs[level]));
> +	if (level == cur->bc_nlevels - 1 && cur->bc_flags & XFS_BTREE_LONG_PTRS)
>  		return XFS_INO_TO_FSB(cur->bc_mp, cur->bc_ino.ip->i_ino);
> -	else if (!(cur->bc_flags & XFS_BTREE_LONG_PTRS))
> +	if (!(cur->bc_flags & XFS_BTREE_LONG_PTRS))
>  		return XFS_AGB_TO_FSB(cur->bc_mp, cur->bc_ag.pag->pag_agno, 0);
>  	return NULLFSBLOCK;
>  }
> diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c
> index d8fdde206867..2b5da6218977 100644
> --- a/fs/xfs/xfs_attr_inactive.c
> +++ b/fs/xfs/xfs_attr_inactive.c
> @@ -151,7 +151,7 @@ xfs_attr3_node_inactive(
>  	}
>  
>  	xfs_da3_node_hdr_from_disk(dp->i_mount, &ichdr, bp->b_addr);
> -	parent_blkno = bp->b_bn;
> +	parent_blkno = xfs_buf_daddr(bp);
>  	if (!ichdr.count) {
>  		xfs_trans_brelse(*trans, bp);
>  		return 0;
> @@ -271,7 +271,7 @@ xfs_attr3_root_inactive(
>  	error = xfs_da3_node_read(*trans, dp, 0, &bp, XFS_ATTR_FORK);
>  	if (error)
>  		return error;
> -	blkno = bp->b_bn;
> +	blkno = xfs_buf_daddr(bp);
>  
>  	/*
>  	 * Invalidate the tree, even if the "tree" is only a single leaf block.
> diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
> index d81b0c5e6e9c..b1ab100c09e1 100644
> --- a/fs/xfs/xfs_buf_item.c
> +++ b/fs/xfs/xfs_buf_item.c
> @@ -581,7 +581,7 @@ xfs_buf_item_push(
>  	if (bp->b_flags & XBF_WRITE_FAIL) {
>  		xfs_buf_alert_ratelimited(bp, "XFS: Failing async write",
>  	    "Failing async write on buffer block 0x%llx. Retrying async write.",
> -					  (long long)bp->b_bn);
> +					  (long long)xfs_buf_daddr(bp));
>  	}
>  
>  	if (!xfs_buf_delwri_queue(bp, buffer_list))
> diff --git a/fs/xfs/xfs_buf_item_recover.c b/fs/xfs/xfs_buf_item_recover.c
> index a14f7039d346..a476c7ef5d53 100644
> --- a/fs/xfs/xfs_buf_item_recover.c
> +++ b/fs/xfs/xfs_buf_item_recover.c
> @@ -497,7 +497,7 @@ xlog_recover_do_reg_buffer(
>  			if (fa) {
>  				xfs_alert(mp,
>  	"dquot corrupt at %pS trying to replay into block 0x%llx",
> -					fa, bp->b_bn);
> +					fa, xfs_buf_daddr(bp));
>  				goto next;
>  			}
>  		}
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index ce3bc1b291a1..81c445e9489b 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -371,7 +371,7 @@ xfs_buf_corruption_error(
>  
>  	xfs_alert_tag(mp, XFS_PTAG_VERIFIER_ERROR,
>  		  "Metadata corruption detected at %pS, %s block 0x%llx",
> -		  fa, bp->b_ops->name, bp->b_bn);
> +		  fa, bp->b_ops->name, xfs_buf_daddr(bp));
>  
>  	xfs_alert(mp, "Unmount and run xfs_repair");
>  
> @@ -402,7 +402,7 @@ xfs_buf_verifier_error(
>  	xfs_alert_tag(mp, XFS_PTAG_VERIFIER_ERROR,
>  		  "Metadata %s detected at %pS, %s block 0x%llx %s",
>  		  bp->b_error == -EFSBADCRC ? "CRC error" : "corruption",
> -		  fa, bp->b_ops->name, bp->b_bn, name);
> +		  fa, bp->b_ops->name, xfs_buf_daddr(bp), name);
>  
>  	xfs_alert(mp, "Unmount and run xfs_repair");
>  
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 5bb974c468e8..2fc51985a59e 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -386,10 +386,7 @@ DECLARE_EVENT_CLASS(xfs_buf_class,
>  	),
>  	TP_fast_assign(
>  		__entry->dev = bp->b_target->bt_dev;
> -		if (bp->b_bn == XFS_BUF_DADDR_NULL)
> -			__entry->bno = bp->b_maps[0].bm_bn;
> -		else
> -			__entry->bno = bp->b_bn;
> +		__entry->bno = xfs_buf_daddr(bp);
>  		__entry->nblks = bp->b_length;
>  		__entry->hold = atomic_read(&bp->b_hold);
>  		__entry->pincount = atomic_read(&bp->b_pin_count);
> @@ -457,7 +454,7 @@ DECLARE_EVENT_CLASS(xfs_buf_flags_class,
>  	),
>  	TP_fast_assign(
>  		__entry->dev = bp->b_target->bt_dev;
> -		__entry->bno = bp->b_bn;
> +		__entry->bno = xfs_buf_daddr(bp);
>  		__entry->buffer_length = BBTOB(bp->b_length);
>  		__entry->flags = flags;
>  		__entry->hold = atomic_read(&bp->b_hold);
> @@ -501,7 +498,7 @@ TRACE_EVENT(xfs_buf_ioerror,
>  	),
>  	TP_fast_assign(
>  		__entry->dev = bp->b_target->bt_dev;
> -		__entry->bno = bp->b_bn;
> +		__entry->bno = xfs_buf_daddr(bp);
>  		__entry->buffer_length = BBTOB(bp->b_length);
>  		__entry->hold = atomic_read(&bp->b_hold);
>  		__entry->pincount = atomic_read(&bp->b_pin_count);
> @@ -544,7 +541,7 @@ DECLARE_EVENT_CLASS(xfs_buf_item_class,
>  		__entry->bli_flags = bip->bli_flags;
>  		__entry->bli_recur = bip->bli_recur;
>  		__entry->bli_refcount = atomic_read(&bip->bli_refcount);
> -		__entry->buf_bno = bip->bli_buf->b_bn;
> +		__entry->buf_bno = xfs_buf_daddr(bip->bli_buf);
>  		__entry->buf_len = BBTOB(bip->bli_buf->b_length);
>  		__entry->buf_flags = bip->bli_buf->b_flags;
>  		__entry->buf_hold = atomic_read(&bip->bli_buf->b_hold);
> @@ -2435,7 +2432,7 @@ DECLARE_EVENT_CLASS(xfs_btree_cur_class,
>  		__entry->level = level;
>  		__entry->nlevels = cur->bc_nlevels;
>  		__entry->ptr = cur->bc_ptrs[level];
> -		__entry->daddr = bp ? bp->b_bn : -1;
> +		__entry->daddr = bp ? xfs_buf_daddr(bp) : -1;
>  	),
>  	TP_printk("dev %d:%d btree %s level %d/%d ptr %d daddr 0x%llx",
>  		  MAJOR(__entry->dev), MINOR(__entry->dev),
> -- 
> 2.31.1
> 

Re: [PATCH 2/3] xfs: convert bp->b_bn references to xfs_buf_daddr()

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 3/3] xfs: rename buffer cache index variable b_bn

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

TO stop external users from using b_bn as the disk address of the
buffer, rename it to b_index to indicate that it is the buffer cache
index, not the block number of the buffer. Code that needs the disk
address should use xfs_buf_daddr() to obtain it.

Do the rename and clean up any of the remaining b_bn cruft that is
left over and is now unused.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_buf.c | 19 +++++++++++--------
 fs/xfs/xfs_buf.h | 18 +-----------------
 2 files changed, 12 insertions(+), 25 deletions(-)

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index c1bb6e41595b..6f6c6937baaa 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -251,7 +251,7 @@ _xfs_buf_alloc(
 		return error;
 	}
 
-	bp->b_bn = map[0].bm_bn;
+	bp->b_index = map[0].bm_bn;
 	bp->b_length = 0;
 	for (i = 0; i < nmaps; i++) {
 		bp->b_maps[i].bm_bn = map[i].bm_bn;
@@ -459,7 +459,7 @@ _xfs_buf_obj_cmp(
 	 */
 	BUILD_BUG_ON(offsetof(struct xfs_buf_map, bm_bn) != 0);
 
-	if (bp->b_bn != map->bm_bn)
+	if (bp->b_index != map->bm_bn)
 		return 1;
 
 	if (unlikely(bp->b_length != map->bm_len)) {
@@ -481,7 +481,7 @@ static const struct rhashtable_params xfs_buf_hash_params = {
 	.min_size		= 32,	/* empty AGs have minimal footprint */
 	.nelem_hint		= 16,
 	.key_len		= sizeof(xfs_daddr_t),
-	.key_offset		= offsetof(struct xfs_buf, b_bn),
+	.key_offset		= offsetof(struct xfs_buf, b_index),
 	.head_offset		= offsetof(struct xfs_buf, b_rhash_head),
 	.automatic_shrinking	= true,
 	.obj_cmpfn		= _xfs_buf_obj_cmp,
@@ -853,7 +853,9 @@ xfs_buf_readahead_map(
 
 /*
  * Read an uncached buffer from disk. Allocates and returns a locked
- * buffer containing the disk contents or nothing.
+ * buffer containing the disk contents or nothing. Uncached buffers always have
+ * a cache index of XFS_BUF_DADDR_NULL so we can easily determine if the buffer
+ * is cached or uncached during fault diagnosis.
  */
 int
 xfs_buf_read_uncached(
@@ -875,7 +877,7 @@ xfs_buf_read_uncached(
 
 	/* set up the buffer for a read IO */
 	ASSERT(bp->b_map_count == 1);
-	bp->b_bn = XFS_BUF_DADDR_NULL;  /* always null for uncached buffers */
+	bp->b_index = XFS_BUF_DADDR_NULL;
 	bp->b_maps[0].bm_bn = daddr;
 	bp->b_flags |= XBF_READ;
 	bp->b_ops = ops;
@@ -1513,7 +1515,7 @@ _xfs_buf_ioapply(
 						   SHUTDOWN_CORRUPT_INCORE);
 				return;
 			}
-		} else if (bp->b_bn != XFS_BUF_DADDR_NULL) {
+		} else if (bp->b_index != XFS_BUF_DADDR_NULL) {
 			struct xfs_mount *mp = bp->b_mount;
 
 			/*
@@ -1523,7 +1525,8 @@ _xfs_buf_ioapply(
 			if (xfs_has_crc(mp)) {
 				xfs_warn(mp,
 					"%s: no buf ops on daddr 0x%llx len %d",
-					__func__, bp->b_bn, bp->b_length);
+					__func__, xfs_buf_daddr(bp),
+					bp->b_length);
 				xfs_hex_dump(bp->b_addr,
 						XFS_CORRUPTION_DUMP_LEN);
 				dump_stack();
@@ -1793,7 +1796,7 @@ xfs_buftarg_drain(
 				xfs_buf_alert_ratelimited(bp,
 					"XFS: Corruption Alert",
 "Corruption Alert: Buffer at daddr 0x%llx had permanent write failures!",
-					(long long)bp->b_bn);
+					(long long)xfs_buf_daddr(bp));
 			}
 			xfs_buf_rele(bp);
 		}
diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index 6db2fba44b46..7e408c416607 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -134,11 +134,7 @@ struct xfs_buf {
 	 */
 	struct rhash_head	b_rhash_head;	/* pag buffer hash node */
 
-	/*
-	 * b_bn is the cache index. Do not use directly, use b_maps[0].bm_bn
-	 * for the buffer disk address instead.
-	 */
-	xfs_daddr_t		b_bn;
+	xfs_daddr_t		b_index;	/* buffer cache index */
 	int			b_length;	/* size of buffer in BBs */
 	atomic_t		b_hold;		/* reference count */
 	atomic_t		b_lru_ref;	/* lru reclaim ref count */
@@ -301,18 +297,6 @@ extern int xfs_buf_delwri_pushbuf(struct xfs_buf *, struct list_head *);
 extern int xfs_buf_init(void);
 extern void xfs_buf_terminate(void);
 
-/*
- * These macros use the IO block map rather than b_bn. b_bn is now really
- * just for the buffer cache index for cached buffers. As IO does not use b_bn
- * anymore, uncached buffers do not use b_bn at all and hence must modify the IO
- * map directly. Uncached buffers are not allowed to be discontiguous, so this
- * is safe to do.
- *
- * In future, uncached buffers will pass the block number directly to the io
- * request function and hence these macros will go away at that point.
- */
-#define XFS_BUF_SET_ADDR(bp, bno)	((bp)->b_maps[0].bm_bn = (xfs_daddr_t)(bno))
-
 static inline xfs_daddr_t xfs_buf_daddr(struct xfs_buf *bp)
 {
 	return bp->b_maps[0].bm_bn;
-- 
2.31.1

Re: [PATCH 3/3] xfs: rename buffer cache index variable b_bn

[Darrick J. Wong]

On Tue, Aug 10, 2021 at 03:28:51PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> TO stop external users from using b_bn as the disk address of the
> buffer, rename it to b_index to indicate that it is the buffer cache
> index, not the block number of the buffer. Code that needs the disk
> address should use xfs_buf_daddr() to obtain it.
> 
> Do the rename and clean up any of the remaining b_bn cruft that is
> left over and is now unused.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  fs/xfs/xfs_buf.c | 19 +++++++++++--------
>  fs/xfs/xfs_buf.h | 18 +-----------------
>  2 files changed, 12 insertions(+), 25 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index c1bb6e41595b..6f6c6937baaa 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -251,7 +251,7 @@ _xfs_buf_alloc(
>  		return error;
>  	}
>  
> -	bp->b_bn = map[0].bm_bn;
> +	bp->b_index = map[0].bm_bn;
>  	bp->b_length = 0;
>  	for (i = 0; i < nmaps; i++) {
>  		bp->b_maps[i].bm_bn = map[i].bm_bn;
> @@ -459,7 +459,7 @@ _xfs_buf_obj_cmp(
>  	 */
>  	BUILD_BUG_ON(offsetof(struct xfs_buf_map, bm_bn) != 0);
>  
> -	if (bp->b_bn != map->bm_bn)
> +	if (bp->b_index != map->bm_bn)
>  		return 1;
>  
>  	if (unlikely(bp->b_length != map->bm_len)) {
> @@ -481,7 +481,7 @@ static const struct rhashtable_params xfs_buf_hash_params = {
>  	.min_size		= 32,	/* empty AGs have minimal footprint */
>  	.nelem_hint		= 16,
>  	.key_len		= sizeof(xfs_daddr_t),
> -	.key_offset		= offsetof(struct xfs_buf, b_bn),
> +	.key_offset		= offsetof(struct xfs_buf, b_index),

I would've called this field b_rhash_key, since "index" is a kind of
vague.

>  	.head_offset		= offsetof(struct xfs_buf, b_rhash_head),
>  	.automatic_shrinking	= true,
>  	.obj_cmpfn		= _xfs_buf_obj_cmp,
> @@ -853,7 +853,9 @@ xfs_buf_readahead_map(
>  
>  /*
>   * Read an uncached buffer from disk. Allocates and returns a locked
> - * buffer containing the disk contents or nothing.
> + * buffer containing the disk contents or nothing. Uncached buffers always have
> + * a cache index of XFS_BUF_DADDR_NULL so we can easily determine if the buffer
> + * is cached or uncached during fault diagnosis.
>   */
>  int
>  xfs_buf_read_uncached(
> @@ -875,7 +877,7 @@ xfs_buf_read_uncached(
>  
>  	/* set up the buffer for a read IO */
>  	ASSERT(bp->b_map_count == 1);
> -	bp->b_bn = XFS_BUF_DADDR_NULL;  /* always null for uncached buffers */
> +	bp->b_index = XFS_BUF_DADDR_NULL;
>  	bp->b_maps[0].bm_bn = daddr;
>  	bp->b_flags |= XBF_READ;
>  	bp->b_ops = ops;
> @@ -1513,7 +1515,7 @@ _xfs_buf_ioapply(
>  						   SHUTDOWN_CORRUPT_INCORE);
>  				return;
>  			}
> -		} else if (bp->b_bn != XFS_BUF_DADDR_NULL) {
> +		} else if (bp->b_index != XFS_BUF_DADDR_NULL) {
>  			struct xfs_mount *mp = bp->b_mount;
>  
>  			/*
> @@ -1523,7 +1525,8 @@ _xfs_buf_ioapply(
>  			if (xfs_has_crc(mp)) {
>  				xfs_warn(mp,
>  					"%s: no buf ops on daddr 0x%llx len %d",
> -					__func__, bp->b_bn, bp->b_length);
> +					__func__, xfs_buf_daddr(bp),
> +					bp->b_length);
>  				xfs_hex_dump(bp->b_addr,
>  						XFS_CORRUPTION_DUMP_LEN);
>  				dump_stack();
> @@ -1793,7 +1796,7 @@ xfs_buftarg_drain(
>  				xfs_buf_alert_ratelimited(bp,
>  					"XFS: Corruption Alert",
>  "Corruption Alert: Buffer at daddr 0x%llx had permanent write failures!",
> -					(long long)bp->b_bn);
> +					(long long)xfs_buf_daddr(bp));

These belong in the previous patch, right?

Aside from those two things, the rest of the patch looks ready to go.

--D

>  			}
>  			xfs_buf_rele(bp);
>  		}
> diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
> index 6db2fba44b46..7e408c416607 100644
> --- a/fs/xfs/xfs_buf.h
> +++ b/fs/xfs/xfs_buf.h
> @@ -134,11 +134,7 @@ struct xfs_buf {
>  	 */
>  	struct rhash_head	b_rhash_head;	/* pag buffer hash node */
>  
> -	/*
> -	 * b_bn is the cache index. Do not use directly, use b_maps[0].bm_bn
> -	 * for the buffer disk address instead.
> -	 */
> -	xfs_daddr_t		b_bn;
> +	xfs_daddr_t		b_index;	/* buffer cache index */
>  	int			b_length;	/* size of buffer in BBs */
>  	atomic_t		b_hold;		/* reference count */
>  	atomic_t		b_lru_ref;	/* lru reclaim ref count */
> @@ -301,18 +297,6 @@ extern int xfs_buf_delwri_pushbuf(struct xfs_buf *, struct list_head *);
>  extern int xfs_buf_init(void);
>  extern void xfs_buf_terminate(void);
>  
> -/*
> - * These macros use the IO block map rather than b_bn. b_bn is now really
> - * just for the buffer cache index for cached buffers. As IO does not use b_bn
> - * anymore, uncached buffers do not use b_bn at all and hence must modify the IO
> - * map directly. Uncached buffers are not allowed to be discontiguous, so this
> - * is safe to do.
> - *
> - * In future, uncached buffers will pass the block number directly to the io
> - * request function and hence these macros will go away at that point.
> - */
> -#define XFS_BUF_SET_ADDR(bp, bno)	((bp)->b_maps[0].bm_bn = (xfs_daddr_t)(bno))
> -
>  static inline xfs_daddr_t xfs_buf_daddr(struct xfs_buf *bp)
>  {
>  	return bp->b_maps[0].bm_bn;
> -- 
> 2.31.1
> 

Re: [PATCH 3/3] xfs: rename buffer cache index variable b_bn

[Dave Chinner]

On Tue, Aug 10, 2021 at 05:46:32PM -0700, Darrick J. Wong wrote:
> On Tue, Aug 10, 2021 at 03:28:51PM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > TO stop external users from using b_bn as the disk address of the
> > buffer, rename it to b_index to indicate that it is the buffer cache
> > index, not the block number of the buffer. Code that needs the disk
> > address should use xfs_buf_daddr() to obtain it.
> > 
> > Do the rename and clean up any of the remaining b_bn cruft that is
> > left over and is now unused.
> > 
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > ---
> >  fs/xfs/xfs_buf.c | 19 +++++++++++--------
> >  fs/xfs/xfs_buf.h | 18 +-----------------
> >  2 files changed, 12 insertions(+), 25 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> > index c1bb6e41595b..6f6c6937baaa 100644
> > --- a/fs/xfs/xfs_buf.c
> > +++ b/fs/xfs/xfs_buf.c
> > @@ -251,7 +251,7 @@ _xfs_buf_alloc(
> >  		return error;
> >  	}
> >  
> > -	bp->b_bn = map[0].bm_bn;
> > +	bp->b_index = map[0].bm_bn;
> >  	bp->b_length = 0;
> >  	for (i = 0; i < nmaps; i++) {
> >  		bp->b_maps[i].bm_bn = map[i].bm_bn;
> > @@ -459,7 +459,7 @@ _xfs_buf_obj_cmp(
> >  	 */
> >  	BUILD_BUG_ON(offsetof(struct xfs_buf_map, bm_bn) != 0);
> >  
> > -	if (bp->b_bn != map->bm_bn)
> > +	if (bp->b_index != map->bm_bn)
> >  		return 1;
> >  
> >  	if (unlikely(bp->b_length != map->bm_len)) {
> > @@ -481,7 +481,7 @@ static const struct rhashtable_params xfs_buf_hash_params = {
> >  	.min_size		= 32,	/* empty AGs have minimal footprint */
> >  	.nelem_hint		= 16,
> >  	.key_len		= sizeof(xfs_daddr_t),
> > -	.key_offset		= offsetof(struct xfs_buf, b_bn),
> > +	.key_offset		= offsetof(struct xfs_buf, b_index),
> 
> I would've called this field b_rhash_key, since "index" is a kind of
> vague.

Ok.

> > @@ -875,7 +877,7 @@ xfs_buf_read_uncached(
> >  
> >  	/* set up the buffer for a read IO */
> >  	ASSERT(bp->b_map_count == 1);
> > -	bp->b_bn = XFS_BUF_DADDR_NULL;  /* always null for uncached buffers */
> > +	bp->b_index = XFS_BUF_DADDR_NULL;
> >  	bp->b_maps[0].bm_bn = daddr;
> >  	bp->b_flags |= XBF_READ;
> >  	bp->b_ops = ops;
> > @@ -1513,7 +1515,7 @@ _xfs_buf_ioapply(
> >  						   SHUTDOWN_CORRUPT_INCORE);
> >  				return;
> >  			}
> > -		} else if (bp->b_bn != XFS_BUF_DADDR_NULL) {
> > +		} else if (bp->b_index != XFS_BUF_DADDR_NULL) {
> >  			struct xfs_mount *mp = bp->b_mount;
> >  
> >  			/*
> > @@ -1523,7 +1525,8 @@ _xfs_buf_ioapply(
> >  			if (xfs_has_crc(mp)) {
> >  				xfs_warn(mp,
> >  					"%s: no buf ops on daddr 0x%llx len %d",
> > -					__func__, bp->b_bn, bp->b_length);
> > +					__func__, xfs_buf_daddr(bp),
> > +					bp->b_length);
> >  				xfs_hex_dump(bp->b_addr,
> >  						XFS_CORRUPTION_DUMP_LEN);
> >  				dump_stack();
> > @@ -1793,7 +1796,7 @@ xfs_buftarg_drain(
> >  				xfs_buf_alert_ratelimited(bp,
> >  					"XFS: Corruption Alert",
> >  "Corruption Alert: Buffer at daddr 0x%llx had permanent write failures!",
> > -					(long long)bp->b_bn);
> > +					(long long)xfs_buf_daddr(bp));
> 
> These belong in the previous patch, right?

Depends on your POV. This patch cleans up all the internal (mis)uses of
b_bn, while the previous patches addressed all the external uses.
The mods could be in either, I just chose to split internal/external
modification based on the file rather than on the specific (ab)use
being corrected....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 3/3] xfs: rename buffer cache index variable b_bn

[Darrick J. Wong]

On Wed, Aug 18, 2021 at 09:48:27AM +1000, Dave Chinner wrote:
> On Tue, Aug 10, 2021 at 05:46:32PM -0700, Darrick J. Wong wrote:
> > On Tue, Aug 10, 2021 at 03:28:51PM +1000, Dave Chinner wrote:
> > > From: Dave Chinner <dchinner@redhat.com>
> > > 
> > > TO stop external users from using b_bn as the disk address of the
> > > buffer, rename it to b_index to indicate that it is the buffer cache
> > > index, not the block number of the buffer. Code that needs the disk
> > > address should use xfs_buf_daddr() to obtain it.
> > > 
> > > Do the rename and clean up any of the remaining b_bn cruft that is
> > > left over and is now unused.
> > > 
> > > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > > ---
> > >  fs/xfs/xfs_buf.c | 19 +++++++++++--------
> > >  fs/xfs/xfs_buf.h | 18 +-----------------
> > >  2 files changed, 12 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> > > index c1bb6e41595b..6f6c6937baaa 100644
> > > --- a/fs/xfs/xfs_buf.c
> > > +++ b/fs/xfs/xfs_buf.c
> > > @@ -251,7 +251,7 @@ _xfs_buf_alloc(
> > >  		return error;
> > >  	}
> > >  
> > > -	bp->b_bn = map[0].bm_bn;
> > > +	bp->b_index = map[0].bm_bn;
> > >  	bp->b_length = 0;
> > >  	for (i = 0; i < nmaps; i++) {
> > >  		bp->b_maps[i].bm_bn = map[i].bm_bn;
> > > @@ -459,7 +459,7 @@ _xfs_buf_obj_cmp(
> > >  	 */
> > >  	BUILD_BUG_ON(offsetof(struct xfs_buf_map, bm_bn) != 0);
> > >  
> > > -	if (bp->b_bn != map->bm_bn)
> > > +	if (bp->b_index != map->bm_bn)
> > >  		return 1;
> > >  
> > >  	if (unlikely(bp->b_length != map->bm_len)) {
> > > @@ -481,7 +481,7 @@ static const struct rhashtable_params xfs_buf_hash_params = {
> > >  	.min_size		= 32,	/* empty AGs have minimal footprint */
> > >  	.nelem_hint		= 16,
> > >  	.key_len		= sizeof(xfs_daddr_t),
> > > -	.key_offset		= offsetof(struct xfs_buf, b_bn),
> > > +	.key_offset		= offsetof(struct xfs_buf, b_index),
> > 
> > I would've called this field b_rhash_key, since "index" is a kind of
> > vague.
> 
> Ok.
> 
> > > @@ -875,7 +877,7 @@ xfs_buf_read_uncached(
> > >  
> > >  	/* set up the buffer for a read IO */
> > >  	ASSERT(bp->b_map_count == 1);
> > > -	bp->b_bn = XFS_BUF_DADDR_NULL;  /* always null for uncached buffers */
> > > +	bp->b_index = XFS_BUF_DADDR_NULL;
> > >  	bp->b_maps[0].bm_bn = daddr;
> > >  	bp->b_flags |= XBF_READ;
> > >  	bp->b_ops = ops;
> > > @@ -1513,7 +1515,7 @@ _xfs_buf_ioapply(
> > >  						   SHUTDOWN_CORRUPT_INCORE);
> > >  				return;
> > >  			}
> > > -		} else if (bp->b_bn != XFS_BUF_DADDR_NULL) {
> > > +		} else if (bp->b_index != XFS_BUF_DADDR_NULL) {
> > >  			struct xfs_mount *mp = bp->b_mount;
> > >  
> > >  			/*
> > > @@ -1523,7 +1525,8 @@ _xfs_buf_ioapply(
> > >  			if (xfs_has_crc(mp)) {
> > >  				xfs_warn(mp,
> > >  					"%s: no buf ops on daddr 0x%llx len %d",
> > > -					__func__, bp->b_bn, bp->b_length);
> > > +					__func__, xfs_buf_daddr(bp),
> > > +					bp->b_length);
> > >  				xfs_hex_dump(bp->b_addr,
> > >  						XFS_CORRUPTION_DUMP_LEN);
> > >  				dump_stack();
> > > @@ -1793,7 +1796,7 @@ xfs_buftarg_drain(
> > >  				xfs_buf_alert_ratelimited(bp,
> > >  					"XFS: Corruption Alert",
> > >  "Corruption Alert: Buffer at daddr 0x%llx had permanent write failures!",
> > > -					(long long)bp->b_bn);
> > > +					(long long)xfs_buf_daddr(bp));
> > 
> > These belong in the previous patch, right?
> 
> Depends on your POV. This patch cleans up all the internal (mis)uses of
> b_bn, while the previous patches addressed all the external uses.
> The mods could be in either, I just chose to split internal/external
> modification based on the file rather than on the specific (ab)use
> being corrected....

<nod> Ok then.  Would you mind noting that in the commit message with
something along the lines of "...and convert internal users."?  My brain
is likely to fall out before you repost this patch. :/

--D

> 
> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com

Re: [PATCH 3/3] xfs: rename buffer cache index variable b_bn

[Dave Chinner]

On Tue, Aug 17, 2021 at 04:56:09PM -0700, Darrick J. Wong wrote:
> On Wed, Aug 18, 2021 at 09:48:27AM +1000, Dave Chinner wrote:
> > On Tue, Aug 10, 2021 at 05:46:32PM -0700, Darrick J. Wong wrote:
> > > On Tue, Aug 10, 2021 at 03:28:51PM +1000, Dave Chinner wrote:
> > > > From: Dave Chinner <dchinner@redhat.com>
> > > > 
> > > > TO stop external users from using b_bn as the disk address of the
> > > > buffer, rename it to b_index to indicate that it is the buffer cache
> > > > index, not the block number of the buffer. Code that needs the disk
> > > > address should use xfs_buf_daddr() to obtain it.
> > > > 
> > > > Do the rename and clean up any of the remaining b_bn cruft that is
> > > > left over and is now unused.
> > > > 
> > > > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > > > ---
> > > >  fs/xfs/xfs_buf.c | 19 +++++++++++--------
> > > >  fs/xfs/xfs_buf.h | 18 +-----------------
> > > >  2 files changed, 12 insertions(+), 25 deletions(-)
> > > > 
> > > > diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> > > > index c1bb6e41595b..6f6c6937baaa 100644
> > > > --- a/fs/xfs/xfs_buf.c
> > > > +++ b/fs/xfs/xfs_buf.c
> > > > @@ -251,7 +251,7 @@ _xfs_buf_alloc(
> > > >  		return error;
> > > >  	}
> > > >  
> > > > -	bp->b_bn = map[0].bm_bn;
> > > > +	bp->b_index = map[0].bm_bn;
> > > >  	bp->b_length = 0;
> > > >  	for (i = 0; i < nmaps; i++) {
> > > >  		bp->b_maps[i].bm_bn = map[i].bm_bn;
> > > > @@ -459,7 +459,7 @@ _xfs_buf_obj_cmp(
> > > >  	 */
> > > >  	BUILD_BUG_ON(offsetof(struct xfs_buf_map, bm_bn) != 0);
> > > >  
> > > > -	if (bp->b_bn != map->bm_bn)
> > > > +	if (bp->b_index != map->bm_bn)
> > > >  		return 1;
> > > >  
> > > >  	if (unlikely(bp->b_length != map->bm_len)) {
> > > > @@ -481,7 +481,7 @@ static const struct rhashtable_params xfs_buf_hash_params = {
> > > >  	.min_size		= 32,	/* empty AGs have minimal footprint */
> > > >  	.nelem_hint		= 16,
> > > >  	.key_len		= sizeof(xfs_daddr_t),
> > > > -	.key_offset		= offsetof(struct xfs_buf, b_bn),
> > > > +	.key_offset		= offsetof(struct xfs_buf, b_index),
> > > 
> > > I would've called this field b_rhash_key, since "index" is a kind of
> > > vague.
> > 
> > Ok.
> > 
> > > > @@ -875,7 +877,7 @@ xfs_buf_read_uncached(
> > > >  
> > > >  	/* set up the buffer for a read IO */
> > > >  	ASSERT(bp->b_map_count == 1);
> > > > -	bp->b_bn = XFS_BUF_DADDR_NULL;  /* always null for uncached buffers */
> > > > +	bp->b_index = XFS_BUF_DADDR_NULL;
> > > >  	bp->b_maps[0].bm_bn = daddr;
> > > >  	bp->b_flags |= XBF_READ;
> > > >  	bp->b_ops = ops;
> > > > @@ -1513,7 +1515,7 @@ _xfs_buf_ioapply(
> > > >  						   SHUTDOWN_CORRUPT_INCORE);
> > > >  				return;
> > > >  			}
> > > > -		} else if (bp->b_bn != XFS_BUF_DADDR_NULL) {
> > > > +		} else if (bp->b_index != XFS_BUF_DADDR_NULL) {
> > > >  			struct xfs_mount *mp = bp->b_mount;
> > > >  
> > > >  			/*
> > > > @@ -1523,7 +1525,8 @@ _xfs_buf_ioapply(
> > > >  			if (xfs_has_crc(mp)) {
> > > >  				xfs_warn(mp,
> > > >  					"%s: no buf ops on daddr 0x%llx len %d",
> > > > -					__func__, bp->b_bn, bp->b_length);
> > > > +					__func__, xfs_buf_daddr(bp),
> > > > +					bp->b_length);
> > > >  				xfs_hex_dump(bp->b_addr,
> > > >  						XFS_CORRUPTION_DUMP_LEN);
> > > >  				dump_stack();
> > > > @@ -1793,7 +1796,7 @@ xfs_buftarg_drain(
> > > >  				xfs_buf_alert_ratelimited(bp,
> > > >  					"XFS: Corruption Alert",
> > > >  "Corruption Alert: Buffer at daddr 0x%llx had permanent write failures!",
> > > > -					(long long)bp->b_bn);
> > > > +					(long long)xfs_buf_daddr(bp));
> > > 
> > > These belong in the previous patch, right?
> > 
> > Depends on your POV. This patch cleans up all the internal (mis)uses of
> > b_bn, while the previous patches addressed all the external uses.
> > The mods could be in either, I just chose to split internal/external
> > modification based on the file rather than on the specific (ab)use
> > being corrected....
> 
> <nod> Ok then.  Would you mind noting that in the commit message with
> something along the lines of "...and convert internal users."?  My brain
> is likely to fall out before you repost this patch. :/

Already done. The previous patch already said "Stop directly
referencing b_bn in code outside the buffer cache, [...]", this one
now says "Do the rename and clean up any of the remaining internal
b_bn users. [...]".

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 3/3] xfs: rename buffer cache index variable b_bn

[Christoph Hellwig]

On Tue, Aug 10, 2021 at 03:28:51PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> TO stop external users from using b_bn as the disk address of the
> buffer, rename it to b_index to indicate that it is the buffer cache
> index, not the block number of the buffer. Code that needs the disk
> address should use xfs_buf_daddr() to obtain it.
> 
> Do the rename and clean up any of the remaining b_bn cruft that is
> left over and is now unused.

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Although the XFS_BUF_SET_ADDR really is a separate thing and should
probably have been a separate patch.

[PATCH 0/3 v2] xfs: clean up buffer cache disk addressing

[Dave Chinner]

Over time, we've moved from using bp->b_bn as the physical disk
address for a buffer to using the address of the first buffer map
attached to the buffer. This can be seen by the implementation of
XFS_BUF_ADDR() macro:

#define XFS_BUF_ADDR(bp) ((bp)->b_maps[0].bm_bn)

The bp->b_bn value is now used as the buffer cache index for the
buffer, and it is always set to XFS_BUF_DADDR_NULL for uncached
buffers. Hence code that uses bp->b_bn for the physical address of
the buffer will not do the right thing when passed an uncached
buffer.

THis series of patches addresses this problem. It adds a helper
function xfs_buf_daddr() to extract the physical address of the
buffer from it, and replaces all the open coded bp->b_bn accesses
and the XFS_BUF_ADDR() users to use it. It then renames b_bn to
b_index and converts all the internal cache lookup code to use
b_index rather than b_bn.

The result is that all code now uses xfs_buf_daddr() where physical
addresses are required, and the cache index variable isn't named in
a manner that engenders external use of access. The changes end up
being relatively small, and the impact on userspace outside libxfs
is even smaller (only a couple of dozen references to b_bn or
XFS_BUF_ADDR).

Version 2
- rebase on 5.14-rc4 + for-next + "xfs: rework feature flags v3"
- use __be64 type for comparison in xfs_btree_new_iroot()
- rename b_index to b_rhash_key
- fix typos and wording in commit messages

Version 1
- based on 5.14-rc4 + for-next
- https://lore.kernel.org/linux-xfs/20210810052851.42312-1-david@fromorbit.com/

[PATCH 3/3] xfs: rename buffer cache index variable b_bn

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

To stop external users from using b_bn as the disk address of the
buffer, rename it to b_rhash_key to indicate that it is the buffer
cache index, not the block number of the buffer. Code that needs the
disk address should use xfs_buf_daddr() to obtain it.

Do the rename and clean up any of the remaining internal b_bn users.
Also clean up any remaining b_bn cruft that is now unused.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_buf.c | 19 +++++++++++--------
 fs/xfs/xfs_buf.h | 18 +-----------------
 2 files changed, 12 insertions(+), 25 deletions(-)

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index c1bb6e41595b..047bd6e3f389 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -251,7 +251,7 @@ _xfs_buf_alloc(
 		return error;
 	}
 
-	bp->b_bn = map[0].bm_bn;
+	bp->b_rhash_key = map[0].bm_bn;
 	bp->b_length = 0;
 	for (i = 0; i < nmaps; i++) {
 		bp->b_maps[i].bm_bn = map[i].bm_bn;
@@ -459,7 +459,7 @@ _xfs_buf_obj_cmp(
 	 */
 	BUILD_BUG_ON(offsetof(struct xfs_buf_map, bm_bn) != 0);
 
-	if (bp->b_bn != map->bm_bn)
+	if (bp->b_rhash_key != map->bm_bn)
 		return 1;
 
 	if (unlikely(bp->b_length != map->bm_len)) {
@@ -481,7 +481,7 @@ static const struct rhashtable_params xfs_buf_hash_params = {
 	.min_size		= 32,	/* empty AGs have minimal footprint */
 	.nelem_hint		= 16,
 	.key_len		= sizeof(xfs_daddr_t),
-	.key_offset		= offsetof(struct xfs_buf, b_bn),
+	.key_offset		= offsetof(struct xfs_buf, b_rhash_key),
 	.head_offset		= offsetof(struct xfs_buf, b_rhash_head),
 	.automatic_shrinking	= true,
 	.obj_cmpfn		= _xfs_buf_obj_cmp,
@@ -853,7 +853,9 @@ xfs_buf_readahead_map(
 
 /*
  * Read an uncached buffer from disk. Allocates and returns a locked
- * buffer containing the disk contents or nothing.
+ * buffer containing the disk contents or nothing. Uncached buffers always have
+ * a cache index of XFS_BUF_DADDR_NULL so we can easily determine if the buffer
+ * is cached or uncached during fault diagnosis.
  */
 int
 xfs_buf_read_uncached(
@@ -875,7 +877,7 @@ xfs_buf_read_uncached(
 
 	/* set up the buffer for a read IO */
 	ASSERT(bp->b_map_count == 1);
-	bp->b_bn = XFS_BUF_DADDR_NULL;  /* always null for uncached buffers */
+	bp->b_rhash_key = XFS_BUF_DADDR_NULL;
 	bp->b_maps[0].bm_bn = daddr;
 	bp->b_flags |= XBF_READ;
 	bp->b_ops = ops;
@@ -1513,7 +1515,7 @@ _xfs_buf_ioapply(
 						   SHUTDOWN_CORRUPT_INCORE);
 				return;
 			}
-		} else if (bp->b_bn != XFS_BUF_DADDR_NULL) {
+		} else if (bp->b_rhash_key != XFS_BUF_DADDR_NULL) {
 			struct xfs_mount *mp = bp->b_mount;
 
 			/*
@@ -1523,7 +1525,8 @@ _xfs_buf_ioapply(
 			if (xfs_has_crc(mp)) {
 				xfs_warn(mp,
 					"%s: no buf ops on daddr 0x%llx len %d",
-					__func__, bp->b_bn, bp->b_length);
+					__func__, xfs_buf_daddr(bp),
+					bp->b_length);
 				xfs_hex_dump(bp->b_addr,
 						XFS_CORRUPTION_DUMP_LEN);
 				dump_stack();
@@ -1793,7 +1796,7 @@ xfs_buftarg_drain(
 				xfs_buf_alert_ratelimited(bp,
 					"XFS: Corruption Alert",
 "Corruption Alert: Buffer at daddr 0x%llx had permanent write failures!",
-					(long long)bp->b_bn);
+					(long long)xfs_buf_daddr(bp));
 			}
 			xfs_buf_rele(bp);
 		}
diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index 6db2fba44b46..6b0200b8007d 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -134,11 +134,7 @@ struct xfs_buf {
 	 */
 	struct rhash_head	b_rhash_head;	/* pag buffer hash node */
 
-	/*
-	 * b_bn is the cache index. Do not use directly, use b_maps[0].bm_bn
-	 * for the buffer disk address instead.
-	 */
-	xfs_daddr_t		b_bn;
+	xfs_daddr_t		b_rhash_key;	/* buffer cache index */
 	int			b_length;	/* size of buffer in BBs */
 	atomic_t		b_hold;		/* reference count */
 	atomic_t		b_lru_ref;	/* lru reclaim ref count */
@@ -301,18 +297,6 @@ extern int xfs_buf_delwri_pushbuf(struct xfs_buf *, struct list_head *);
 extern int xfs_buf_init(void);
 extern void xfs_buf_terminate(void);
 
-/*
- * These macros use the IO block map rather than b_bn. b_bn is now really
- * just for the buffer cache index for cached buffers. As IO does not use b_bn
- * anymore, uncached buffers do not use b_bn at all and hence must modify the IO
- * map directly. Uncached buffers are not allowed to be discontiguous, so this
- * is safe to do.
- *
- * In future, uncached buffers will pass the block number directly to the io
- * request function and hence these macros will go away at that point.
- */
-#define XFS_BUF_SET_ADDR(bp, bno)	((bp)->b_maps[0].bm_bn = (xfs_daddr_t)(bno))
-
 static inline xfs_daddr_t xfs_buf_daddr(struct xfs_buf *bp)
 {
 	return bp->b_maps[0].bm_bn;
-- 
2.31.1

Re: [PATCH 3/3] xfs: rename buffer cache index variable b_bn

[Darrick J. Wong]

On Thu, Aug 19, 2021 at 10:00:55AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> To stop external users from using b_bn as the disk address of the
> buffer, rename it to b_rhash_key to indicate that it is the buffer
> cache index, not the block number of the buffer. Code that needs the
> disk address should use xfs_buf_daddr() to obtain it.
> 
> Do the rename and clean up any of the remaining internal b_bn users.
> Also clean up any remaining b_bn cruft that is now unused.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Haaaa my brain didn't fall out!

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_buf.c | 19 +++++++++++--------
>  fs/xfs/xfs_buf.h | 18 +-----------------
>  2 files changed, 12 insertions(+), 25 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index c1bb6e41595b..047bd6e3f389 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -251,7 +251,7 @@ _xfs_buf_alloc(
>  		return error;
>  	}
>  
> -	bp->b_bn = map[0].bm_bn;
> +	bp->b_rhash_key = map[0].bm_bn;
>  	bp->b_length = 0;
>  	for (i = 0; i < nmaps; i++) {
>  		bp->b_maps[i].bm_bn = map[i].bm_bn;
> @@ -459,7 +459,7 @@ _xfs_buf_obj_cmp(
>  	 */
>  	BUILD_BUG_ON(offsetof(struct xfs_buf_map, bm_bn) != 0);
>  
> -	if (bp->b_bn != map->bm_bn)
> +	if (bp->b_rhash_key != map->bm_bn)
>  		return 1;
>  
>  	if (unlikely(bp->b_length != map->bm_len)) {
> @@ -481,7 +481,7 @@ static const struct rhashtable_params xfs_buf_hash_params = {
>  	.min_size		= 32,	/* empty AGs have minimal footprint */
>  	.nelem_hint		= 16,
>  	.key_len		= sizeof(xfs_daddr_t),
> -	.key_offset		= offsetof(struct xfs_buf, b_bn),
> +	.key_offset		= offsetof(struct xfs_buf, b_rhash_key),
>  	.head_offset		= offsetof(struct xfs_buf, b_rhash_head),
>  	.automatic_shrinking	= true,
>  	.obj_cmpfn		= _xfs_buf_obj_cmp,
> @@ -853,7 +853,9 @@ xfs_buf_readahead_map(
>  
>  /*
>   * Read an uncached buffer from disk. Allocates and returns a locked
> - * buffer containing the disk contents or nothing.
> + * buffer containing the disk contents or nothing. Uncached buffers always have
> + * a cache index of XFS_BUF_DADDR_NULL so we can easily determine if the buffer
> + * is cached or uncached during fault diagnosis.
>   */
>  int
>  xfs_buf_read_uncached(
> @@ -875,7 +877,7 @@ xfs_buf_read_uncached(
>  
>  	/* set up the buffer for a read IO */
>  	ASSERT(bp->b_map_count == 1);
> -	bp->b_bn = XFS_BUF_DADDR_NULL;  /* always null for uncached buffers */
> +	bp->b_rhash_key = XFS_BUF_DADDR_NULL;
>  	bp->b_maps[0].bm_bn = daddr;
>  	bp->b_flags |= XBF_READ;
>  	bp->b_ops = ops;
> @@ -1513,7 +1515,7 @@ _xfs_buf_ioapply(
>  						   SHUTDOWN_CORRUPT_INCORE);
>  				return;
>  			}
> -		} else if (bp->b_bn != XFS_BUF_DADDR_NULL) {
> +		} else if (bp->b_rhash_key != XFS_BUF_DADDR_NULL) {
>  			struct xfs_mount *mp = bp->b_mount;
>  
>  			/*
> @@ -1523,7 +1525,8 @@ _xfs_buf_ioapply(
>  			if (xfs_has_crc(mp)) {
>  				xfs_warn(mp,
>  					"%s: no buf ops on daddr 0x%llx len %d",
> -					__func__, bp->b_bn, bp->b_length);
> +					__func__, xfs_buf_daddr(bp),
> +					bp->b_length);
>  				xfs_hex_dump(bp->b_addr,
>  						XFS_CORRUPTION_DUMP_LEN);
>  				dump_stack();
> @@ -1793,7 +1796,7 @@ xfs_buftarg_drain(
>  				xfs_buf_alert_ratelimited(bp,
>  					"XFS: Corruption Alert",
>  "Corruption Alert: Buffer at daddr 0x%llx had permanent write failures!",
> -					(long long)bp->b_bn);
> +					(long long)xfs_buf_daddr(bp));
>  			}
>  			xfs_buf_rele(bp);
>  		}
> diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
> index 6db2fba44b46..6b0200b8007d 100644
> --- a/fs/xfs/xfs_buf.h
> +++ b/fs/xfs/xfs_buf.h
> @@ -134,11 +134,7 @@ struct xfs_buf {
>  	 */
>  	struct rhash_head	b_rhash_head;	/* pag buffer hash node */
>  
> -	/*
> -	 * b_bn is the cache index. Do not use directly, use b_maps[0].bm_bn
> -	 * for the buffer disk address instead.
> -	 */
> -	xfs_daddr_t		b_bn;
> +	xfs_daddr_t		b_rhash_key;	/* buffer cache index */
>  	int			b_length;	/* size of buffer in BBs */
>  	atomic_t		b_hold;		/* reference count */
>  	atomic_t		b_lru_ref;	/* lru reclaim ref count */
> @@ -301,18 +297,6 @@ extern int xfs_buf_delwri_pushbuf(struct xfs_buf *, struct list_head *);
>  extern int xfs_buf_init(void);
>  extern void xfs_buf_terminate(void);
>  
> -/*
> - * These macros use the IO block map rather than b_bn. b_bn is now really
> - * just for the buffer cache index for cached buffers. As IO does not use b_bn
> - * anymore, uncached buffers do not use b_bn at all and hence must modify the IO
> - * map directly. Uncached buffers are not allowed to be discontiguous, so this
> - * is safe to do.
> - *
> - * In future, uncached buffers will pass the block number directly to the io
> - * request function and hence these macros will go away at that point.
> - */
> -#define XFS_BUF_SET_ADDR(bp, bno)	((bp)->b_maps[0].bm_bn = (xfs_daddr_t)(bno))
> -
>  static inline xfs_daddr_t xfs_buf_daddr(struct xfs_buf *bp)
>  {
>  	return bp->b_maps[0].bm_bn;
> -- 
> 2.31.1
>