Re: [PATCH 5/6] xfs: update sb field checks when metadir is turned on

Dave Chinner
2024-08-26T09:52:47Z

On Thu, Aug 22, 2024 at 05:29:15PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> When metadir is enabled, we want to check the two new rtgroups fields,
> and we don't want to check the old inumbers that are now in the metadir.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> fs/xfs/scrub/agheader.c | 36 ++++++++++++++++++++++++------------
> 1 file changed, 24 insertions(+), 12 deletions(-)
>
>
> diff --git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
> index cad997f38a424..0d22d70950a5c 100644
> --- a/fs/xfs/scrub/agheader.c
> +++ b/fs/xfs/scrub/agheader.c
> @@ -147,14 +147,14 @@ xchk_superblock(
> if (xfs_has_metadir(sc->mp)) {
> if (sb->sb_metadirino != cpu_to_be64(mp->m_sb.sb_metadirino))
> xchk_block_set_preen(sc, bp);
> + } else {
> + if (sb->sb_rbmino != cpu_to_be64(mp->m_sb.sb_rbmino))
> + xchk_block_set_preen(sc, bp);
> +
> + if (sb->sb_rsumino != cpu_to_be64(mp->m_sb.sb_rsumino))
> + xchk_block_set_preen(sc, bp);
> }
>
> - if (sb->sb_rbmino != cpu_to_be64(mp->m_sb.sb_rbmino))
> - xchk_block_set_preen(sc, bp);
> -
> - if (sb->sb_rsumino != cpu_to_be64(mp->m_sb.sb_rsumino))
> - xchk_block_set_preen(sc, bp);
> -

If metadir is enabled, then shouldn't sb->sb_rbmino/sb_rsumino both
be NULLFSINO to indicate they aren't valid?

Given the rt inodes should have a well defined value even when
metadir is enabled, I would say the current code that is validating
the values are consistent with the primary across all secondary
superblocks is correct and this change is unnecessary....

> @@ -229,11 +229,13 @@ xchk_superblock(
> * sb_icount, sb_ifree, sb_fdblocks, sb_frexents
> */
>
> - if (sb->sb_uquotino != cpu_to_be64(mp->m_sb.sb_uquotino))
> - xchk_block_set_preen(sc, bp);
> + if (!xfs_has_metadir(mp)) {
> + if (sb->sb_uquotino != cpu_to_be64(mp->m_sb.sb_uquotino))
> + xchk_block_set_preen(sc, bp);
>
> - if (sb->sb_gquotino != cpu_to_be64(mp->m_sb.sb_gquotino))
> - xchk_block_set_preen(sc, bp);
> + if (sb->sb_gquotino != cpu_to_be64(mp->m_sb.sb_gquotino))
> + xchk_block_set_preen(sc, bp);
> + }

Same - if metadir is in use and quota inodes are in the metadir,
then the superblock quota inodes should be NULLFSINO....

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