Re: [PATCH 35/43] xfs: enable fsmap reporting for internal RT devices

Darrick J. Wong
2024-12-13T23:11:16Z

On Wed, Dec 11, 2024 at 09:55:00AM +0100, Christoph Hellwig wrote:
> File system with internal RT devices are a bit odd in that we need
> to report AGs and RGs. To make this happen use separate synthetic
> fmr_device values for the different sections instead of the dev_t
> mapping used by other XFS configurations.
>
> The data device is reported as file system metadata before the
> start of the RGs for the synthetic RT fmr_device.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/libxfs/xfs_fs.h | 9 +++++
> fs/xfs/xfs_fsmap.c | 80 +++++++++++++++++++++++++++++++++---------
> 2 files changed, 72 insertions(+), 17 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> index 5e66fb2b2cc7..12463ba766da 100644
> --- a/fs/xfs/libxfs/xfs_fs.h
> +++ b/fs/xfs/libxfs/xfs_fs.h
> @@ -1082,6 +1082,15 @@ struct xfs_rtgroup_geometry {
> #define XFS_IOC_COMMIT_RANGE _IOW ('X', 131, struct xfs_commit_range)
> /* XFS_IOC_GETFSUUID ---------- deprecated 140 */
>
> +/*
> + * Devices supported by a single XFS file system. Reported in fsmaps fmr_device
> + * when using internal RT devices.
> + */
> +enum xfs_device {
> + XFS_DEV_DATA = 1,
> + XFS_DEV_LOG = 2,
> + XFS_DEV_RT = 3,
> +};
>
> #ifndef HAVE_BBMACROS
> /*
> diff --git a/fs/xfs/xfs_fsmap.c b/fs/xfs/xfs_fsmap.c
> index 917d4d0e51b3..a4bc1642fe56 100644
> --- a/fs/xfs/xfs_fsmap.c
> +++ b/fs/xfs/xfs_fsmap.c
> @@ -879,17 +879,39 @@ xfs_getfsmap_rtdev_rmapbt(
> struct xfs_mount *mp = tp->t_mountp;
> struct xfs_rtgroup *rtg = NULL;
> struct xfs_btree_cur *bt_cur = NULL;
> + xfs_daddr_t rtstart_daddr;
> xfs_rtblock_t start_rtb;
> xfs_rtblock_t end_rtb;
> xfs_rgnumber_t start_rg, end_rg;
> uint64_t eofs;
> int error = 0;
>
> - eofs = XFS_FSB_TO_BB(mp, mp->m_sb.sb_rblocks);
> + eofs = XFS_FSB_TO_BB(mp, mp->m_sb.sb_rtstart + mp->m_sb.sb_rblocks);
> if (keys[0].fmr_physical >= eofs)
> return 0;
> - start_rtb = xfs_daddr_to_rtb(mp, keys[0].fmr_physical);
> - end_rtb = xfs_daddr_to_rtb(mp, min(eofs - 1, keys[1].fmr_physical));
> +
> + rtstart_daddr = XFS_FSB_TO_BB(mp, mp->m_sb.sb_rtstart);
> + if (keys[0].fmr_physical < rtstart_daddr) {
> + struct xfs_fsmap_irec frec = {
> + .owner = XFS_RMAP_OWN_FS,
> + .len_daddr = rtstart_daddr,
> + };
> +
> + /* Adjust the low key if we are continuing from where we left off. */
> + if (keys[0].fmr_length > 0) {
> + info->low_daddr = keys[0].fmr_physical + keys[0].fmr_length;
> + return 0;
> + }
> +
> + /* Fabricate an rmap entry for space occupied by the data dev */
> + error = xfs_getfsmap_helper(tp, info, &frec);
> + if (error)
> + return error;

Seeing as you report different fmr_device values for the data and rt
devices, I'd have though that you'd want the rt fsmappings to start at
fmr_physical == 0. But then I guess for the sb_rtstart > 0 case, the
rtblock values that get written into the bmbt have that rtstart value
added in, don't they?

--D

> + }
> +
> + start_rtb = xfs_daddr_to_rtb(mp, rtstart_daddr + keys[0].fmr_physical);
> + end_rtb = xfs_daddr_to_rtb(mp, rtstart_daddr +
> + min(eofs - 1, keys[1].fmr_physical));
>
> info->missing_owner = XFS_FMR_OWN_FREE;
>
> @@ -1004,22 +1026,40 @@ xfs_getfsmap_rtdev_rmapbt(
> }
> #endif /* CONFIG_XFS_RT */
>
> +static uint32_t
> +xfs_getfsmap_device(
> + struct xfs_mount *mp,
> + enum xfs_device dev)
> +{
> + if (mp->m_sb.sb_rtstart)
> + return dev;
> +
> + switch (dev) {
> + case XFS_DEV_DATA:
> + return new_encode_dev(mp->m_ddev_targp->bt_dev);
> + case XFS_DEV_LOG:
> + return new_encode_dev(mp->m_logdev_targp->bt_dev);
> + case XFS_DEV_RT:
> + if (!mp->m_rtdev_targp)
> + break;
> + return new_encode_dev(mp->m_rtdev_targp->bt_dev);
> + }
> +
> + return -1;
> +}
> +
> /* Do we recognize the device? */
> STATIC bool
> xfs_getfsmap_is_valid_device(
> struct xfs_mount *mp,
> struct xfs_fsmap *fm)
> {
> - if (fm->fmr_device == 0 || fm->fmr_device == UINT_MAX ||
> - fm->fmr_device == new_encode_dev(mp->m_ddev_targp->bt_dev))
> - return true;
> - if (mp->m_logdev_targp &&
> - fm->fmr_device == new_encode_dev(mp->m_logdev_targp->bt_dev))
> - return true;
> - if (mp->m_rtdev_targp &&
> - fm->fmr_device == new_encode_dev(mp->m_rtdev_targp->bt_dev))
> - return true;
> - return false;
> + return fm->fmr_device == 0 ||
> + fm->fmr_device == UINT_MAX ||
> + fm->fmr_device == xfs_getfsmap_device(mp, XFS_DEV_DATA) ||
> + fm->fmr_device == xfs_getfsmap_device(mp, XFS_DEV_LOG) ||
> + (mp->m_rtdev_targp &&
> + fm->fmr_device == xfs_getfsmap_device(mp, XFS_DEV_RT));
> }
>
> /* Ensure that the low key is less than the high key. */
> @@ -1126,7 +1166,7 @@ xfs_getfsmap(
> /* Set up our device handlers. */
> memset(handlers, 0, sizeof(handlers));
> handlers[0].nr_sectors = XFS_FSB_TO_BB(mp, mp->m_sb.sb_dblocks);
> - handlers[0].dev = new_encode_dev(mp->m_ddev_targp->bt_dev);
> + handlers[0].dev = xfs_getfsmap_device(mp, XFS_DEV_DATA);
> if (use_rmap)
> handlers[0].fn = xfs_getfsmap_datadev_rmapbt;
> else
> @@ -1134,7 +1174,7 @@ xfs_getfsmap(
> if (mp->m_logdev_targp != mp->m_ddev_targp) {
> handlers[1].nr_sectors = XFS_FSB_TO_BB(mp,
> mp->m_sb.sb_logblocks);
> - handlers[1].dev = new_encode_dev(mp->m_logdev_targp->bt_dev);
> + handlers[1].dev = xfs_getfsmap_device(mp, XFS_DEV_LOG);
> handlers[1].fn = xfs_getfsmap_logdev;
> }
> #ifdef CONFIG_XFS_RT
> @@ -1144,7 +1184,7 @@ xfs_getfsmap(
> */
> if (mp->m_rtdev_targp && (use_rmap || !xfs_has_zoned(mp))) {
> handlers[2].nr_sectors = XFS_FSB_TO_BB(mp, mp->m_sb.sb_rblocks);
> - handlers[2].dev = new_encode_dev(mp->m_rtdev_targp->bt_dev);
> + handlers[2].dev = xfs_getfsmap_device(mp, XFS_DEV_RT);
> if (use_rmap)
> handlers[2].fn = xfs_getfsmap_rtdev_rmapbt;
> else
> @@ -1234,7 +1274,13 @@ xfs_getfsmap(
>
> if (tp)
> xfs_trans_cancel(tp);
> - head->fmh_oflags = FMH_OF_DEV_T;
> +
> + /*
> + * For internal RT device we need to report different synthetic devices
> + * for a single physical device, and thus can't report the actual dev_t.
> + */
> + if (!mp->m_sb.sb_rtstart)
> + head->fmh_oflags = FMH_OF_DEV_T;
> return error;
> }
>
> --
> 2.45.2
>
>