Re: [PATCH 3/3] xfs: rename buffer cache index variable b_bn
From: Dave Chinner <david@fromorbit.com>
Date: 2021-08-17 23:48:33
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: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
@@ -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