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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Krisman,

Thanks for the feedback!

Em 03/09/2024 13:15, Gabriel Krisman Bertazi escreveu:
André Almeida <andrealmeid@xxxxxxxxxx> 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@xxxxxxxxxx>

[...]

+
+	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;
}




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux