Re: [PATCH 2/5] xfs: introduce 'fixed agfl' feature

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Wed, Jan 17, 2018 at 04:07:23PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Introduce a new rocompat flag "FIXED_AGFL" to indicate that we've
> scanned and verified that the AGFL does not hit the troublesome last
> element of the AGFL array, and update xfs_agfl_size to its new
> definition based on this feature.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_format.h |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index bb42a11..3befc92f 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -464,6 +464,7 @@ xfs_sb_has_compat_feature(
>  #define XFS_SB_FEAT_RO_COMPAT_FINOBT   (1 << 0)		/* free inode btree */
>  #define XFS_SB_FEAT_RO_COMPAT_RMAPBT   (1 << 1)		/* reverse map btree */
>  #define XFS_SB_FEAT_RO_COMPAT_REFLINK  (1 << 2)		/* reflinked files */
> +#define XFS_SB_FEAT_RO_COMPAT_FIXED_AGFL (1 << 3)	/* fixed agfl */

(echoing comments made on irc by myself & Dave)

It suddenly occurred to me that we could do without this rocompat
feature if we simply don't allow wrapped AGFLs anymore.  As soon as
fllast points to that last agfl_bno slot, we simply move the whole list
back to the start of the agfl block (i.e. flfirst = 0) and relog the
whole agfl.  We still have problems if the admin mounts with an
unpatched kernel, gets the agfl to wrap, and then then moves to another
unpatched kernel, but at least the up-to-date kernels can just fix the
problem and move on.

I guess the nice thing about the rocompat feature is that the unpatched
kernels won't mount, instead of blowing up randomly some time in the
future and the user's antiquarian xfsprogs also refuses to touch it.

We could also only set the rocompat flag if we actually had to fix
something (vs. now where we always set it) though I wonder if that just
makes the "I reverted kernels and now it blew up wtf?!" even more
sporadic?  Hm.  Maybe we want that, since either is a rude awakening &
making the rude wakeup less frequent isn't a bad thing.

Ugh.  Maybe bikeshed now? :)

--D

>  #define XFS_SB_FEAT_RO_COMPAT_ALL \
>  		(XFS_SB_FEAT_RO_COMPAT_FINOBT | \
>  		 XFS_SB_FEAT_RO_COMPAT_RMAPBT | \
> @@ -566,6 +567,12 @@ static inline bool xfs_sb_version_hasreflink(struct xfs_sb *sbp)
>  		(sbp->sb_features_ro_compat & XFS_SB_FEAT_RO_COMPAT_REFLINK);
>  }
>  
> +static inline bool xfs_sb_version_hasfixedagfl(struct xfs_sb *sbp)
> +{
> +	return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 &&
> +		(sbp->sb_features_ro_compat & XFS_SB_FEAT_RO_COMPAT_FIXED_AGFL);
> +}
> +
>  /*
>   * end of superblock version macros
>   */
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html