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:

quoted

On Tue, Aug 10, 2021 at 05:46:32PM -0700, Darrick J. Wong wrote:

quoted

On Tue, Aug 10, 2021 at 03:28:51PM +1000, Dave Chinner wrote:

quoted

From: Dave Chinner <redacted>

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

quoted

quoted

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