linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* store the buftarg size in the buftarg v2
@ 2025-08-25 11:19 Christoph Hellwig
  2025-08-25 11:19 ` [PATCH 1/2] xfs: track the number of blocks in each buftarg Christoph Hellwig
  2025-08-25 11:19 ` [PATCH 2/2] xfs: use bt_nr_blocks in xfs_dax_translate_range Christoph Hellwig
  0 siblings, 2 replies; 7+ messages in thread
From: Christoph Hellwig @ 2025-08-25 11:19 UTC (permalink / raw)
  To: Carlos Maiolino; +Cc: linux-xfs

Hi all,

when playing around with new code that stores more than the current
superblock in block zero of the RT device I ran into the assert in
xfs_buf_map_verify that checks that a buffer is withing sb_dblocks.

That check is obviously incorrect for targets that are not the main data
device, but unlike to hit in practice as the log buftarg doesn't store
any cached buffers, the RT device currently only one for block 0, and
the in-memory buftargs better don't grow larger than the data device.

It's probably better to fix it anyway before running into real issues
on day, and storing the value also removes the need for the notify
failure code to poke into the block device.

Changes since v1:
 - grow the correct buftarg when applying the growfs deltas

Diffstat:
 xfs_buf.c              |   38 +++++++++++++++++++++-----------------
 xfs_buf.h              |    4 +++-
 xfs_buf_item_recover.c |    7 +++++++
 xfs_notify_failure.c   |    2 +-
 xfs_super.c            |    7 ++++---
 xfs_trans.c            |   21 ++++++++++-----------
 6 files changed, 46 insertions(+), 33 deletions(-)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] xfs: track the number of blocks in each buftarg
  2025-08-25 11:19 store the buftarg size in the buftarg v2 Christoph Hellwig
@ 2025-08-25 11:19 ` Christoph Hellwig
  2025-08-25 15:26   ` Darrick J. Wong
  2025-08-26  5:42   ` Dave Chinner
  2025-08-25 11:19 ` [PATCH 2/2] xfs: use bt_nr_blocks in xfs_dax_translate_range Christoph Hellwig
  1 sibling, 2 replies; 7+ messages in thread
From: Christoph Hellwig @ 2025-08-25 11:19 UTC (permalink / raw)
  To: Carlos Maiolino; +Cc: linux-xfs

Add a bt_nr_blocks to track the number of blocks in each buftarg, and
replace the check that hard codes sb_dblock in xfs_buf_map_verify with
this new value so that it is correct for non-ddev buftargs.  The
RT buftarg only has a superblock in the first block, so it is unlikely
to trigger this, or are we likely to ever have enough blocks in the
in-memory buftargs, but we might as well get the check right.

Fixes: 10616b806d1d ("xfs: fix _xfs_buf_find oops on blocks beyond the filesystem end")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_buf.c              | 38 +++++++++++++++++++----------------
 fs/xfs/xfs_buf.h              |  4 +++-
 fs/xfs/xfs_buf_item_recover.c |  7 +++++++
 fs/xfs/xfs_super.c            |  7 ++++---
 fs/xfs/xfs_trans.c            | 21 +++++++++----------
 5 files changed, 45 insertions(+), 32 deletions(-)

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index f9ef3b2a332a..b9b89f1243a0 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -397,7 +397,7 @@ xfs_buf_map_verify(
 	 * Corrupted block numbers can get through to here, unfortunately, so we
 	 * have to check that the buffer falls within the filesystem bounds.
 	 */
-	eofs = XFS_FSB_TO_BB(btp->bt_mount, btp->bt_mount->m_sb.sb_dblocks);
+	eofs = XFS_FSB_TO_BB(btp->bt_mount, btp->bt_nr_blocks);
 	if (map->bm_bn < 0 || map->bm_bn >= eofs) {
 		xfs_alert(btp->bt_mount,
 			  "%s: daddr 0x%llx out of range, EOFS 0x%llx",
@@ -1720,26 +1720,27 @@ xfs_configure_buftarg_atomic_writes(
 int
 xfs_configure_buftarg(
 	struct xfs_buftarg	*btp,
-	unsigned int		sectorsize)
-{
-	int			error;
+	unsigned int		sectorsize,
+	xfs_rfsblock_t		nr_blocks)
+{
+	if (btp->bt_bdev) {
+		int		error;
+
+		error = bdev_validate_blocksize(btp->bt_bdev, sectorsize);
+		if (error) {
+			xfs_warn(btp->bt_mount,
+				"Cannot use blocksize %u on device %pg, err %d",
+				sectorsize, btp->bt_bdev, error);
+			return -EINVAL;
+		}
 
-	ASSERT(btp->bt_bdev != NULL);
+		if (bdev_can_atomic_write(btp->bt_bdev))
+			xfs_configure_buftarg_atomic_writes(btp);
+	}
 
-	/* Set up metadata sector size info */
 	btp->bt_meta_sectorsize = sectorsize;
 	btp->bt_meta_sectormask = sectorsize - 1;
-
-	error = bdev_validate_blocksize(btp->bt_bdev, sectorsize);
-	if (error) {
-		xfs_warn(btp->bt_mount,
-			"Cannot use blocksize %u on device %pg, err %d",
-			sectorsize, btp->bt_bdev, error);
-		return -EINVAL;
-	}
-
-	if (bdev_can_atomic_write(btp->bt_bdev))
-		xfs_configure_buftarg_atomic_writes(btp);
+	btp->bt_nr_blocks = nr_blocks;
 	return 0;
 }
 
@@ -1749,6 +1750,9 @@ xfs_init_buftarg(
 	size_t				logical_sectorsize,
 	const char			*descr)
 {
+	/* The maximum size of the buftarg is only known once the sb is read. */
+	btp->bt_nr_blocks = (xfs_rfsblock_t)-1;
+
 	/* Set up device logical sector size mask */
 	btp->bt_logical_sectorsize = logical_sectorsize;
 	btp->bt_logical_sectormask = logical_sectorsize - 1;
diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index b269e115d9ac..a9b3def89cfb 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -103,6 +103,7 @@ struct xfs_buftarg {
 	size_t			bt_meta_sectormask;
 	size_t			bt_logical_sectorsize;
 	size_t			bt_logical_sectormask;
+	xfs_rfsblock_t		bt_nr_blocks;
 
 	/* LRU control structures */
 	struct shrinker		*bt_shrinker;
@@ -372,7 +373,8 @@ struct xfs_buftarg *xfs_alloc_buftarg(struct xfs_mount *mp,
 extern void xfs_free_buftarg(struct xfs_buftarg *);
 extern void xfs_buftarg_wait(struct xfs_buftarg *);
 extern void xfs_buftarg_drain(struct xfs_buftarg *);
-int xfs_configure_buftarg(struct xfs_buftarg *btp, unsigned int sectorsize);
+int xfs_configure_buftarg(struct xfs_buftarg *btp, unsigned int sectorsize,
+		xfs_rfsblock_t nr_blocks);
 
 #define xfs_readonly_buftarg(buftarg)	bdev_read_only((buftarg)->bt_bdev)
 
diff --git a/fs/xfs/xfs_buf_item_recover.c b/fs/xfs/xfs_buf_item_recover.c
index 5d58e2ae4972..d43234e04174 100644
--- a/fs/xfs/xfs_buf_item_recover.c
+++ b/fs/xfs/xfs_buf_item_recover.c
@@ -736,6 +736,13 @@ xlog_recover_do_primary_sb_buffer(
 	 */
 	xfs_sb_from_disk(&mp->m_sb, dsb);
 
+	/*
+	 * Grow can change the device size.  Mirror that into the buftarg.
+	 */
+	mp->m_ddev_targp->bt_nr_blocks = mp->m_sb.sb_dblocks;
+	if (mp->m_rtdev_targp && mp->m_rtdev_targp != mp->m_ddev_targp)
+		mp->m_rtdev_targp->bt_nr_blocks = mp->m_sb.sb_dblocks;
+
 	if (mp->m_sb.sb_agcount < orig_agcount) {
 		xfs_alert(mp, "Shrinking AG count in log recovery not supported");
 		return -EFSCORRUPTED;
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index bb0a82635a77..78f0c4707c22 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -541,7 +541,8 @@ xfs_setup_devices(
 {
 	int			error;
 
-	error = xfs_configure_buftarg(mp->m_ddev_targp, mp->m_sb.sb_sectsize);
+	error = xfs_configure_buftarg(mp->m_ddev_targp, mp->m_sb.sb_sectsize,
+			mp->m_sb.sb_dblocks);
 	if (error)
 		return error;
 
@@ -551,7 +552,7 @@ xfs_setup_devices(
 		if (xfs_has_sector(mp))
 			log_sector_size = mp->m_sb.sb_logsectsize;
 		error = xfs_configure_buftarg(mp->m_logdev_targp,
-					    log_sector_size);
+				log_sector_size, mp->m_sb.sb_logblocks);
 		if (error)
 			return error;
 	}
@@ -565,7 +566,7 @@ xfs_setup_devices(
 		mp->m_rtdev_targp = mp->m_ddev_targp;
 	} else if (mp->m_rtname) {
 		error = xfs_configure_buftarg(mp->m_rtdev_targp,
-					    mp->m_sb.sb_sectsize);
+				mp->m_sb.sb_sectsize, mp->m_sb.sb_rblocks);
 		if (error)
 			return error;
 	}
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 575e7028f423..d56820de863e 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -452,19 +452,17 @@ xfs_trans_mod_sb(
  */
 STATIC void
 xfs_trans_apply_sb_deltas(
-	xfs_trans_t	*tp)
+	struct xfs_trans	*tp)
 {
-	struct xfs_dsb	*sbp;
-	struct xfs_buf	*bp;
-	int		whole = 0;
-
-	bp = xfs_trans_getsb(tp);
-	sbp = bp->b_addr;
+	struct xfs_mount	*mp = tp->t_mountp;
+	struct xfs_buf		*bp = xfs_trans_getsb(tp);
+	struct xfs_dsb		*sbp = bp->b_addr;
+	int			whole = 0;
 
 	/*
 	 * Only update the superblock counters if we are logging them
 	 */
-	if (!xfs_has_lazysbcount((tp->t_mountp))) {
+	if (!xfs_has_lazysbcount(mp)) {
 		if (tp->t_icount_delta)
 			be64_add_cpu(&sbp->sb_icount, tp->t_icount_delta);
 		if (tp->t_ifree_delta)
@@ -491,8 +489,7 @@ xfs_trans_apply_sb_deltas(
 	 * write the correct value ondisk.
 	 */
 	if ((tp->t_frextents_delta || tp->t_res_frextents_delta) &&
-	    !xfs_has_rtgroups(tp->t_mountp)) {
-		struct xfs_mount	*mp = tp->t_mountp;
+	    !xfs_has_rtgroups(mp)) {
 		int64_t			rtxdelta;
 
 		rtxdelta = tp->t_frextents_delta + tp->t_res_frextents_delta;
@@ -505,6 +502,7 @@ xfs_trans_apply_sb_deltas(
 
 	if (tp->t_dblocks_delta) {
 		be64_add_cpu(&sbp->sb_dblocks, tp->t_dblocks_delta);
+		mp->m_ddev_targp->bt_nr_blocks += tp->t_dblocks_delta;
 		whole = 1;
 	}
 	if (tp->t_agcount_delta) {
@@ -524,7 +522,7 @@ xfs_trans_apply_sb_deltas(
 		 * recompute the ondisk rtgroup block log.  The incore values
 		 * will be recomputed in xfs_trans_unreserve_and_mod_sb.
 		 */
-		if (xfs_has_rtgroups(tp->t_mountp)) {
+		if (xfs_has_rtgroups(mp)) {
 			sbp->sb_rgblklog = xfs_compute_rgblklog(
 						be32_to_cpu(sbp->sb_rgextents),
 						be32_to_cpu(sbp->sb_rextsize));
@@ -537,6 +535,7 @@ xfs_trans_apply_sb_deltas(
 	}
 	if (tp->t_rblocks_delta) {
 		be64_add_cpu(&sbp->sb_rblocks, tp->t_rblocks_delta);
+		mp->m_rtdev_targp->bt_nr_blocks += tp->t_rblocks_delta;
 		whole = 1;
 	}
 	if (tp->t_rextents_delta) {
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] xfs: use bt_nr_blocks in xfs_dax_translate_range
  2025-08-25 11:19 store the buftarg size in the buftarg v2 Christoph Hellwig
  2025-08-25 11:19 ` [PATCH 1/2] xfs: track the number of blocks in each buftarg Christoph Hellwig
@ 2025-08-25 11:19 ` Christoph Hellwig
  1 sibling, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2025-08-25 11:19 UTC (permalink / raw)
  To: Carlos Maiolino; +Cc: linux-xfs, Darrick J. Wong

Only ranges inside the file system can be translated, and the file system
can be smaller than the containing device.

Fixes: f4ed93037966 ("xfs: don't shut down the filesystem for media failures beyond end of log")
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/xfs_notify_failure.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_notify_failure.c b/fs/xfs/xfs_notify_failure.c
index fbeddcac4792..3726caa38375 100644
--- a/fs/xfs/xfs_notify_failure.c
+++ b/fs/xfs/xfs_notify_failure.c
@@ -165,7 +165,7 @@ xfs_dax_translate_range(
 	uint64_t		*bblen)
 {
 	u64			dev_start = btp->bt_dax_part_off;
-	u64			dev_len = bdev_nr_bytes(btp->bt_bdev);
+	u64			dev_len = BBTOB(btp->bt_nr_blocks);
 	u64			dev_end = dev_start + dev_len - 1;
 
 	/* Notify failure on the whole device. */
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] xfs: track the number of blocks in each buftarg
  2025-08-25 11:19 ` [PATCH 1/2] xfs: track the number of blocks in each buftarg Christoph Hellwig
@ 2025-08-25 15:26   ` Darrick J. Wong
  2025-08-26 13:28     ` Christoph Hellwig
  2025-08-26  5:42   ` Dave Chinner
  1 sibling, 1 reply; 7+ messages in thread
From: Darrick J. Wong @ 2025-08-25 15:26 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Carlos Maiolino, linux-xfs

On Mon, Aug 25, 2025 at 01:19:35PM +0200, Christoph Hellwig wrote:
> Add a bt_nr_blocks to track the number of blocks in each buftarg, and
> replace the check that hard codes sb_dblock in xfs_buf_map_verify with
> this new value so that it is correct for non-ddev buftargs.  The
> RT buftarg only has a superblock in the first block, so it is unlikely
> to trigger this, or are we likely to ever have enough blocks in the
> in-memory buftargs, but we might as well get the check right.
> 
> Fixes: 10616b806d1d ("xfs: fix _xfs_buf_find oops on blocks beyond the filesystem end")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_buf.c              | 38 +++++++++++++++++++----------------
>  fs/xfs/xfs_buf.h              |  4 +++-
>  fs/xfs/xfs_buf_item_recover.c |  7 +++++++
>  fs/xfs/xfs_super.c            |  7 ++++---
>  fs/xfs/xfs_trans.c            | 21 +++++++++----------
>  5 files changed, 45 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index f9ef3b2a332a..b9b89f1243a0 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -397,7 +397,7 @@ xfs_buf_map_verify(
>  	 * Corrupted block numbers can get through to here, unfortunately, so we
>  	 * have to check that the buffer falls within the filesystem bounds.
>  	 */
> -	eofs = XFS_FSB_TO_BB(btp->bt_mount, btp->bt_mount->m_sb.sb_dblocks);
> +	eofs = XFS_FSB_TO_BB(btp->bt_mount, btp->bt_nr_blocks);
>  	if (map->bm_bn < 0 || map->bm_bn >= eofs) {
>  		xfs_alert(btp->bt_mount,
>  			  "%s: daddr 0x%llx out of range, EOFS 0x%llx",
> @@ -1720,26 +1720,27 @@ xfs_configure_buftarg_atomic_writes(
>  int
>  xfs_configure_buftarg(
>  	struct xfs_buftarg	*btp,
> -	unsigned int		sectorsize)
> -{
> -	int			error;
> +	unsigned int		sectorsize,
> +	xfs_rfsblock_t		nr_blocks)
> +{
> +	if (btp->bt_bdev) {
> +		int		error;
> +
> +		error = bdev_validate_blocksize(btp->bt_bdev, sectorsize);
> +		if (error) {
> +			xfs_warn(btp->bt_mount,
> +				"Cannot use blocksize %u on device %pg, err %d",
> +				sectorsize, btp->bt_bdev, error);
> +			return -EINVAL;
> +		}
>  
> -	ASSERT(btp->bt_bdev != NULL);
> +		if (bdev_can_atomic_write(btp->bt_bdev))
> +			xfs_configure_buftarg_atomic_writes(btp);
> +	}
>  
> -	/* Set up metadata sector size info */
>  	btp->bt_meta_sectorsize = sectorsize;
>  	btp->bt_meta_sectormask = sectorsize - 1;
> -
> -	error = bdev_validate_blocksize(btp->bt_bdev, sectorsize);
> -	if (error) {
> -		xfs_warn(btp->bt_mount,
> -			"Cannot use blocksize %u on device %pg, err %d",
> -			sectorsize, btp->bt_bdev, error);
> -		return -EINVAL;
> -	}
> -
> -	if (bdev_can_atomic_write(btp->bt_bdev))
> -		xfs_configure_buftarg_atomic_writes(btp);
> +	btp->bt_nr_blocks = nr_blocks;
>  	return 0;
>  }
>  
> @@ -1749,6 +1750,9 @@ xfs_init_buftarg(
>  	size_t				logical_sectorsize,
>  	const char			*descr)
>  {
> +	/* The maximum size of the buftarg is only known once the sb is read. */
> +	btp->bt_nr_blocks = (xfs_rfsblock_t)-1;
> +
>  	/* Set up device logical sector size mask */
>  	btp->bt_logical_sectorsize = logical_sectorsize;
>  	btp->bt_logical_sectormask = logical_sectorsize - 1;
> diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
> index b269e115d9ac..a9b3def89cfb 100644
> --- a/fs/xfs/xfs_buf.h
> +++ b/fs/xfs/xfs_buf.h
> @@ -103,6 +103,7 @@ struct xfs_buftarg {
>  	size_t			bt_meta_sectormask;
>  	size_t			bt_logical_sectorsize;
>  	size_t			bt_logical_sectormask;
> +	xfs_rfsblock_t		bt_nr_blocks;
>  
>  	/* LRU control structures */
>  	struct shrinker		*bt_shrinker;
> @@ -372,7 +373,8 @@ struct xfs_buftarg *xfs_alloc_buftarg(struct xfs_mount *mp,
>  extern void xfs_free_buftarg(struct xfs_buftarg *);
>  extern void xfs_buftarg_wait(struct xfs_buftarg *);
>  extern void xfs_buftarg_drain(struct xfs_buftarg *);
> -int xfs_configure_buftarg(struct xfs_buftarg *btp, unsigned int sectorsize);
> +int xfs_configure_buftarg(struct xfs_buftarg *btp, unsigned int sectorsize,
> +		xfs_rfsblock_t nr_blocks);
>  
>  #define xfs_readonly_buftarg(buftarg)	bdev_read_only((buftarg)->bt_bdev)
>  
> diff --git a/fs/xfs/xfs_buf_item_recover.c b/fs/xfs/xfs_buf_item_recover.c
> index 5d58e2ae4972..d43234e04174 100644
> --- a/fs/xfs/xfs_buf_item_recover.c
> +++ b/fs/xfs/xfs_buf_item_recover.c
> @@ -736,6 +736,13 @@ xlog_recover_do_primary_sb_buffer(
>  	 */
>  	xfs_sb_from_disk(&mp->m_sb, dsb);
>  
> +	/*
> +	 * Grow can change the device size.  Mirror that into the buftarg.
> +	 */
> +	mp->m_ddev_targp->bt_nr_blocks = mp->m_sb.sb_dblocks;
> +	if (mp->m_rtdev_targp && mp->m_rtdev_targp != mp->m_ddev_targp)
> +		mp->m_rtdev_targp->bt_nr_blocks = mp->m_sb.sb_dblocks;

Crazy question here: say we start with dblocks==1G on a 2GB disk.  Then
we start growfs to double the size of the filesystem, but crash midway
through.  Then in the process of restarting the system, some harried
sysadmin accidentally shrinks the disk to 1GB before letting the
filesystem mount.  Log recovery will try to replay the expansion, but
now the disk is no longer large enough to contain the filesystem.

Should we be checking for that here and erroring out?

--D

> +
>  	if (mp->m_sb.sb_agcount < orig_agcount) {
>  		xfs_alert(mp, "Shrinking AG count in log recovery not supported");
>  		return -EFSCORRUPTED;
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index bb0a82635a77..78f0c4707c22 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -541,7 +541,8 @@ xfs_setup_devices(
>  {
>  	int			error;
>  
> -	error = xfs_configure_buftarg(mp->m_ddev_targp, mp->m_sb.sb_sectsize);
> +	error = xfs_configure_buftarg(mp->m_ddev_targp, mp->m_sb.sb_sectsize,
> +			mp->m_sb.sb_dblocks);
>  	if (error)
>  		return error;
>  
> @@ -551,7 +552,7 @@ xfs_setup_devices(
>  		if (xfs_has_sector(mp))
>  			log_sector_size = mp->m_sb.sb_logsectsize;
>  		error = xfs_configure_buftarg(mp->m_logdev_targp,
> -					    log_sector_size);
> +				log_sector_size, mp->m_sb.sb_logblocks);
>  		if (error)
>  			return error;
>  	}
> @@ -565,7 +566,7 @@ xfs_setup_devices(
>  		mp->m_rtdev_targp = mp->m_ddev_targp;
>  	} else if (mp->m_rtname) {
>  		error = xfs_configure_buftarg(mp->m_rtdev_targp,
> -					    mp->m_sb.sb_sectsize);
> +				mp->m_sb.sb_sectsize, mp->m_sb.sb_rblocks);
>  		if (error)
>  			return error;
>  	}
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index 575e7028f423..d56820de863e 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -452,19 +452,17 @@ xfs_trans_mod_sb(
>   */
>  STATIC void
>  xfs_trans_apply_sb_deltas(
> -	xfs_trans_t	*tp)
> +	struct xfs_trans	*tp)
>  {
> -	struct xfs_dsb	*sbp;
> -	struct xfs_buf	*bp;
> -	int		whole = 0;
> -
> -	bp = xfs_trans_getsb(tp);
> -	sbp = bp->b_addr;
> +	struct xfs_mount	*mp = tp->t_mountp;
> +	struct xfs_buf		*bp = xfs_trans_getsb(tp);
> +	struct xfs_dsb		*sbp = bp->b_addr;
> +	int			whole = 0;
>  
>  	/*
>  	 * Only update the superblock counters if we are logging them
>  	 */
> -	if (!xfs_has_lazysbcount((tp->t_mountp))) {
> +	if (!xfs_has_lazysbcount(mp)) {
>  		if (tp->t_icount_delta)
>  			be64_add_cpu(&sbp->sb_icount, tp->t_icount_delta);
>  		if (tp->t_ifree_delta)
> @@ -491,8 +489,7 @@ xfs_trans_apply_sb_deltas(
>  	 * write the correct value ondisk.
>  	 */
>  	if ((tp->t_frextents_delta || tp->t_res_frextents_delta) &&
> -	    !xfs_has_rtgroups(tp->t_mountp)) {
> -		struct xfs_mount	*mp = tp->t_mountp;
> +	    !xfs_has_rtgroups(mp)) {
>  		int64_t			rtxdelta;
>  
>  		rtxdelta = tp->t_frextents_delta + tp->t_res_frextents_delta;
> @@ -505,6 +502,7 @@ xfs_trans_apply_sb_deltas(
>  
>  	if (tp->t_dblocks_delta) {
>  		be64_add_cpu(&sbp->sb_dblocks, tp->t_dblocks_delta);
> +		mp->m_ddev_targp->bt_nr_blocks += tp->t_dblocks_delta;
>  		whole = 1;
>  	}
>  	if (tp->t_agcount_delta) {
> @@ -524,7 +522,7 @@ xfs_trans_apply_sb_deltas(
>  		 * recompute the ondisk rtgroup block log.  The incore values
>  		 * will be recomputed in xfs_trans_unreserve_and_mod_sb.
>  		 */
> -		if (xfs_has_rtgroups(tp->t_mountp)) {
> +		if (xfs_has_rtgroups(mp)) {
>  			sbp->sb_rgblklog = xfs_compute_rgblklog(
>  						be32_to_cpu(sbp->sb_rgextents),
>  						be32_to_cpu(sbp->sb_rextsize));
> @@ -537,6 +535,7 @@ xfs_trans_apply_sb_deltas(
>  	}
>  	if (tp->t_rblocks_delta) {
>  		be64_add_cpu(&sbp->sb_rblocks, tp->t_rblocks_delta);
> +		mp->m_rtdev_targp->bt_nr_blocks += tp->t_rblocks_delta;
>  		whole = 1;
>  	}
>  	if (tp->t_rextents_delta) {
> -- 
> 2.47.2
> 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] xfs: track the number of blocks in each buftarg
  2025-08-25 11:19 ` [PATCH 1/2] xfs: track the number of blocks in each buftarg Christoph Hellwig
  2025-08-25 15:26   ` Darrick J. Wong
@ 2025-08-26  5:42   ` Dave Chinner
  2025-08-26 13:29     ` Christoph Hellwig
  1 sibling, 1 reply; 7+ messages in thread
From: Dave Chinner @ 2025-08-26  5:42 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Carlos Maiolino, linux-xfs

On Mon, Aug 25, 2025 at 01:19:35PM +0200, Christoph Hellwig wrote:
> Add a bt_nr_blocks to track the number of blocks in each buftarg, and
> replace the check that hard codes sb_dblock in xfs_buf_map_verify with
> this new value so that it is correct for non-ddev buftargs.  The
> RT buftarg only has a superblock in the first block, so it is unlikely
> to trigger this, or are we likely to ever have enough blocks in the
> in-memory buftargs, but we might as well get the check right.
> 
> Fixes: 10616b806d1d ("xfs: fix _xfs_buf_find oops on blocks beyond the filesystem end")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_buf.c              | 38 +++++++++++++++++++----------------
>  fs/xfs/xfs_buf.h              |  4 +++-
>  fs/xfs/xfs_buf_item_recover.c |  7 +++++++
>  fs/xfs/xfs_super.c            |  7 ++++---
>  fs/xfs/xfs_trans.c            | 21 +++++++++----------
>  5 files changed, 45 insertions(+), 32 deletions(-)
....

> diff --git a/fs/xfs/xfs_buf_item_recover.c b/fs/xfs/xfs_buf_item_recover.c
> index 5d58e2ae4972..d43234e04174 100644
> --- a/fs/xfs/xfs_buf_item_recover.c
> +++ b/fs/xfs/xfs_buf_item_recover.c
> @@ -736,6 +736,13 @@ xlog_recover_do_primary_sb_buffer(
>  	 */
>  	xfs_sb_from_disk(&mp->m_sb, dsb);
>  
> +	/*
> +	 * Grow can change the device size.  Mirror that into the buftarg.
> +	 */
> +	mp->m_ddev_targp->bt_nr_blocks = mp->m_sb.sb_dblocks;
> +	if (mp->m_rtdev_targp && mp->m_rtdev_targp != mp->m_ddev_targp)
> +		mp->m_rtdev_targp->bt_nr_blocks = mp->m_sb.sb_dblocks;
                                                              ^
That's not right.

Perhaps we need some growfs crash/recovery tests to exercise this
code....

-Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] xfs: track the number of blocks in each buftarg
  2025-08-25 15:26   ` Darrick J. Wong
@ 2025-08-26 13:28     ` Christoph Hellwig
  0 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2025-08-26 13:28 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Christoph Hellwig, Carlos Maiolino, linux-xfs

On Mon, Aug 25, 2025 at 08:26:49AM -0700, Darrick J. Wong wrote:
> > +	mp->m_ddev_targp->bt_nr_blocks = mp->m_sb.sb_dblocks;
> > +	if (mp->m_rtdev_targp && mp->m_rtdev_targp != mp->m_ddev_targp)
> > +		mp->m_rtdev_targp->bt_nr_blocks = mp->m_sb.sb_dblocks;
> 
> Crazy question here: say we start with dblocks==1G on a 2GB disk.  Then
> we start growfs to double the size of the filesystem, but crash midway
> through.  Then in the process of restarting the system, some harried
> sysadmin accidentally shrinks the disk to 1GB before letting the
> filesystem mount.  Log recovery will try to replay the expansion, but
> now the disk is no longer large enough to contain the filesystem.
> 
> Should we be checking for that here and erroring out?

I don't think having tons of sanity checks here hurts, but I also
don't volunteer to implement it as there's a lot of bigger fish to
fry :)


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] xfs: track the number of blocks in each buftarg
  2025-08-26  5:42   ` Dave Chinner
@ 2025-08-26 13:29     ` Christoph Hellwig
  0 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2025-08-26 13:29 UTC (permalink / raw)
  To: Dave Chinner; +Cc: Christoph Hellwig, Carlos Maiolino, linux-xfs

On Tue, Aug 26, 2025 at 03:42:48PM +1000, Dave Chinner wrote:
> > +	/*
> > +	 * Grow can change the device size.  Mirror that into the buftarg.
> > +	 */
> > +	mp->m_ddev_targp->bt_nr_blocks = mp->m_sb.sb_dblocks;
> > +	if (mp->m_rtdev_targp && mp->m_rtdev_targp != mp->m_ddev_targp)
> > +		mp->m_rtdev_targp->bt_nr_blocks = mp->m_sb.sb_dblocks;
>                                                               ^
> That's not right.

Indeed.  It reverts us to the current state without this patch if
someone does a growfs :)

> Perhaps we need some growfs crash/recovery tests to exercise this
> code....

This code is exercises by the RT/zoned growfs tests.  However as the
only RT metadata buffer is at block 0 nothing will notice a wrong
value here (which is also the reason why the current state of this
that hardcodes the sb_dblocks check actually works).


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-08-26 13:29 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-25 11:19 store the buftarg size in the buftarg v2 Christoph Hellwig
2025-08-25 11:19 ` [PATCH 1/2] xfs: track the number of blocks in each buftarg Christoph Hellwig
2025-08-25 15:26   ` Darrick J. Wong
2025-08-26 13:28     ` Christoph Hellwig
2025-08-26  5:42   ` Dave Chinner
2025-08-26 13:29     ` Christoph Hellwig
2025-08-25 11:19 ` [PATCH 2/2] xfs: use bt_nr_blocks in xfs_dax_translate_range Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).