1. Patchew
  2. linux
  3. [PATCH v2 0/8] tmpfs: Add case-insesitive support for tmpfs
  4. View patch

[PATCH v2 6/8] tmpfs: Add flag FS_CASEFOLD_FL support for tmpfs dirs

André Almeida posted 8 patches 1 year, 3 months ago
[PATCH v2 6/8] tmpfs: Add flag FS_CASEFOLD_FL support for tmpfs dirs
Posted by André Almeida 1 year, 3 months ago 
Enable setting flag FS_CASEFOLD_FL for tmpfs directories, when tmpfs is
mounted with casefold support. A special check is need for this flag,
since it can't be set for non-empty directories.

Signed-off-by: André Almeida <andrealmeid@igalia.com>
---
 include/linux/shmem_fs.h |  6 +++---
 mm/shmem.c               | 40 +++++++++++++++++++++++++++++++++-------
 2 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/include/linux/shmem_fs.h b/include/linux/shmem_fs.h
index 1d06b1e5408a..8367ca2b99d9 100644
--- a/include/linux/shmem_fs.h
+++ b/include/linux/shmem_fs.h
@@ -42,10 +42,10 @@ struct shmem_inode_info {
 	struct inode		vfs_inode;
 };
 
-#define SHMEM_FL_USER_VISIBLE		FS_FL_USER_VISIBLE
+#define SHMEM_FL_USER_VISIBLE		(FS_FL_USER_VISIBLE | FS_CASEFOLD_FL)
 #define SHMEM_FL_USER_MODIFIABLE \
-	(FS_IMMUTABLE_FL | FS_APPEND_FL | FS_NODUMP_FL | FS_NOATIME_FL)
-#define SHMEM_FL_INHERITED		(FS_NODUMP_FL | FS_NOATIME_FL)
+	(FS_IMMUTABLE_FL | FS_APPEND_FL | FS_NODUMP_FL | FS_NOATIME_FL | FS_CASEFOLD_FL)
+#define SHMEM_FL_INHERITED		(FS_NODUMP_FL | FS_NOATIME_FL | FS_CASEFOLD_FL)
 
 struct shmem_quota_limits {
 	qsize_t usrquota_bhardlimit; /* Default user quota block hard limit */
diff --git a/mm/shmem.c b/mm/shmem.c
index 0f918010bc54..9a0fc7636629 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2617,9 +2617,26 @@ static int shmem_initxattrs(struct inode *, const struct xattr *, void *);
  * chattr's fsflags are unrelated to extended attributes,
  * but tmpfs has chosen to enable them under the same config option.
  */
-static void shmem_set_inode_flags(struct inode *inode, unsigned int fsflags)
+static int shmem_set_inode_flags(struct inode *inode, unsigned int fsflags, struct dentry *dentry)
 {
-	unsigned int i_flags = 0;
+	unsigned int i_flags = 0, old = inode->i_flags;
+	struct super_block *sb = inode->i_sb;
+
+	if (fsflags & FS_CASEFOLD_FL) {
+		if (!sb->s_encoding)
+			return -EOPNOTSUPP;
+
+		if (!S_ISDIR(inode->i_mode))
+			return -ENOTDIR;
+
+		if (dentry && !simple_empty(dentry))
+			return -ENOTEMPTY;
+
+		i_flags |= S_CASEFOLD;
+	} else if (old & S_CASEFOLD) {
+		if (dentry && !simple_empty(dentry))
+			return -ENOTEMPTY;
+	}
 
 	if (fsflags & FS_NOATIME_FL)
 		i_flags |= S_NOATIME;
@@ -2630,10 +2647,12 @@ static void shmem_set_inode_flags(struct inode *inode, unsigned int fsflags)
 	/*
 	 * But FS_NODUMP_FL does not require any action in i_flags.
 	 */
-	inode_set_flags(inode, i_flags, S_NOATIME | S_APPEND | S_IMMUTABLE);
+	inode_set_flags(inode, i_flags, S_NOATIME | S_APPEND | S_IMMUTABLE | S_CASEFOLD);
+
+	return 0;
 }
 #else
-static void shmem_set_inode_flags(struct inode *inode, unsigned int fsflags)
+static void shmem_set_inode_flags(struct inode *inode, unsigned int fsflags, struct dentry *dentry)
 {
 }
 #define shmem_initxattrs NULL
@@ -2680,7 +2699,7 @@ static struct inode *__shmem_get_inode(struct mnt_idmap *idmap,
 	info->fsflags = (dir == NULL) ? 0 :
 		SHMEM_I(dir)->fsflags & SHMEM_FL_INHERITED;
 	if (info->fsflags)
-		shmem_set_inode_flags(inode, info->fsflags);
+		shmem_set_inode_flags(inode, info->fsflags, NULL);
 	INIT_LIST_HEAD(&info->shrinklist);
 	INIT_LIST_HEAD(&info->swaplist);
 	simple_xattrs_init(&info->xattrs);
@@ -3790,16 +3809,23 @@ static int shmem_fileattr_set(struct mnt_idmap *idmap,
 {
 	struct inode *inode = d_inode(dentry);
 	struct shmem_inode_info *info = SHMEM_I(inode);
+	int ret, flags;
 
 	if (fileattr_has_fsx(fa))
 		return -EOPNOTSUPP;
 	if (fa->flags & ~SHMEM_FL_USER_MODIFIABLE)
 		return -EOPNOTSUPP;
 
-	info->fsflags = (info->fsflags & ~SHMEM_FL_USER_MODIFIABLE) |
+	flags = (info->fsflags & ~SHMEM_FL_USER_MODIFIABLE) |
 		(fa->flags & SHMEM_FL_USER_MODIFIABLE);
 
-	shmem_set_inode_flags(inode, info->fsflags);
+	ret = shmem_set_inode_flags(inode, flags, dentry);
+
+	if (ret)
+		return ret;
+
+	info->fsflags = flags;
+
 	inode_set_ctime_current(inode);
 	inode_inc_iversion(inode);
 	return 0;
-- 
2.46.0
Re: [PATCH v2 6/8] tmpfs: Add flag FS_CASEFOLD_FL support for tmpfs dirs
Posted by Gabriel Krisman Bertazi 1 year, 3 months ago 
André Almeida <andrealmeid@igalia.com> writes:

> Enable setting flag FS_CASEFOLD_FL for tmpfs directories, when tmpfs is
> mounted with casefold support. A special check is need for this flag,
> since it can't be set for non-empty directories.
>
> Signed-off-by: André Almeida <andrealmeid@igalia.com>
> ---
>  include/linux/shmem_fs.h |  6 +++---
>  mm/shmem.c               | 40 +++++++++++++++++++++++++++++++++-------
>  2 files changed, 36 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/shmem_fs.h b/include/linux/shmem_fs.h
> index 1d06b1e5408a..8367ca2b99d9 100644
> --- a/include/linux/shmem_fs.h
> +++ b/include/linux/shmem_fs.h
> @@ -42,10 +42,10 @@ struct shmem_inode_info {
>  	struct inode		vfs_inode;
>  };
>  
> -#define SHMEM_FL_USER_VISIBLE		FS_FL_USER_VISIBLE
> +#define SHMEM_FL_USER_VISIBLE		(FS_FL_USER_VISIBLE | FS_CASEFOLD_FL)
>  #define SHMEM_FL_USER_MODIFIABLE \
> -	(FS_IMMUTABLE_FL | FS_APPEND_FL | FS_NODUMP_FL | FS_NOATIME_FL)
> -#define SHMEM_FL_INHERITED		(FS_NODUMP_FL | FS_NOATIME_FL)
> +	(FS_IMMUTABLE_FL | FS_APPEND_FL | FS_NODUMP_FL | FS_NOATIME_FL | FS_CASEFOLD_FL)
> +#define SHMEM_FL_INHERITED		(FS_NODUMP_FL | FS_NOATIME_FL | FS_CASEFOLD_FL)
>  
>  struct shmem_quota_limits {
>  	qsize_t usrquota_bhardlimit; /* Default user quota block hard limit */
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 0f918010bc54..9a0fc7636629 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -2617,9 +2617,26 @@ static int shmem_initxattrs(struct inode *, const struct xattr *, void *);
>   * chattr's fsflags are unrelated to extended attributes,
>   * but tmpfs has chosen to enable them under the same config option.
>   */
> -static void shmem_set_inode_flags(struct inode *inode, unsigned int fsflags)
> +static int shmem_set_inode_flags(struct inode *inode, unsigned int fsflags, struct dentry *dentry)
>  {
> -	unsigned int i_flags = 0;
> +	unsigned int i_flags = 0, old = inode->i_flags;
> +	struct super_block *sb = inode->i_sb;
> +
> +	if (fsflags & FS_CASEFOLD_FL) {
> +		if (!sb->s_encoding)
> +			return -EOPNOTSUPP;
> +
> +		if (!S_ISDIR(inode->i_mode))
> +			return -ENOTDIR;
> +
> +		if (dentry && !simple_empty(dentry))
> +			return -ENOTEMPTY;
> +
> +		i_flags |= S_CASEFOLD;
> +	} else if (old & S_CASEFOLD) {
> +		if (dentry && !simple_empty(dentry))
> +			return -ENOTEMPTY;

We don't want to fail if a directory already has the S_CASEFOLD
flag and we are not flipping it in the current operation.  Something like:

if ((fsflags ^ old) & S_CASEFOLD) {
	if (!sb->s_encoding)
		return -EOPNOTSUPP;

	if (!S_ISDIR(inode->i_mode))
		return -ENOTDIR;

	if (dentry && !simple_empty(dentry))
		return -ENOTEMPTY;
        i_flags |= fsflags & S_CASEFOLD;
}

>
>  	if (fsflags & FS_NOATIME_FL)
>  		i_flags |= S_NOATIME;
> @@ -2630,10 +2647,12 @@ static void shmem_set_inode_flags(struct inode *inode, unsigned int fsflags)
>  	/*
>  	 * But FS_NODUMP_FL does not require any action in i_flags.
>  	 */
> -	inode_set_flags(inode, i_flags, S_NOATIME | S_APPEND | S_IMMUTABLE);
> +	inode_set_flags(inode, i_flags, S_NOATIME | S_APPEND | S_IMMUTABLE | S_CASEFOLD);
> +
> +	return 0;
>  }
>  #else
> -static void shmem_set_inode_flags(struct inode *inode, unsigned int fsflags)
> +static void shmem_set_inode_flags(struct inode *inode, unsigned int fsflags, struct dentry *dentry)
>  {
>  }
>  #define shmem_initxattrs NULL
> @@ -2680,7 +2699,7 @@ static struct inode *__shmem_get_inode(struct mnt_idmap *idmap,
>  	info->fsflags = (dir == NULL) ? 0 :
>  		SHMEM_I(dir)->fsflags & SHMEM_FL_INHERITED;
>  	if (info->fsflags)
> -		shmem_set_inode_flags(inode, info->fsflags);
> +		shmem_set_inode_flags(inode, info->fsflags, NULL);
>  	INIT_LIST_HEAD(&info->shrinklist);
>  	INIT_LIST_HEAD(&info->swaplist);
>  	simple_xattrs_init(&info->xattrs);
> @@ -3790,16 +3809,23 @@ static int shmem_fileattr_set(struct mnt_idmap *idmap,
>  {
>  	struct inode *inode = d_inode(dentry);
>  	struct shmem_inode_info *info = SHMEM_I(inode);
> +	int ret, flags;
>  
>  	if (fileattr_has_fsx(fa))
>  		return -EOPNOTSUPP;
>  	if (fa->flags & ~SHMEM_FL_USER_MODIFIABLE)
>  		return -EOPNOTSUPP;
>  
> -	info->fsflags = (info->fsflags & ~SHMEM_FL_USER_MODIFIABLE) |
> +	flags = (info->fsflags & ~SHMEM_FL_USER_MODIFIABLE) |
>  		(fa->flags & SHMEM_FL_USER_MODIFIABLE);
>  
> -	shmem_set_inode_flags(inode, info->fsflags);
> +	ret = shmem_set_inode_flags(inode, flags, dentry);
> +
> +	if (ret)
> +		return ret;
> +
> +	info->fsflags = flags;
> +
>  	inode_set_ctime_current(inode);
>  	inode_inc_iversion(inode);
>  	return 0;

-- 
Gabriel Krisman Bertazi
Re: [PATCH v2 6/8] tmpfs: Add flag FS_CASEFOLD_FL support for tmpfs dirs
Posted by André Almeida 1 year, 3 months ago 
Hi Krisman,

Thanks for the feedback!

Em 03/09/2024 13:15, Gabriel Krisman Bertazi escreveu:
> André Almeida <andrealmeid@igalia.com> writes:
> 
>> Enable setting flag FS_CASEFOLD_FL for tmpfs directories, when tmpfs is
>> mounted with casefold support. A special check is need for this flag,
>> since it can't be set for non-empty directories.
>>
>> Signed-off-by: André Almeida <andrealmeid@igalia.com>

[...]

>> +
>> +	if (fsflags & FS_CASEFOLD_FL) {
>> +		if (!sb->s_encoding)
>> +			return -EOPNOTSUPP;
>> +
>> +		if (!S_ISDIR(inode->i_mode))
>> +			return -ENOTDIR;
>> +
>> +		if (dentry && !simple_empty(dentry))
>> +			return -ENOTEMPTY;
>> +
>> +		i_flags |= S_CASEFOLD;
>> +	} else if (old & S_CASEFOLD) {
>> +		if (dentry && !simple_empty(dentry))
>> +			return -ENOTEMPTY;
> 
> We don't want to fail if a directory already has the S_CASEFOLD
> flag and we are not flipping it in the current operation.  Something like:
> 
> if ((fsflags ^ old) & S_CASEFOLD) {
> 	if (!sb->s_encoding)
> 		return -EOPNOTSUPP;
> 
> 	if (!S_ISDIR(inode->i_mode))
> 		return -ENOTDIR;
> 
> 	if (dentry && !simple_empty(dentry))
> 		return -ENOTEMPTY;
>          i_flags |= fsflags & S_CASEFOLD;
> }
> 

You are right, it's broken and failing for directories with S_CASEFOLD. 
Here's a small test showing that we can't add the +d attribute to a 
non-empty CI folder (+d doesn't require the directory to be empty):

folder ) mkdir A
folder ) mkdir A/B
folder ) chattr +d A/B
folder ) chattr +d A
chattr: Directory not empty while setting flags on A

However, FS_CASEFOLD_FL != S_CASEFOLD and the set of values for 
inode->i_flags (var old) and fsflags aren't the same, so your proposed 
snippet didn't work. I see that ext4 has a very similar code as your 
proposal, but I think they do something different with the flag values.

I rewrote my code separating the three possible paths and it worked:

/* inheritance from parent dir/keeping the same flags path */
if ((fsflags & FS_CASEFOLD_FL) && (old & S_CASEFOLD))
	i_flags |= S_CASEFOLD;

/* removing flag path */
if (!(fsflags & FS_CASEFOLD_FL) && (old & S_CASEFOLD))
	if (dentry && !simple_empty(dentry))
		return -ENOTEMPTY;

/* adding flag path */
if ((fsflags & FS_CASEFOLD_FL) && !(old & S_CASEFOLD)) {
	if (!sb->s_encoding)
		return -EOPNOTSUPP;

	if (!S_ISDIR(inode->i_mode))
		return -ENOTDIR;

	if (dentry && !simple_empty(dentry))
		return -ENOTEMPTY;

	i_flags |= S_CASEFOLD;
}

In that way, the `chattr +d` call doesn't fall into the simple_empty() 
check. I simplified the code like this for the v3:

if (fsflags & FS_CASEFOLD_FL) {
	if (!(old & S_CASEFOLD)) {
		if (!sb->s_encoding)
			return -EOPNOTSUPP;

		if (!S_ISDIR(inode->i_mode))
			return -ENOTDIR;

		if (dentry && !simple_empty(dentry))
			return -ENOTEMPTY;
	}

	i_flags |= S_CASEFOLD;
} else if (old & S_CASEFOLD) {
	if (dentry && !simple_empty(dentry))
		return -ENOTEMPTY;
}
Re: [PATCH v2 6/8] tmpfs: Add flag FS_CASEFOLD_FL support for tmpfs dirs
Posted by kernel test robot 1 year, 3 months ago 
Hi André,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]
[also build test ERROR on tytso-ext4/dev brauner-vfs/vfs.all linus/master v6.11-rc6 next-20240903]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Andr-Almeida/unicode-Fix-utf8_load-error-path/20240903-070149
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20240902225511.757831-7-andrealmeid%40igalia.com
patch subject: [PATCH v2 6/8] tmpfs: Add flag FS_CASEFOLD_FL support for tmpfs dirs
config: i386-buildonly-randconfig-005-20240903 (https://download.01.org/0day-ci/archive/20240903/202409031620.BOshMDgn-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240903/202409031620.BOshMDgn-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409031620.BOshMDgn-lkp@intel.com/

All errors (new ones prefixed by >>):

>> mm/shmem.c:2771:12: error: no member named 's_encoding' in 'struct super_block'
    2771 |                 if (!sb->s_encoding)
         |                      ~~  ^
   1 error generated.


vim +2771 mm/shmem.c

  2760	
  2761	/*
  2762	 * chattr's fsflags are unrelated to extended attributes,
  2763	 * but tmpfs has chosen to enable them under the same config option.
  2764	 */
  2765	static int shmem_set_inode_flags(struct inode *inode, unsigned int fsflags, struct dentry *dentry)
  2766	{
  2767		unsigned int i_flags = 0, old = inode->i_flags;
  2768		struct super_block *sb = inode->i_sb;
  2769	
  2770		if (fsflags & FS_CASEFOLD_FL) {
> 2771			if (!sb->s_encoding)
  2772				return -EOPNOTSUPP;
  2773	
  2774			if (!S_ISDIR(inode->i_mode))
  2775				return -ENOTDIR;
  2776	
  2777			if (dentry && !simple_empty(dentry))
  2778				return -ENOTEMPTY;
  2779	
  2780			i_flags |= S_CASEFOLD;
  2781		} else if (old & S_CASEFOLD) {
  2782			if (dentry && !simple_empty(dentry))
  2783				return -ENOTEMPTY;
  2784		}
  2785	
  2786		if (fsflags & FS_NOATIME_FL)
  2787			i_flags |= S_NOATIME;
  2788		if (fsflags & FS_APPEND_FL)
  2789			i_flags |= S_APPEND;
  2790		if (fsflags & FS_IMMUTABLE_FL)
  2791			i_flags |= S_IMMUTABLE;
  2792		/*
  2793		 * But FS_NODUMP_FL does not require any action in i_flags.
  2794		 */
  2795		inode_set_flags(inode, i_flags, S_NOATIME | S_APPEND | S_IMMUTABLE | S_CASEFOLD);
  2796	
  2797		return 0;
  2798	}
  2799	#else
  2800	static void shmem_set_inode_flags(struct inode *inode, unsigned int fsflags, struct dentry *dentry)
  2801	{
  2802	}
  2803	#define shmem_initxattrs NULL
  2804	#endif
  2805	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH v2 6/8] tmpfs: Add flag FS_CASEFOLD_FL support for tmpfs dirs
Posted by kernel test robot 1 year, 3 months ago 
Hi André,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]
[also build test ERROR on tytso-ext4/dev brauner-vfs/vfs.all linus/master v6.11-rc6 next-20240903]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Andr-Almeida/unicode-Fix-utf8_load-error-path/20240903-070149
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20240902225511.757831-7-andrealmeid%40igalia.com
patch subject: [PATCH v2 6/8] tmpfs: Add flag FS_CASEFOLD_FL support for tmpfs dirs
config: i386-buildonly-randconfig-002-20240903 (https://download.01.org/0day-ci/archive/20240903/202409031642.6kP6Ra8c-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240903/202409031642.6kP6Ra8c-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409031642.6kP6Ra8c-lkp@intel.com/

All errors (new ones prefixed by >>):

   mm/shmem.c: In function 'shmem_set_inode_flags':
>> mm/shmem.c:2771:24: error: 'struct super_block' has no member named 's_encoding'
    2771 |                 if (!sb->s_encoding)
         |                        ^~


vim +2771 mm/shmem.c

  2760	
  2761	/*
  2762	 * chattr's fsflags are unrelated to extended attributes,
  2763	 * but tmpfs has chosen to enable them under the same config option.
  2764	 */
  2765	static int shmem_set_inode_flags(struct inode *inode, unsigned int fsflags, struct dentry *dentry)
  2766	{
  2767		unsigned int i_flags = 0, old = inode->i_flags;
  2768		struct super_block *sb = inode->i_sb;
  2769	
  2770		if (fsflags & FS_CASEFOLD_FL) {
> 2771			if (!sb->s_encoding)
  2772				return -EOPNOTSUPP;
  2773	
  2774			if (!S_ISDIR(inode->i_mode))
  2775				return -ENOTDIR;
  2776	
  2777			if (dentry && !simple_empty(dentry))
  2778				return -ENOTEMPTY;
  2779	
  2780			i_flags |= S_CASEFOLD;
  2781		} else if (old & S_CASEFOLD) {
  2782			if (dentry && !simple_empty(dentry))
  2783				return -ENOTEMPTY;
  2784		}
  2785	
  2786		if (fsflags & FS_NOATIME_FL)
  2787			i_flags |= S_NOATIME;
  2788		if (fsflags & FS_APPEND_FL)
  2789			i_flags |= S_APPEND;
  2790		if (fsflags & FS_IMMUTABLE_FL)
  2791			i_flags |= S_IMMUTABLE;
  2792		/*
  2793		 * But FS_NODUMP_FL does not require any action in i_flags.
  2794		 */
  2795		inode_set_flags(inode, i_flags, S_NOATIME | S_APPEND | S_IMMUTABLE | S_CASEFOLD);
  2796	
  2797		return 0;
  2798	}
  2799	#else
  2800	static void shmem_set_inode_flags(struct inode *inode, unsigned int fsflags, struct dentry *dentry)
  2801	{
  2802	}
  2803	#define shmem_initxattrs NULL
  2804	#endif
  2805	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.