diff mbox series

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

Message ID 20210810052851.42312-4-david@fromorbit.com (mailing list archive)
State Superseded
Headers show
Series xfs: clean up buffer cache disk addressing | expand

Commit Message

Dave Chinner Aug. 10, 2021, 5:28 a.m. UTC
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(-)

Comments

Darrick J. Wong Aug. 11, 2021, 12:46 a.m. UTC | #1
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
>
Christoph Hellwig Aug. 12, 2021, 8:23 a.m. UTC | #2
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.
Dave Chinner Aug. 17, 2021, 11:48 p.m. UTC | #3
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.
Darrick J. Wong Aug. 17, 2021, 11:56 p.m. UTC | #4
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
Dave Chinner Aug. 18, 2021, 12:07 a.m. UTC | #5
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.
diff mbox series

Patch

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;