Re: [PATCH 08/10] xfs: open code sb verifier feature checks
From: Jan Tulak <hidden>
Date: 2018-08-21 16:21:37
On Mon, Aug 20, 2018 at 6:49 AM Dave Chinner [off-list ref] wrote:
quoted hunk ↗ jump to hunk
From: Dave Chinner <redacted> The superblock verifiers are one of the last places that use the sb version functions to do feature checks. This are all quite simple uses, and there aren't many of them so open code them all. Also, move the good version number check into xfs_sb.c instead of it being an inline function in xfs_format.h Signed-off-by: Dave Chinner <redacted> --- fs/xfs/libxfs/xfs_format.h | 26 --------- fs/xfs/libxfs/xfs_sb.c | 116 +++++++++++++++++++++++++------------ fs/xfs/libxfs/xfs_sb.h | 1 + 3 files changed, 81 insertions(+), 62 deletions(-)diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h index e21c39b13890..1f1107892dcd 100644 --- a/fs/xfs/libxfs/xfs_format.h +++ b/fs/xfs/libxfs/xfs_format.h@@ -280,32 +280,6 @@ typedef struct xfs_dsb { #define XFS_SB_VERSION_NUM(sbp) ((sbp)->sb_versionnum & XFS_SB_VERSION_NUMBITS) -/* - * The first XFS version we support is a v4 superblock with V2 directories. - */ -static inline bool xfs_sb_good_v4_features(struct xfs_sb *sbp) -{ - if (!(sbp->sb_versionnum & XFS_SB_VERSION_DIRV2BIT)) - return false; - - /* check for unknown features in the fs */ - if ((sbp->sb_versionnum & ~XFS_SB_VERSION_OKBITS) || - ((sbp->sb_versionnum & XFS_SB_VERSION_MOREBITSBIT) && - (sbp->sb_features2 & ~XFS_SB_VERSION2_OKBITS))) - return false; - - return true; -} - -static inline bool xfs_sb_good_version(struct xfs_sb *sbp) -{ - if (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5) - return true; - if (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_4) - return xfs_sb_good_v4_features(sbp); - return false; -} -
As I understand this removed code, if, in the future, we would have e.g. superblock v6, then xfs_sb_good_version would return false, right? Which I think is not correctly replicated in the new function below.
quoted hunk ↗ jump to hunk
static inline bool xfs_sb_version_hasrealtime(struct xfs_sb *sbp) { return sbp->sb_rblocks > 0;diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c index bedf6c6bf990..b83cf8adca1a 100644 --- a/fs/xfs/libxfs/xfs_sb.c +++ b/fs/xfs/libxfs/xfs_sb.c@@ -96,6 +96,35 @@ xfs_perag_put( trace_xfs_perag_put(pag->pag_mount, pag->pag_agno, ref, _RET_IP_); } +/* + * We support all XFS versions newer than a v4 superblock with V2 directories. + */ +bool +xfs_sb_good_version( + struct xfs_sb *sbp) +{ + /* all v5 filesystems are supported */ + if (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5) + return true; + + /* versions prior to v4 are not supported */ + if (XFS_SB_VERSION_NUM(sbp) < XFS_SB_VERSION_4) + return false; + + /* V4 filesystems need v2 directories */ + if (!(sbp->sb_versionnum & XFS_SB_VERSION_DIRV2BIT)) + return false; + + /* And must not have any unknown v4 feature bits set */ + if ((sbp->sb_versionnum & ~XFS_SB_VERSION_OKBITS) || + ((sbp->sb_versionnum & XFS_SB_VERSION_MOREBITSBIT) && + (sbp->sb_features2 & ~XFS_SB_VERSION2_OKBITS))) + return false; + + /* It's a supported v4 filesystem */ + return true; +} +
If we call this xfs_sb_good_version() with superblock v6 or higher, it returns true too, not only for a supported v4. Unless the V4 specific checks (v2 directories and feature bits) somehow implicitly prevents that from happening, which is something I can't tell. :-) Cheers, Jan -- Jan Tulak jtulak@redhat.com / jan@tulak.me