Re: [PATCH 16/43] xfs: allow internal RT devices for zoned mode

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Dec 11, 2024 at 09:54:41AM +0100, Christoph Hellwig wrote:
> Allow creating an RT subvolume on the same device as the main data
> device.  This is mostly used for SMR HDDs where the conventional zones
> are used for the data device and the sequential write required zones
> for the zoned RT section.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

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

--D

>  fs/xfs/libxfs/xfs_group.h   |  6 ++++--
>  fs/xfs/libxfs/xfs_rtgroup.h |  8 +++++---
>  fs/xfs/libxfs/xfs_sb.c      |  1 +
>  fs/xfs/xfs_file.c           |  2 +-
>  fs/xfs/xfs_mount.h          |  7 +++++++
>  fs/xfs/xfs_rtalloc.c        |  3 ++-
>  fs/xfs/xfs_super.c          | 12 ++++++++++--
>  7 files changed, 30 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_group.h b/fs/xfs/libxfs/xfs_group.h
> index 242b05627c7a..a70096113384 100644
> --- a/fs/xfs/libxfs/xfs_group.h
> +++ b/fs/xfs/libxfs/xfs_group.h
> @@ -107,9 +107,11 @@ xfs_gbno_to_daddr(
>  	xfs_agblock_t		gbno)
>  {
>  	struct xfs_mount	*mp = xg->xg_mount;
> -	uint32_t		blocks = mp->m_groups[xg->xg_type].blocks;
> +	struct xfs_groups	*g = &mp->m_groups[xg->xg_type];
> +	xfs_fsblock_t		fsbno;
>  
> -	return XFS_FSB_TO_BB(mp, (xfs_fsblock_t)xg->xg_gno * blocks + gbno);
> +	fsbno = (xfs_fsblock_t)xg->xg_gno * g->blocks + gbno;
> +	return XFS_FSB_TO_BB(mp, g->start_fsb + fsbno);
>  }
>  
>  static inline uint32_t
> diff --git a/fs/xfs/libxfs/xfs_rtgroup.h b/fs/xfs/libxfs/xfs_rtgroup.h
> index 0e1d9474ab77..d4c15c706b17 100644
> --- a/fs/xfs/libxfs/xfs_rtgroup.h
> +++ b/fs/xfs/libxfs/xfs_rtgroup.h
> @@ -230,7 +230,8 @@ xfs_rtb_to_daddr(
>  	xfs_rgnumber_t		rgno = xfs_rtb_to_rgno(mp, rtbno);
>  	uint64_t		start_bno = (xfs_rtblock_t)rgno * g->blocks;
>  
> -	return XFS_FSB_TO_BB(mp, start_bno + (rtbno & g->blkmask));
> +	return XFS_FSB_TO_BB(mp,
> +		g->start_fsb + start_bno + (rtbno & g->blkmask));
>  }
>  
>  static inline xfs_rtblock_t
> @@ -238,10 +239,11 @@ xfs_daddr_to_rtb(
>  	struct xfs_mount	*mp,
>  	xfs_daddr_t		daddr)
>  {
> -	xfs_rfsblock_t		bno = XFS_BB_TO_FSBT(mp, daddr);
> +	struct xfs_groups	*g = &mp->m_groups[XG_TYPE_RTG];
> +	xfs_rfsblock_t		bno;
>  
> +	bno = XFS_BB_TO_FSBT(mp, daddr) - g->start_fsb;
>  	if (xfs_has_rtgroups(mp)) {
> -		struct xfs_groups *g = &mp->m_groups[XG_TYPE_RTG];
>  		xfs_rgnumber_t	rgno;
>  		uint32_t	rgbno;
>  
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index 0bbe0b87bf04..20b8318d4a59 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -1175,6 +1175,7 @@ xfs_sb_mount_rextsize(
>  		rgs->blocks = sbp->sb_rgextents * sbp->sb_rextsize;
>  		rgs->blklog = mp->m_sb.sb_rgblklog;
>  		rgs->blkmask = xfs_mask32lo(mp->m_sb.sb_rgblklog);
> +		rgs->start_fsb = mp->m_sb.sb_rtstart;
>  	} else {
>  		rgs->blocks = 0;
>  		rgs->blklog = 0;
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 6bcfd4c34a37..27301229011b 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -150,7 +150,7 @@ xfs_file_fsync(
>  	 * ensure newly written file data make it to disk before logging the new
>  	 * inode size in case of an extending write.
>  	 */
> -	if (XFS_IS_REALTIME_INODE(ip))
> +	if (XFS_IS_REALTIME_INODE(ip) && mp->m_rtdev_targp != mp->m_ddev_targp)
>  		error = blkdev_issue_flush(mp->m_rtdev_targp->bt_bdev);
>  	else if (mp->m_logdev_targp != mp->m_ddev_targp)
>  		error = blkdev_issue_flush(mp->m_ddev_targp->bt_bdev);
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index 72c5389ff78b..3d92678d2c3b 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -103,6 +103,13 @@ struct xfs_groups {
>  	 * rtgroup, so this mask must be 64-bit.
>  	 */
>  	uint64_t		blkmask;
> +
> +	/*
> +	 * Start of the first group in the device.  This is used to support a
> +	 * RT device following the data device on the same block device for
> +	 * SMR hard drives.
> +	 */
> +	xfs_fsblock_t		start_fsb;
>  };
>  
>  enum xfs_free_counter {
> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> index e457a2c2d561..7ef62e7a91c1 100644
> --- a/fs/xfs/xfs_rtalloc.c
> +++ b/fs/xfs/xfs_rtalloc.c
> @@ -1266,7 +1266,8 @@ xfs_rt_check_size(
>  		return -EFBIG;
>  	}
>  
> -	error = xfs_buf_read_uncached(mp->m_rtdev_targp, daddr,
> +	error = xfs_buf_read_uncached(mp->m_rtdev_targp,
> +			XFS_FSB_TO_BB(mp, mp->m_sb.sb_rtstart) + daddr,
>  			XFS_FSB_TO_BB(mp, 1), 0, &bp, NULL);
>  	if (error)
>  		xfs_warn(mp, "cannot read last RT device sector (%lld)",
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 92dd44965943..18430e975c53 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -533,7 +533,15 @@ xfs_setup_devices(
>  		if (error)
>  			return error;
>  	}
> -	if (mp->m_rtdev_targp) {
> +
> +	if (mp->m_sb.sb_rtstart) {
> +		if (mp->m_rtdev_targp) {
> +			xfs_warn(mp,
> +		"can't use internal and external rtdev at the same time");
> +			return -EINVAL;
> +		}
> +		mp->m_rtdev_targp = mp->m_ddev_targp;
> +	} else if (mp->m_rtname) {
>  		error = xfs_setsize_buftarg(mp->m_rtdev_targp,
>  					    mp->m_sb.sb_sectsize);
>  		if (error)
> @@ -757,7 +765,7 @@ xfs_mount_free(
>  {
>  	if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp)
>  		xfs_free_buftarg(mp->m_logdev_targp);
> -	if (mp->m_rtdev_targp)
> +	if (mp->m_rtdev_targp && mp->m_rtdev_targp != mp->m_ddev_targp)
>  		xfs_free_buftarg(mp->m_rtdev_targp);
>  	if (mp->m_ddev_targp)
>  		xfs_free_buftarg(mp->m_ddev_targp);
> -- 
> 2.45.2
> 
>