Re: [PATCH 02/43] xfs: factor out a xfs_rt_check_size helper

Darrick J. Wong
2024-12-12T21:11:10Z

On Wed, Dec 11, 2024 at 09:54:27AM +0100, Christoph Hellwig wrote:
> Add a helper to check that the last block of a RT device is readable
> to share the code between mount and growfs. This also adds the mount
> time overflow check to growfs and improves the error messages.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Seems fine to me...
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
> fs/xfs/xfs_rtalloc.c | 62 ++++++++++++++++++++++----------------------
> 1 file changed, 31 insertions(+), 31 deletions(-)
>
> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> index d8e6d073d64d..bc18b694db75 100644
> --- a/fs/xfs/xfs_rtalloc.c
> +++ b/fs/xfs/xfs_rtalloc.c
> @@ -1248,6 +1248,34 @@ xfs_grow_last_rtg(
> mp->m_sb.sb_rgextents;
> }
>
> +/*
> + * Read in the last block of the RT device to make sure it is accessible.
> + */
> +static int
> +xfs_rt_check_size(
> + struct xfs_mount *mp,
> + xfs_rfsblock_t last_block)
> +{
> + xfs_daddr_t daddr = XFS_FSB_TO_BB(mp, last_block);
> + struct xfs_buf *bp;
> + int error;
> +
> + if (XFS_BB_TO_FSB(mp, daddr) != last_block) {
> + xfs_warn(mp, "RT device size overflow: %llu != %llu",
> + XFS_BB_TO_FSB(mp, daddr), last_block);
> + return -EFBIG;
> + }
> +
> + error = xfs_buf_read_uncached(mp->m_rtdev_targp, daddr,
> + XFS_FSB_TO_BB(mp, 1), 0, &bp, NULL);
> + if (error)
> + xfs_warn(mp, "cannot read last RT device sector (%lld)",
> + last_block);
> + else
> + xfs_buf_relse(bp);
> + return error;
> +}
> +
> /*
> * Grow the realtime area of the filesystem.
> */
> @@ -1259,7 +1287,6 @@ xfs_growfs_rt(
> xfs_rgnumber_t old_rgcount = mp->m_sb.sb_rgcount;
> xfs_rgnumber_t new_rgcount = 1;
> xfs_rgnumber_t rgno;
> - struct xfs_buf *bp;
> xfs_agblock_t old_rextsize = mp->m_sb.sb_rextsize;
> int error;
>
> @@ -1302,15 +1329,10 @@ xfs_growfs_rt(
> error = xfs_sb_validate_fsb_count(&mp->m_sb, in->newblocks);
> if (error)
> goto out_unlock;
> - /*
> - * Read in the last block of the device, make sure it exists.
> - */
> - error = xfs_buf_read_uncached(mp->m_rtdev_targp,
> - XFS_FSB_TO_BB(mp, in->newblocks - 1),
> - XFS_FSB_TO_BB(mp, 1), 0, &bp, NULL);
> +
> + error = xfs_rt_check_size(mp, in->newblocks - 1);
> if (error)
> goto out_unlock;
> - xfs_buf_relse(bp);
>
> /*
> * Calculate new parameters. These are the final values to be reached.
> @@ -1444,10 +1466,6 @@ int /* error */
> xfs_rtmount_init(
> struct xfs_mount *mp) /* file system mount structure */
> {
> - struct xfs_buf *bp; /* buffer for last block of subvolume */
> - xfs_daddr_t d; /* address of last block of subvolume */
> - int error;
> -
> if (mp->m_sb.sb_rblocks == 0)
> return 0;
> if (mp->m_rtdev_targp == NULL) {
> @@ -1458,25 +1476,7 @@ xfs_rtmount_init(
>
> mp->m_rsumblocks = xfs_rtsummary_blockcount(mp, &mp->m_rsumlevels);
>
> - /*
> - * Check that the realtime section is an ok size.
> - */
> - d = (xfs_daddr_t)XFS_FSB_TO_BB(mp, mp->m_sb.sb_rblocks);
> - if (XFS_BB_TO_FSB(mp, d) != mp->m_sb.sb_rblocks) {
> - xfs_warn(mp, "realtime mount -- %llu != %llu",
> - (unsigned long long) XFS_BB_TO_FSB(mp, d),
> - (unsigned long long) mp->m_sb.sb_rblocks);
> - return -EFBIG;
> - }
> - error = xfs_buf_read_uncached(mp->m_rtdev_targp,
> - d - XFS_FSB_TO_BB(mp, 1),
> - XFS_FSB_TO_BB(mp, 1), 0, &bp, NULL);
> - if (error) {
> - xfs_warn(mp, "realtime device size check failed");
> - return error;
> - }
> - xfs_buf_relse(bp);
> - return 0;
> + return xfs_rt_check_size(mp, mp->m_sb.sb_rblocks - 1);
> }
>
> static int
> --
> 2.45.2
>
>