Re: [PATCH 19/43] xfs: disable sb_frextents for zoned file systems

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

On Wed, Dec 11, 2024 at 09:54:44AM +0100, Christoph Hellwig wrote:
> Zoned file systems not only don't use the global frextents counter, but
> for them the in-memory percpu counter also includes reservations taken
> before even allocating delalloc extent records, so it will never match
> the per-zone used information.  Disable all updates and verification of
> the sb counter for zoned file systems as it isn't useful for them.

How is XC_FREE_RTEXTENTS initialized at mount time, then?

/me peeks ahead.

Oh, it and XC_FREE_RTAVAILABLE are set in xfs_mount_zones from values
that are computed by querying the hardware zone write pointers (or its
software equivalents if the rt device isn't zoned).  So the two incore
free rt space counters are completely detached from the ondisk
superblock counters.

What should be the value of sb_frextents, then?  Zero?  Please specify
that in the definition of xfs_dsb and update the verifiers to reject
nonzero values.

--D

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_sb.c           |  2 +-
>  fs/xfs/scrub/fscounters.c        | 11 +++++++++--
>  fs/xfs/scrub/fscounters_repair.c | 10 ++++++----
>  fs/xfs/xfs_mount.c               |  2 +-
>  fs/xfs/xfs_super.c               |  4 +++-
>  5 files changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index 6fc21c0a332b..ee56fc22fd06 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -1305,7 +1305,7 @@ xfs_log_sb(
>  	 * we handle nearly-lockless reservations, so we must use the _positive
>  	 * variant here to avoid writing out nonsense frextents.
>  	 */
> -	if (xfs_has_rtgroups(mp)) {
> +	if (xfs_has_rtgroups(mp) && !xfs_has_zoned(mp)) {
>  		mp->m_sb.sb_frextents =
>  			xfs_sum_freecounter(mp, XC_FREE_RTEXTENTS);
>  	}
> diff --git a/fs/xfs/scrub/fscounters.c b/fs/xfs/scrub/fscounters.c
> index 732658a62a2d..5f5f67947440 100644
> --- a/fs/xfs/scrub/fscounters.c
> +++ b/fs/xfs/scrub/fscounters.c
> @@ -413,7 +413,13 @@ xchk_fscount_count_frextents(
>  
>  	fsc->frextents = 0;
>  	fsc->frextents_delayed = 0;
> -	if (!xfs_has_realtime(mp))
> +
> +	/*
> +	 * Don't bother verifying and repairing the fs counters for zoned file
> +	 * systems as they don't track an on-disk frextents count, and the
> +	 * in-memory percpu counter also includes reservations.
> +	 */
> +	if (!xfs_has_realtime(mp) || xfs_has_zoned(mp))
>  		return 0;
>  
>  	while ((rtg = xfs_rtgroup_next(mp, rtg))) {
> @@ -597,7 +603,8 @@ xchk_fscounters(
>  			try_again = true;
>  	}
>  
> -	if (!xchk_fscount_within_range(sc, frextents,
> +	if (!xfs_has_zoned(mp) &&
> +	    !xchk_fscount_within_range(sc, frextents,
>  			&mp->m_free[XC_FREE_RTEXTENTS],
>  			fsc->frextents - fsc->frextents_delayed)) {
>  		if (fsc->frozen)
> diff --git a/fs/xfs/scrub/fscounters_repair.c b/fs/xfs/scrub/fscounters_repair.c
> index 8fb0db78489e..f0d2b04644e4 100644
> --- a/fs/xfs/scrub/fscounters_repair.c
> +++ b/fs/xfs/scrub/fscounters_repair.c
> @@ -74,10 +74,12 @@ xrep_fscounters(
>  	 * track of the delalloc reservations separately, as they are are
>  	 * subtracted from m_frextents, but not included in sb_frextents.
>  	 */
> -	xfs_set_freecounter(mp, XC_FREE_RTEXTENTS,
> -		fsc->frextents - fsc->frextents_delayed);
> -	if (!xfs_has_rtgroups(mp))
> -		mp->m_sb.sb_frextents = fsc->frextents;
> +	if (!xfs_has_zoned(mp)) {
> +		xfs_set_freecounter(mp, XC_FREE_RTEXTENTS,
> +				fsc->frextents - fsc->frextents_delayed);
> +		if (!xfs_has_rtgroups(mp))
> +			mp->m_sb.sb_frextents = fsc->frextents;
> +	}
>  
>  	return 0;
>  }
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index db910ecc1ed4..72fa28263e14 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -551,7 +551,7 @@ xfs_check_summary_counts(
>  	 * If we're mounting the rt volume after recovering the log, recompute
>  	 * frextents from the rtbitmap file to fix the inconsistency.
>  	 */
> -	if (xfs_has_realtime(mp) && !xfs_is_clean(mp)) {
> +	if (xfs_has_realtime(mp) && !xfs_has_zoned(mp) && !xfs_is_clean(mp)) {
>  		error = xfs_rtalloc_reinit_frextents(mp);
>  		if (error)
>  			return error;
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 18430e975c53..d0b7e0d02366 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1127,7 +1127,9 @@ xfs_reinit_percpu_counters(
>  	percpu_counter_set(&mp->m_icount, mp->m_sb.sb_icount);
>  	percpu_counter_set(&mp->m_ifree, mp->m_sb.sb_ifree);
>  	xfs_set_freecounter(mp, XC_FREE_BLOCKS, mp->m_sb.sb_fdblocks);
> -	xfs_set_freecounter(mp, XC_FREE_RTEXTENTS, mp->m_sb.sb_frextents);
> +	if (!xfs_has_zoned(mp))
> +		xfs_set_freecounter(mp, XC_FREE_RTEXTENTS,
> +				mp->m_sb.sb_frextents);
>  }
>  
>  static void
> -- 
> 2.45.2
> 
>