* store the buftarg size in the buftarg @ 2025-08-18 5:11 Christoph Hellwig 2025-08-18 5:11 ` [PATCH 1/2] xfs: track the number of blocks in each buftarg Christoph Hellwig 2025-08-18 5:11 ` [PATCH 2/2] xfs: use bt_nr_blocks in xfs_dax_translate_range Christoph Hellwig 0 siblings, 2 replies; 15+ messages in thread From: Christoph Hellwig @ 2025-08-18 5:11 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. 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] 15+ messages in thread
* [PATCH 1/2] xfs: track the number of blocks in each buftarg 2025-08-18 5:11 store the buftarg size in the buftarg Christoph Hellwig @ 2025-08-18 5:11 ` Christoph Hellwig 2025-08-18 20:48 ` Darrick J. Wong 2025-08-18 5:11 ` [PATCH 2/2] xfs: use bt_nr_blocks in xfs_dax_translate_range Christoph Hellwig 1 sibling, 1 reply; 15+ messages in thread From: Christoph Hellwig @ 2025-08-18 5:11 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..584bd725ef18 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_dblocks_delta; whole = 1; } if (tp->t_rextents_delta) { -- 2.47.2 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] xfs: track the number of blocks in each buftarg 2025-08-18 5:11 ` [PATCH 1/2] xfs: track the number of blocks in each buftarg Christoph Hellwig @ 2025-08-18 20:48 ` Darrick J. Wong 2025-08-19 8:06 ` Christoph Hellwig 0 siblings, 1 reply; 15+ messages in thread From: Darrick J. Wong @ 2025-08-18 20:48 UTC (permalink / raw) To: Christoph Hellwig; +Cc: Carlos Maiolino, linux-xfs On Mon, Aug 18, 2025 at 07:11:23AM +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; > + > 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..584bd725ef18 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_dblocks_delta; Should this be += tp->t_rblocks_delta here? --D > whole = 1; > } > if (tp->t_rextents_delta) { > -- > 2.47.2 > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] xfs: track the number of blocks in each buftarg 2025-08-18 20:48 ` Darrick J. Wong @ 2025-08-19 8:06 ` Christoph Hellwig 0 siblings, 0 replies; 15+ messages in thread From: Christoph Hellwig @ 2025-08-19 8:06 UTC (permalink / raw) To: Darrick J. Wong; +Cc: Christoph Hellwig, Carlos Maiolino, linux-xfs On Mon, Aug 18, 2025 at 01:48:30PM -0700, Darrick J. Wong wrote: > > if (tp->t_rblocks_delta) { > > be64_add_cpu(&sbp->sb_rblocks, tp->t_rblocks_delta); > > + mp->m_rtdev_targp->bt_nr_blocks += tp->t_dblocks_delta; > > Should this be += tp->t_rblocks_delta here? Yes. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/2] xfs: use bt_nr_blocks in xfs_dax_translate_range 2025-08-18 5:11 store the buftarg size in the buftarg Christoph Hellwig 2025-08-18 5:11 ` [PATCH 1/2] xfs: track the number of blocks in each buftarg Christoph Hellwig @ 2025-08-18 5:11 ` Christoph Hellwig 2025-08-18 20:49 ` Darrick J. Wong 1 sibling, 1 reply; 15+ messages in thread From: Christoph Hellwig @ 2025-08-18 5:11 UTC (permalink / raw) To: Carlos Maiolino; +Cc: linux-xfs 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> --- 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] 15+ messages in thread
* Re: [PATCH 2/2] xfs: use bt_nr_blocks in xfs_dax_translate_range 2025-08-18 5:11 ` [PATCH 2/2] xfs: use bt_nr_blocks in xfs_dax_translate_range Christoph Hellwig @ 2025-08-18 20:49 ` Darrick J. Wong 0 siblings, 0 replies; 15+ messages in thread From: Darrick J. Wong @ 2025-08-18 20:49 UTC (permalink / raw) To: Christoph Hellwig; +Cc: Carlos Maiolino, linux-xfs On Mon, Aug 18, 2025 at 07:11:24AM +0200, Christoph Hellwig wrote: > 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> > --- > 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); Yeah, dev_len is used to filter out ranges beyond what the filesystem cares about, so Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> --D > u64 dev_end = dev_start + dev_len - 1; > > /* Notify failure on the whole device. */ > -- > 2.47.2 > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* 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 0 siblings, 1 reply; 15+ 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] 15+ 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 0 siblings, 2 replies; 15+ 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] 15+ 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; 15+ 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] 15+ 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; 15+ 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] 15+ 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; 15+ 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] 15+ 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; 15+ 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] 15+ messages in thread
* store the buftarg size in the buftarg v3 @ 2025-09-16 13:52 Christoph Hellwig 2025-09-16 13:52 ` [PATCH 1/2] xfs: track the number of blocks in each buftarg Christoph Hellwig 0 siblings, 1 reply; 15+ messages in thread From: Christoph Hellwig @ 2025-09-16 13:52 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 replying a primary sb update 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] 15+ messages in thread
* [PATCH 1/2] xfs: track the number of blocks in each buftarg 2025-09-16 13:52 store the buftarg size in the buftarg v3 Christoph Hellwig @ 2025-09-16 13:52 ` Christoph Hellwig 2025-09-17 21:26 ` Dave Chinner 0 siblings, 1 reply; 15+ messages in thread From: Christoph Hellwig @ 2025-09-16 13:52 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..7653ec56eb74 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_rblocks; + 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 77acb3e5a4ec..9e759c5b1096 100644 --- a/fs/xfs/xfs_super.c +++ b/fs/xfs/xfs_super.c @@ -535,7 +535,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; @@ -545,7 +546,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; } @@ -559,7 +560,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] 15+ messages in thread
* Re: [PATCH 1/2] xfs: track the number of blocks in each buftarg 2025-09-16 13:52 ` [PATCH 1/2] xfs: track the number of blocks in each buftarg Christoph Hellwig @ 2025-09-17 21:26 ` Dave Chinner 0 siblings, 0 replies; 15+ messages in thread From: Dave Chinner @ 2025-09-17 21:26 UTC (permalink / raw) To: Christoph Hellwig; +Cc: Carlos Maiolino, linux-xfs On Tue, Sep 16, 2025 at 06:52:31AM -0700, 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); Why store the number of blocks in filesystem block units? The buffer cache/buftarg uses daddr addressing, and this is the only place in the whole buffer cache where we've needed to do filesystem block to daddr unit conversion. IMO it should be stored in daddr units to be consistent with all other buffer cache addresses and length, with the conversion to daddr units being done by the xfs_configure_buftarg() caller. This gets rid of this unit conversion in the lookup fast path in addition to removing to pointer chase through the mount to the superblock. This means the only remaining fast path references to the xfs_mount in the lookup path is the stats update code. If the stats then got moved to the buftarg, then we don't access the xfs_mount at all in the buffer cache lookup path except when an error occurs... Other than that, the code looks fine. -Dave. -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* store the buftarg size in the buftarg v4 @ 2025-09-19 13:12 Christoph Hellwig 2025-09-19 13:12 ` [PATCH 1/2] xfs: track the number of blocks in each buftarg Christoph Hellwig 0 siblings, 1 reply; 15+ messages in thread From: Christoph Hellwig @ 2025-09-19 13:12 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 v3: - account in sectors Changes since v2: - grow the correct buftarg when replying a primary sb update Changes since v1: - grow the correct buftarg when applying the growfs deltas Diffstat: xfs_buf.c | 42 +++++++++++++++++++++++------------------- xfs_buf.h | 4 +++- xfs_buf_item_recover.c | 10 ++++++++++ xfs_notify_failure.c | 2 +- xfs_super.c | 7 ++++--- xfs_trans.c | 23 ++++++++++++----------- 6 files changed, 53 insertions(+), 35 deletions(-) ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/2] xfs: track the number of blocks in each buftarg 2025-09-19 13:12 store the buftarg size in the buftarg v4 Christoph Hellwig @ 2025-09-19 13:12 ` Christoph Hellwig 2025-09-19 17:52 ` Darrick J. Wong 0 siblings, 1 reply; 15+ messages in thread From: Christoph Hellwig @ 2025-09-19 13:12 UTC (permalink / raw) To: Carlos Maiolino; +Cc: linux-xfs Add a bt_nr_sectors to track the number of sector 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 | 42 +++++++++++++++++++---------------- fs/xfs/xfs_buf.h | 4 +++- fs/xfs/xfs_buf_item_recover.c | 10 +++++++++ fs/xfs/xfs_super.c | 7 +++--- fs/xfs/xfs_trans.c | 23 ++++++++++--------- 5 files changed, 52 insertions(+), 34 deletions(-) diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c index f9ef3b2a332a..2037c88e604a 100644 --- a/fs/xfs/xfs_buf.c +++ b/fs/xfs/xfs_buf.c @@ -387,8 +387,6 @@ xfs_buf_map_verify( struct xfs_buftarg *btp, struct xfs_buf_map *map) { - xfs_daddr_t eofs; - /* Check for IOs smaller than the sector size / not sector aligned */ ASSERT(!(BBTOB(map->bm_len) < btp->bt_meta_sectorsize)); ASSERT(!(BBTOB(map->bm_bn) & (xfs_off_t)btp->bt_meta_sectormask)); @@ -397,11 +395,10 @@ 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); - if (map->bm_bn < 0 || map->bm_bn >= eofs) { + if (map->bm_bn < 0 || map->bm_bn >= btp->bt_nr_sectors) { xfs_alert(btp->bt_mount, "%s: daddr 0x%llx out of range, EOFS 0x%llx", - __func__, map->bm_bn, eofs); + __func__, map->bm_bn, btp->bt_nr_sectors); WARN_ON(1); return -EFSCORRUPTED; } @@ -1720,26 +1717,30 @@ xfs_configure_buftarg_atomic_writes( int xfs_configure_buftarg( struct xfs_buftarg *btp, - unsigned int sectorsize) + unsigned int sectorsize, + xfs_rfsblock_t nr_blocks) { - int error; + struct xfs_mount *mp = btp->bt_mount; - ASSERT(btp->bt_bdev != NULL); + if (btp->bt_bdev) { + int error; - /* 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(mp, + "Cannot use blocksize %u on device %pg, err %d", + sectorsize, btp->bt_bdev, error); + return -EINVAL; + } - 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); } - if (bdev_can_atomic_write(btp->bt_bdev)) - xfs_configure_buftarg_atomic_writes(btp); + btp->bt_meta_sectorsize = sectorsize; + btp->bt_meta_sectormask = sectorsize - 1; + /* m_blkbb_log is not set up yet */ + btp->bt_nr_sectors = nr_blocks << (mp->m_sb.sb_blocklog - BBSHIFT); 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_sectors = (xfs_daddr_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..8fa7bdf59c91 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_daddr_t bt_nr_sectors; /* 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_fsblock_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..e4c8af873632 100644 --- a/fs/xfs/xfs_buf_item_recover.c +++ b/fs/xfs/xfs_buf_item_recover.c @@ -736,6 +736,16 @@ 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_sectors = + XFS_FSB_TO_BB(mp, mp->m_sb.sb_dblocks); + if (mp->m_rtdev_targp && mp->m_rtdev_targp != mp->m_ddev_targp) { + mp->m_rtdev_targp->bt_nr_sectors = + XFS_FSB_TO_BB(mp, mp->m_sb.sb_rblocks); + } + 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 77acb3e5a4ec..9e759c5b1096 100644 --- a/fs/xfs/xfs_super.c +++ b/fs/xfs/xfs_super.c @@ -535,7 +535,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; @@ -545,7 +546,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; } @@ -559,7 +560,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..474f5a04ec63 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,8 @@ 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_sectors += + XFS_FSB_TO_BB(mp, tp->t_dblocks_delta); whole = 1; } if (tp->t_agcount_delta) { @@ -524,7 +523,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 +536,8 @@ 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_sectors += + XFS_FSB_TO_BB(mp, tp->t_rblocks_delta); whole = 1; } if (tp->t_rextents_delta) { -- 2.47.2 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] xfs: track the number of blocks in each buftarg 2025-09-19 13:12 ` [PATCH 1/2] xfs: track the number of blocks in each buftarg Christoph Hellwig @ 2025-09-19 17:52 ` Darrick J. Wong 0 siblings, 0 replies; 15+ messages in thread From: Darrick J. Wong @ 2025-09-19 17:52 UTC (permalink / raw) To: Christoph Hellwig; +Cc: Carlos Maiolino, linux-xfs On Fri, Sep 19, 2025 at 06:12:08AM -0700, Christoph Hellwig wrote: > Add a bt_nr_sectors to track the number of sector 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> Looks reasonable to me, Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> --D > --- > fs/xfs/xfs_buf.c | 42 +++++++++++++++++++---------------- > fs/xfs/xfs_buf.h | 4 +++- > fs/xfs/xfs_buf_item_recover.c | 10 +++++++++ > fs/xfs/xfs_super.c | 7 +++--- > fs/xfs/xfs_trans.c | 23 ++++++++++--------- > 5 files changed, 52 insertions(+), 34 deletions(-) > > diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c > index f9ef3b2a332a..2037c88e604a 100644 > --- a/fs/xfs/xfs_buf.c > +++ b/fs/xfs/xfs_buf.c > @@ -387,8 +387,6 @@ xfs_buf_map_verify( > struct xfs_buftarg *btp, > struct xfs_buf_map *map) > { > - xfs_daddr_t eofs; > - > /* Check for IOs smaller than the sector size / not sector aligned */ > ASSERT(!(BBTOB(map->bm_len) < btp->bt_meta_sectorsize)); > ASSERT(!(BBTOB(map->bm_bn) & (xfs_off_t)btp->bt_meta_sectormask)); > @@ -397,11 +395,10 @@ 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); > - if (map->bm_bn < 0 || map->bm_bn >= eofs) { > + if (map->bm_bn < 0 || map->bm_bn >= btp->bt_nr_sectors) { > xfs_alert(btp->bt_mount, > "%s: daddr 0x%llx out of range, EOFS 0x%llx", > - __func__, map->bm_bn, eofs); > + __func__, map->bm_bn, btp->bt_nr_sectors); > WARN_ON(1); > return -EFSCORRUPTED; > } > @@ -1720,26 +1717,30 @@ xfs_configure_buftarg_atomic_writes( > int > xfs_configure_buftarg( > struct xfs_buftarg *btp, > - unsigned int sectorsize) > + unsigned int sectorsize, > + xfs_rfsblock_t nr_blocks) > { > - int error; > + struct xfs_mount *mp = btp->bt_mount; > > - ASSERT(btp->bt_bdev != NULL); > + if (btp->bt_bdev) { > + int error; > > - /* 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(mp, > + "Cannot use blocksize %u on device %pg, err %d", > + sectorsize, btp->bt_bdev, error); > + return -EINVAL; > + } > > - 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); > } > > - if (bdev_can_atomic_write(btp->bt_bdev)) > - xfs_configure_buftarg_atomic_writes(btp); > + btp->bt_meta_sectorsize = sectorsize; > + btp->bt_meta_sectormask = sectorsize - 1; > + /* m_blkbb_log is not set up yet */ > + btp->bt_nr_sectors = nr_blocks << (mp->m_sb.sb_blocklog - BBSHIFT); > 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_sectors = (xfs_daddr_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..8fa7bdf59c91 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_daddr_t bt_nr_sectors; > > /* 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_fsblock_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..e4c8af873632 100644 > --- a/fs/xfs/xfs_buf_item_recover.c > +++ b/fs/xfs/xfs_buf_item_recover.c > @@ -736,6 +736,16 @@ 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_sectors = > + XFS_FSB_TO_BB(mp, mp->m_sb.sb_dblocks); > + if (mp->m_rtdev_targp && mp->m_rtdev_targp != mp->m_ddev_targp) { > + mp->m_rtdev_targp->bt_nr_sectors = > + XFS_FSB_TO_BB(mp, mp->m_sb.sb_rblocks); > + } > + > 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 77acb3e5a4ec..9e759c5b1096 100644 > --- a/fs/xfs/xfs_super.c > +++ b/fs/xfs/xfs_super.c > @@ -535,7 +535,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; > > @@ -545,7 +546,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; > } > @@ -559,7 +560,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..474f5a04ec63 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,8 @@ 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_sectors += > + XFS_FSB_TO_BB(mp, tp->t_dblocks_delta); > whole = 1; > } > if (tp->t_agcount_delta) { > @@ -524,7 +523,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 +536,8 @@ 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_sectors += > + XFS_FSB_TO_BB(mp, tp->t_rblocks_delta); > whole = 1; > } > if (tp->t_rextents_delta) { > -- > 2.47.2 > > ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2025-09-19 17:52 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-08-18 5:11 store the buftarg size in the buftarg Christoph Hellwig 2025-08-18 5:11 ` [PATCH 1/2] xfs: track the number of blocks in each buftarg Christoph Hellwig 2025-08-18 20:48 ` Darrick J. Wong 2025-08-19 8:06 ` Christoph Hellwig 2025-08-18 5:11 ` [PATCH 2/2] xfs: use bt_nr_blocks in xfs_dax_translate_range Christoph Hellwig 2025-08-18 20:49 ` Darrick J. Wong -- strict thread matches above, loose matches on Subject: below -- 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-09-16 13:52 store the buftarg size in the buftarg v3 Christoph Hellwig 2025-09-16 13:52 ` [PATCH 1/2] xfs: track the number of blocks in each buftarg Christoph Hellwig 2025-09-17 21:26 ` Dave Chinner 2025-09-19 13:12 store the buftarg size in the buftarg v4 Christoph Hellwig 2025-09-19 13:12 ` [PATCH 1/2] xfs: track the number of blocks in each buftarg Christoph Hellwig 2025-09-19 17:52 ` Darrick J. Wong
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).