Re: [PATCH 09/16] xfs: add a superblock feature bit to indicate UTF-8 support.

[Dave Chinner <david@fromorbit.com>]

On Fri, Oct 03, 2014 at 04:59:46PM -0500, Ben Myers wrote:
> From: Olaf Weber <olaf@sgi.com>
> 
> When UTF-8 support is enabled, the xfs_dir_ci_inode_operations must be
> installed. Add xfs_sb_version_hasci(), which tests both the borgbit and
> the utf8bit, and returns true if at least one of them is set. Replace
> calls to xfs_sb_version_hasasciici() as needed.
> 
> Signed-off-by: Olaf Weber <olaf@sgi.com>
> ---
>  fs/xfs/libxfs/xfs_sb.h | 24 +++++++++++++++++++++++-
>  fs/xfs/xfs_fs.h        |  1 +
>  fs/xfs/xfs_fsops.c     |  4 +++-
>  fs/xfs/xfs_iops.c      |  4 ++--
>  4 files changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_sb.h b/fs/xfs/libxfs/xfs_sb.h
> index 2e73970..525eacb 100644
> --- a/fs/xfs/libxfs/xfs_sb.h
> +++ b/fs/xfs/libxfs/xfs_sb.h
> @@ -70,6 +70,7 @@ struct xfs_trans;
>  #define XFS_SB_VERSION2_RESERVED4BIT	0x00000004
>  #define XFS_SB_VERSION2_ATTR2BIT	0x00000008	/* Inline attr rework */
>  #define XFS_SB_VERSION2_PARENTBIT	0x00000010	/* parent pointers */
> +#define XFS_SB_VERSION2_UTF8BIT		0x00000020      /* utf8 names */
>  #define XFS_SB_VERSION2_PROJID32BIT	0x00000080	/* 32 bit project id */

Can you explain why this bit is safe to use? I don't recall why
XFS_SB_VERSION2_PROJID32BIT skipped several bits because there
aren't any comments explaining why that value was chosen. Adding a
comment about the 0x00000040 bit at the same time would be useful.

>  #define XFS_SB_VERSION2_CRCBIT		0x00000100	/* metadata CRCs */
>  #define XFS_SB_VERSION2_FTYPE		0x00000200	/* inode type in dir */
> @@ -77,6 +78,7 @@ struct xfs_trans;
>  #define	XFS_SB_VERSION2_OKBITS		\
>  	(XFS_SB_VERSION2_LAZYSBCOUNTBIT	| \
>  	 XFS_SB_VERSION2_ATTR2BIT	| \
> +	 XFS_SB_VERSION2_UTF8BIT	| \
>  	 XFS_SB_VERSION2_PROJID32BIT	| \
>  	 XFS_SB_VERSION2_FTYPE)
>  
> @@ -509,8 +511,10 @@ xfs_sb_has_ro_compat_feature(
>  }
>  
>  #define XFS_SB_FEAT_INCOMPAT_FTYPE	(1 << 0)	/* filetype in dirent */
> +#define XFS_SB_FEAT_INCOMPAT_UTF8	(1 << 1)	/* utf-8 name support */
>  #define XFS_SB_FEAT_INCOMPAT_ALL \
> -		(XFS_SB_FEAT_INCOMPAT_FTYPE)
> +		(XFS_SB_FEAT_INCOMPAT_FTYPE | \
> +		 XFS_SB_FEAT_INCOMPAT_UTF8)

Don't add support to the filesystem until all the supporting
code is in place. This avoids git bisects landing on commits in the
series where the filesystem says it supports the feature bit it
doesn't actually work. Add a patch at the end of the series that
adds these bits to the feature masks.

>  
>  #define XFS_SB_FEAT_INCOMPAT_UNKNOWN	~XFS_SB_FEAT_INCOMPAT_ALL
>  static inline bool
> @@ -558,6 +562,24 @@ static inline int xfs_sb_version_hasfinobt(xfs_sb_t *sbp)
>  		(sbp->sb_features_ro_compat & XFS_SB_FEAT_RO_COMPAT_FINOBT);
>  }
>  
> +static inline int xfs_sb_version_hasutf8(xfs_sb_t *sbp)

bool, no typedefs.

> +{
> +	return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 &&
> +		xfs_sb_has_incompat_feature(sbp, XFS_SB_FEAT_INCOMPAT_UTF8)) ||
> +		(xfs_sb_version_hasmorebits(sbp) &&
> +		(sbp->sb_features2 & XFS_SB_VERSION2_UTF8BIT));

xfs_sb_version_hasmorebits() already checks for XFS_SB_VERSION_5,
so this could be:

	return xfs_sb_version_hasmorebits(sbp) &&
		(xfs_sb_has_incompat_feature(sbp, XFS_SB_FEAT_INCOMPAT_UTF8) ||
		 (sbp->sb_features2 & XFS_SB_VERSION2_UTF8BIT));


> +}
> +
> +/*
> + * Special case: there are a number of places where we need to test
> + * both the borgbit and the utf8bit, and take the same action if
> + * either of those is set.
> + */
> +static inline int xfs_sb_version_hasci(xfs_sb_t *sbp)
> +{

bool, no typedefs, and probably should be a separate patch.


-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs