[PATCH v3 0/9] fs/ntfs3: Use new mount api and change some opts

[PATCH v3 0/9] fs/ntfs3: Use new mount api and change some opts

[Kari Argillander]

See V2 if you want:
lore.kernel.org/ntfs3/20210819002633.689831-1-kari.argillander@gmail.com

NLS change is now blocked when remounting. Christoph also suggest that
we block all other mount options, but I have tested a couple and they
seem to work. I wish that we do not block any other than NLS because
in theory they should work. Also Konstantin can comment about this.

I have not include reviewed/acked to patch "Use new api for mounting"
because it change so much. I have also included three new patch to this
series:
	- Convert mount options to pointer in sbi
		So that we do not need to initiliaze whole spi in 
		remount.
	- Init spi more in init_fs_context than fill_super
		This is just refactoring. (Series does not depend on this)
	- Show uid/gid always in show_options()
		Christian Brauner kinda ask this. (Series does not depend
		on this)

Series is ones again tested with kvm-xfstests. Every commit is build
tested.

v3:
	- Add patch "Convert mount options to pointer in sbi"
	- Add patch "Init spi more in init_fs_context than fill_super"
	- Add patch "Show uid/gid always in show_options"
	- Patch "Use new api for mounting" has make over
	- NLS loading is not anymore possible when remounting
	- show_options() iocharset printing is fixed
	- Delete comment that testing should be done with other
	  mount options.
	- Add reviewed/acked-tags to 1,2,6,8 
	- Rewrite this cover
v2:
	- Rewrite this cover leter
	- Reorder noatime to first patch
	- NLS loading with string
	- Delete default_options function
	- Remove remount flags
	- Rename no_acl_rules mount option
	- Making code cleaner
	- Add comment that mount options should be tested

Kari Argillander (9):
  fs/ntfs3: Remove unnecesarry mount option noatime
  fs/ntfs3: Remove unnecesarry remount flag handling
  fs/ntfs3: Convert mount options to pointer in sbi
  fs/ntfs3: Use new api for mounting
  fs/ntfs3: Init spi more in init_fs_context than fill_super
  fs/ntfs3: Make mount option nohidden more universal
  fs/ntfs3: Add iocharset= mount option as alias for nls=
  fs/ntfs3: Rename mount option no_acl_rules > (no)acl_rules
  fs/ntfs3: Show uid/gid always in show_options()

 Documentation/filesystems/ntfs3.rst |  10 +-
 fs/ntfs3/attrib.c                   |   2 +-
 fs/ntfs3/dir.c                      |   8 +-
 fs/ntfs3/file.c                     |   4 +-
 fs/ntfs3/inode.c                    |  12 +-
 fs/ntfs3/ntfs_fs.h                  |  26 +-
 fs/ntfs3/super.c                    | 486 +++++++++++++++-------------
 fs/ntfs3/xattr.c                    |   2 +-
 8 files changed, 284 insertions(+), 266 deletions(-)

-- 
2.25.1

[PATCH v3 1/9] fs/ntfs3: Remove unnecesarry mount option noatime

[Kari Argillander]

Remove unnecesarry mount option noatime because this will be handled
by VFS. Our option parser will never get opt like this.

Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 Documentation/filesystems/ntfs3.rst | 4 ----
 fs/ntfs3/super.c                    | 7 -------
 2 files changed, 11 deletions(-)

diff --git a/Documentation/filesystems/ntfs3.rst b/Documentation/filesystems/ntfs3.rst
index ffe9ea0c1499..af7158de6fde 100644
--- a/Documentation/filesystems/ntfs3.rst
+++ b/Documentation/filesystems/ntfs3.rst
@@ -85,10 +85,6 @@ acl			Support POSIX ACLs (Access Control Lists). Effective if
 			supported by Kernel. Not to be confused with NTFS ACLs.
 			The option specified as acl enables support for POSIX ACLs.
 
-noatime			All files and directories will not update their last access
-			time attribute if a partition is mounted with this parameter.
-			This option can speed up file system operation.
-
 ===============================================================================
 
 ToDo list
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 17ee715ab539..267f123b0109 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -216,7 +216,6 @@ enum Opt {
 	Opt_nohidden,
 	Opt_showmeta,
 	Opt_acl,
-	Opt_noatime,
 	Opt_nls,
 	Opt_prealloc,
 	Opt_no_acs_rules,
@@ -235,7 +234,6 @@ static const match_table_t ntfs_tokens = {
 	{ Opt_sparse, "sparse" },
 	{ Opt_nohidden, "nohidden" },
 	{ Opt_acl, "acl" },
-	{ Opt_noatime, "noatime" },
 	{ Opt_showmeta, "showmeta" },
 	{ Opt_nls, "nls=%s" },
 	{ Opt_prealloc, "prealloc" },
@@ -326,9 +324,6 @@ static noinline int ntfs_parse_options(struct super_block *sb, char *options,
 			ntfs_err(sb, "support for ACL not compiled in!");
 			return -EINVAL;
 #endif
-		case Opt_noatime:
-			sb->s_flags |= SB_NOATIME;
-			break;
 		case Opt_showmeta:
 			opts->showmeta = 1;
 			break;
@@ -575,8 +570,6 @@ static int ntfs_show_options(struct seq_file *m, struct dentry *root)
 		seq_puts(m, ",prealloc");
 	if (sb->s_flags & SB_POSIXACL)
 		seq_puts(m, ",acl");
-	if (sb->s_flags & SB_NOATIME)
-		seq_puts(m, ",noatime");
 
 	return 0;
 }
-- 
2.25.1

[PATCH v3 2/9] fs/ntfs3: Remove unnecesarry remount flag handling

[Kari Argillander]

Remove unnecesarry remount flag handling. This does not do anything for
this driver. We have already set SB_NODIRATIME when we fill super. Also
noatime should be set from mount option. Now for some reson we try to
set it when remounting.

Lazytime part looks like it is copied from f2fs and there is own mount
parameter for it. That is why they use it. We do not set lazytime
anywhere in our code. So basically this just blocks lazytime when
remounting.

Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 fs/ntfs3/super.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 267f123b0109..c590872070e1 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -407,8 +407,6 @@ static int ntfs_remount(struct super_block *sb, int *flags, char *data)
 
 	clear_mount_options(&old_opts);
 
-	*flags = (*flags & ~SB_LAZYTIME) | (sb->s_flags & SB_LAZYTIME) |
-		 SB_NODIRATIME | SB_NOATIME;
 	ntfs_info(sb, "re-mounted. Opts: %s", orig_data);
 	err = 0;
 	goto out;
-- 
2.25.1

[PATCH v3 3/9] fs/ntfs3: Convert mount options to pointer in sbi

[Kari Argillander]

Use pointer to mount options. We want to do this because we will use new
mount api which will benefit that we have spi and mount options in
different allocations. When we remount we do not have to make whole new
spi it is enough that we will allocate just mount options.

Please note that we can do example remount lot cleaner but things will
change in next patch so this should be just functional.

Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 fs/ntfs3/attrib.c  |  2 +-
 fs/ntfs3/dir.c     |  8 ++++----
 fs/ntfs3/file.c    |  4 ++--
 fs/ntfs3/inode.c   | 12 ++++++------
 fs/ntfs3/ntfs_fs.h |  2 +-
 fs/ntfs3/super.c   | 31 +++++++++++++++++++------------
 fs/ntfs3/xattr.c   |  2 +-
 7 files changed, 34 insertions(+), 27 deletions(-)

diff --git a/fs/ntfs3/attrib.c b/fs/ntfs3/attrib.c
index 4eae9886e27d..7eb366a1b7c7 100644
--- a/fs/ntfs3/attrib.c
+++ b/fs/ntfs3/attrib.c
@@ -538,7 +538,7 @@ int attr_set_size(struct ntfs_inode *ni, enum ATTR_TYPE type,
 		} else if (pre_alloc == -1) {
 			pre_alloc = 0;
 			if (type == ATTR_DATA && !name_len &&
-			    sbi->options.prealloc) {
+			    sbi->options->prealloc) {
 				CLST new_alen2 = bytes_to_cluster(
 					sbi, get_pre_allocated(new_size));
 				pre_alloc = new_alen2 - new_alen;
diff --git a/fs/ntfs3/dir.c b/fs/ntfs3/dir.c
index d36d7fbc2b1d..e41e5a259be7 100644
--- a/fs/ntfs3/dir.c
+++ b/fs/ntfs3/dir.c
@@ -25,7 +25,7 @@ int ntfs_utf16_to_nls(struct ntfs_sb_info *sbi, const struct le_str *uni,
 	int ret, uni_len, warn;
 	const __le16 *ip;
 	u8 *op;
-	struct nls_table *nls = sbi->options.nls;
+	struct nls_table *nls = sbi->options->nls;
 
 	static_assert(sizeof(wchar_t) == sizeof(__le16));
 
@@ -183,7 +183,7 @@ int ntfs_nls_to_utf16(struct ntfs_sb_info *sbi, const u8 *name, u32 name_len,
 {
 	int ret, slen;
 	const u8 *end;
-	struct nls_table *nls = sbi->options.nls;
+	struct nls_table *nls = sbi->options->nls;
 	u16 *uname = uni->name;
 
 	static_assert(sizeof(wchar_t) == sizeof(u16));
@@ -296,10 +296,10 @@ static inline int ntfs_filldir(struct ntfs_sb_info *sbi, struct ntfs_inode *ni,
 		return 0;
 
 	/* Skip meta files ( unless option to show metafiles is set ) */
-	if (!sbi->options.showmeta && ntfs_is_meta_file(sbi, ino))
+	if (!sbi->options->showmeta && ntfs_is_meta_file(sbi, ino))
 		return 0;
 
-	if (sbi->options.nohidden && (fname->dup.fa & FILE_ATTRIBUTE_HIDDEN))
+	if (sbi->options->nohidden && (fname->dup.fa & FILE_ATTRIBUTE_HIDDEN))
 		return 0;
 
 	name_len = ntfs_utf16_to_nls(sbi, (struct le_str *)&fname->name_len,
diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c
index a959f6197c99..c79e4aff7a19 100644
--- a/fs/ntfs3/file.c
+++ b/fs/ntfs3/file.c
@@ -743,7 +743,7 @@ int ntfs3_setattr(struct user_namespace *mnt_userns, struct dentry *dentry,
 	umode_t mode = inode->i_mode;
 	int err;
 
-	if (sbi->options.no_acs_rules) {
+	if (sbi->options->no_acs_rules) {
 		/* "no access rules" - force any changes of time etc. */
 		attr->ia_valid |= ATTR_FORCE;
 		/* and disable for editing some attributes */
@@ -1189,7 +1189,7 @@ static int ntfs_file_release(struct inode *inode, struct file *file)
 	int err = 0;
 
 	/* if we are the last writer on the inode, drop the block reservation */
-	if (sbi->options.prealloc && ((file->f_mode & FMODE_WRITE) &&
+	if (sbi->options->prealloc && ((file->f_mode & FMODE_WRITE) &&
 				      atomic_read(&inode->i_writecount) == 1)) {
 		ni_lock(ni);
 		down_write(&ni->file.run_lock);
diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index 520471f35e29..b28dd111e6cf 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -51,8 +51,8 @@ static struct inode *ntfs_read_mft(struct inode *inode,
 
 	inode->i_op = NULL;
 	/* Setup 'uid' and 'gid' */
-	inode->i_uid = sbi->options.fs_uid;
-	inode->i_gid = sbi->options.fs_gid;
+	inode->i_uid = sbi->options->fs_uid;
+	inode->i_gid = sbi->options->fs_gid;
 
 	err = mi_init(&ni->mi, sbi, ino);
 	if (err)
@@ -231,7 +231,7 @@ static struct inode *ntfs_read_mft(struct inode *inode,
 			t32 = le16_to_cpu(attr->nres.run_off);
 		}
 
-		mode = S_IFREG | (0777 & sbi->options.fs_fmask_inv);
+		mode = S_IFREG | (0777 & sbi->options->fs_fmask_inv);
 
 		if (!attr->non_res) {
 			ni->ni_flags |= NI_FLAG_RESIDENT;
@@ -274,7 +274,7 @@ static struct inode *ntfs_read_mft(struct inode *inode,
 			goto out;
 
 		mode = sb->s_root
-			       ? (S_IFDIR | (0777 & sbi->options.fs_dmask_inv))
+			       ? (S_IFDIR | (0777 & sbi->options->fs_dmask_inv))
 			       : (S_IFDIR | 0777);
 		goto next_attr;
 
@@ -439,7 +439,7 @@ static struct inode *ntfs_read_mft(struct inode *inode,
 		goto out;
 	}
 
-	if ((sbi->options.sys_immutable &&
+	if ((sbi->options->sys_immutable &&
 	     (std5->fa & FILE_ATTRIBUTE_SYSTEM)) &&
 	    !S_ISFIFO(mode) && !S_ISSOCK(mode) && !S_ISLNK(mode)) {
 		inode->i_flags |= S_IMMUTABLE;
@@ -1228,7 +1228,7 @@ struct inode *ntfs_create_inode(struct user_namespace *mnt_userns,
 		 *	}
 		 */
 	} else if (S_ISREG(mode)) {
-		if (sbi->options.sparse) {
+		if (sbi->options->sparse) {
 			/* sparsed regular file, cause option 'sparse' */
 			fa = FILE_ATTRIBUTE_SPARSE_FILE |
 			     FILE_ATTRIBUTE_ARCHIVE;
diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index e3a667e9838f..43b177280f39 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -279,7 +279,7 @@ struct ntfs_sb_info {
 #endif
 	} compress;
 
-	struct ntfs_mount_options options;
+	struct ntfs_mount_options *options;
 	struct ratelimit_state msg_ratelimit;
 };
 
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index c590872070e1..e9dfe274b558 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -379,11 +379,11 @@ static int ntfs_remount(struct super_block *sb, int *flags, char *data)
 		return -ENOMEM;
 
 	/* Store  original options */
-	memcpy(&old_opts, &sbi->options, sizeof(old_opts));
-	clear_mount_options(&sbi->options);
-	memset(&sbi->options, 0, sizeof(sbi->options));
+	memcpy(&old_opts, sbi->options, sizeof(old_opts));
+	clear_mount_options(sbi->options);
+	memset(sbi->options, 0, sizeof(old_opts));
 
-	err = ntfs_parse_options(sb, data, 0, &sbi->options);
+	err = ntfs_parse_options(sb, data, 0, sbi->options);
 	if (err)
 		goto restore_opts;
 
@@ -399,7 +399,7 @@ static int ntfs_remount(struct super_block *sb, int *flags, char *data)
 	sync_filesystem(sb);
 
 	if (ro_rw && (sbi->volume.flags & VOLUME_FLAG_DIRTY) &&
-	    !sbi->options.force) {
+	    !sbi->options->force) {
 		ntfs_warn(sb, "volume is dirty and \"force\" flag is not set!");
 		err = -EINVAL;
 		goto restore_opts;
@@ -412,8 +412,8 @@ static int ntfs_remount(struct super_block *sb, int *flags, char *data)
 	goto out;
 
 restore_opts:
-	clear_mount_options(&sbi->options);
-	memcpy(&sbi->options, &old_opts, sizeof(old_opts));
+	clear_mount_options(sbi->options);
+	memcpy(sbi->options, &old_opts, sizeof(old_opts));
 
 out:
 	kfree(orig_data);
@@ -494,7 +494,8 @@ static noinline void put_ntfs(struct ntfs_sb_info *sbi)
 	xpress_free_decompressor(sbi->compress.xpress);
 	lzx_free_decompressor(sbi->compress.lzx);
 #endif
-	clear_mount_options(&sbi->options);
+	clear_mount_options(sbi->options);
+	kfree(sbi->options);
 
 	kfree(sbi);
 }
@@ -533,7 +534,7 @@ static int ntfs_show_options(struct seq_file *m, struct dentry *root)
 {
 	struct super_block *sb = root->d_sb;
 	struct ntfs_sb_info *sbi = sb->s_fs_info;
-	struct ntfs_mount_options *opts = &sbi->options;
+	struct ntfs_mount_options *opts = sbi->options;
 	struct user_namespace *user_ns = seq_user_ns(m);
 
 	if (opts->uid)
@@ -910,6 +911,12 @@ static int ntfs_fill_super(struct super_block *sb, void *data, int silent)
 	if (!sbi)
 		return -ENOMEM;
 
+	sbi->options = kzalloc(sizeof(struct ntfs_mount_options), GFP_NOFS);
+	if (!sbi->options) {
+		kfree(sbi);
+		return -ENOMEM;
+	}
+
 	sb->s_fs_info = sbi;
 	sbi->sb = sb;
 	sb->s_flags |= SB_NODIRATIME;
@@ -922,7 +929,7 @@ static int ntfs_fill_super(struct super_block *sb, void *data, int silent)
 	ratelimit_state_init(&sbi->msg_ratelimit, DEFAULT_RATELIMIT_INTERVAL,
 			     DEFAULT_RATELIMIT_BURST);
 
-	err = ntfs_parse_options(sb, data, silent, &sbi->options);
+	err = ntfs_parse_options(sb, data, silent, sbi->options);
 	if (err)
 		goto out;
 
@@ -1054,7 +1061,7 @@ static int ntfs_fill_super(struct super_block *sb, void *data, int silent)
 			goto out;
 		}
 	} else if (sbi->volume.flags & VOLUME_FLAG_DIRTY) {
-		if (!is_ro && !sbi->options.force) {
+		if (!is_ro && !sbi->options->force) {
 			ntfs_warn(
 				sb,
 				"volume is dirty and \"force\" flag is not set!");
@@ -1376,7 +1383,7 @@ int ntfs_discard(struct ntfs_sb_info *sbi, CLST lcn, CLST len)
 	if (sbi->flags & NTFS_FLAGS_NODISCARD)
 		return -EOPNOTSUPP;
 
-	if (!sbi->options.discard)
+	if (!sbi->options->discard)
 		return -EOPNOTSUPP;
 
 	lbo = (u64)lcn << sbi->cluster_bits;
diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
index d3d5b9d331d1..a17d48735b99 100644
--- a/fs/ntfs3/xattr.c
+++ b/fs/ntfs3/xattr.c
@@ -774,7 +774,7 @@ int ntfs_acl_chmod(struct user_namespace *mnt_userns, struct inode *inode)
 int ntfs_permission(struct user_namespace *mnt_userns, struct inode *inode,
 		    int mask)
 {
-	if (ntfs_sb(inode->i_sb)->options.no_acs_rules) {
+	if (ntfs_sb(inode->i_sb)->options->no_acs_rules) {
 		/* "no access rules" mode - allow all changes */
 		return 0;
 	}
-- 
2.25.1

[PATCH v3 4/9] fs/ntfs3: Use new api for mounting

[Kari Argillander]

We have now new mount api as described in Documentation/filesystems. We
should use it as it gives us some benefits which are desribed here
lore.kernel.org/linux-fsdevel/159646178122.1784947.11705396571718464082.stgit@warthog.procyon.org.uk/

Nls loading is changed a to load with string. This did make code also
little cleaner.

Also try to use fsparam_flag_no as much as possible. This is just nice
little touch and is not mandatory but it should not make any harm. It
is just convenient that we can use example acl/noacl mount options.

Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 fs/ntfs3/ntfs_fs.h |   1 +
 fs/ntfs3/super.c   | 426 ++++++++++++++++++++++++---------------------
 2 files changed, 225 insertions(+), 202 deletions(-)

diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index 43b177280f39..45d6f4f91222 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -52,6 +52,7 @@
 // clang-format on
 
 struct ntfs_mount_options {
+	char *nls_name;
 	struct nls_table *nls;
 
 	kuid_t fs_uid;
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index e9dfe274b558..99102a146cf5 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -28,11 +28,12 @@
 #include <linux/buffer_head.h>
 #include <linux/exportfs.h>
 #include <linux/fs.h>
+#include <linux/fs_context.h>
+#include <linux/fs_parser.h>
 #include <linux/iversion.h>
 #include <linux/log2.h>
 #include <linux/module.h>
 #include <linux/nls.h>
-#include <linux/parser.h>
 #include <linux/seq_file.h>
 #include <linux/statfs.h>
 
@@ -198,9 +199,11 @@ void *ntfs_put_shared(void *ptr)
 	return ret;
 }
 
-static inline void clear_mount_options(struct ntfs_mount_options *options)
+static inline void put_mount_options(struct ntfs_mount_options *options)
 {
+	kfree(options->nls_name);
 	unload_nls(options->nls);
+	kfree(options);
 }
 
 enum Opt {
@@ -222,202 +225,171 @@ enum Opt {
 	Opt_err,
 };
 
-static const match_table_t ntfs_tokens = {
-	{ Opt_uid, "uid=%u" },
-	{ Opt_gid, "gid=%u" },
-	{ Opt_umask, "umask=%o" },
-	{ Opt_dmask, "dmask=%o" },
-	{ Opt_fmask, "fmask=%o" },
-	{ Opt_immutable, "sys_immutable" },
-	{ Opt_discard, "discard" },
-	{ Opt_force, "force" },
-	{ Opt_sparse, "sparse" },
-	{ Opt_nohidden, "nohidden" },
-	{ Opt_acl, "acl" },
-	{ Opt_showmeta, "showmeta" },
-	{ Opt_nls, "nls=%s" },
-	{ Opt_prealloc, "prealloc" },
-	{ Opt_no_acs_rules, "no_acs_rules" },
-	{ Opt_err, NULL },
+static const struct fs_parameter_spec ntfs_fs_parameters[] = {
+	fsparam_u32("uid",			Opt_uid),
+	fsparam_u32("gid",			Opt_gid),
+	fsparam_u32oct("umask",			Opt_umask),
+	fsparam_u32oct("dmask",			Opt_dmask),
+	fsparam_u32oct("fmask",			Opt_fmask),
+	fsparam_flag_no("sys_immutable",	Opt_immutable),
+	fsparam_flag_no("discard",		Opt_discard),
+	fsparam_flag_no("force",		Opt_force),
+	fsparam_flag_no("sparse",		Opt_sparse),
+	fsparam_flag("nohidden",		Opt_nohidden),
+	fsparam_flag_no("acl",			Opt_acl),
+	fsparam_flag_no("showmeta",		Opt_showmeta),
+	fsparam_string("nls",			Opt_nls),
+	fsparam_flag_no("prealloc",		Opt_prealloc),
+	fsparam_flag("no_acs_rules",		Opt_no_acs_rules),
+	{}
 };
 
-static noinline int ntfs_parse_options(struct super_block *sb, char *options,
-				       int silent,
-				       struct ntfs_mount_options *opts)
+/*
+ * Load nls table or if @nls is utf8 then return NULL.
+ */
+static struct nls_table *ntfs_load_nls(char *nls)
 {
-	char *p;
-	substring_t args[MAX_OPT_ARGS];
-	int option;
-	char nls_name[30];
-	struct nls_table *nls;
+	struct nls_table *ret;
 
-	opts->fs_uid = current_uid();
-	opts->fs_gid = current_gid();
-	opts->fs_fmask_inv = opts->fs_dmask_inv = ~current_umask();
-	nls_name[0] = 0;
+	if (!nls)
+		nls = CONFIG_NLS_DEFAULT;
 
-	if (!options)
-		goto out;
+	if (strcmp(nls, "utf8") == 0)
+		return NULL;
 
-	while ((p = strsep(&options, ","))) {
-		int token;
+	if (strcmp(nls, CONFIG_NLS_DEFAULT) == 0)
+		return load_nls_default();
 
-		if (!*p)
-			continue;
+	ret = load_nls(nls);
+	if (ret)
+		return ret;
 
-		token = match_token(p, ntfs_tokens, args);
-		switch (token) {
-		case Opt_immutable:
-			opts->sys_immutable = 1;
-			break;
-		case Opt_uid:
-			if (match_int(&args[0], &option))
-				return -EINVAL;
-			opts->fs_uid = make_kuid(current_user_ns(), option);
-			if (!uid_valid(opts->fs_uid))
-				return -EINVAL;
-			opts->uid = 1;
-			break;
-		case Opt_gid:
-			if (match_int(&args[0], &option))
-				return -EINVAL;
-			opts->fs_gid = make_kgid(current_user_ns(), option);
-			if (!gid_valid(opts->fs_gid))
-				return -EINVAL;
-			opts->gid = 1;
-			break;
-		case Opt_umask:
-			if (match_octal(&args[0], &option))
-				return -EINVAL;
-			opts->fs_fmask_inv = opts->fs_dmask_inv = ~option;
-			opts->fmask = opts->dmask = 1;
-			break;
-		case Opt_dmask:
-			if (match_octal(&args[0], &option))
-				return -EINVAL;
-			opts->fs_dmask_inv = ~option;
-			opts->dmask = 1;
-			break;
-		case Opt_fmask:
-			if (match_octal(&args[0], &option))
-				return -EINVAL;
-			opts->fs_fmask_inv = ~option;
-			opts->fmask = 1;
-			break;
-		case Opt_discard:
-			opts->discard = 1;
-			break;
-		case Opt_force:
-			opts->force = 1;
-			break;
-		case Opt_sparse:
-			opts->sparse = 1;
-			break;
-		case Opt_nohidden:
-			opts->nohidden = 1;
-			break;
-		case Opt_acl:
+	return ERR_PTR(-EINVAL);
+}
+
+static int ntfs_fs_parse_param(struct fs_context *fc,
+			       struct fs_parameter *param)
+{
+	struct ntfs_mount_options *opts = fc->fs_private;
+	struct fs_parse_result result;
+	int opt;
+
+	opt = fs_parse(fc, ntfs_fs_parameters, param, &result);
+	if (opt < 0)
+		return opt;
+
+	switch (opt) {
+	case Opt_uid:
+		opts->fs_uid = make_kuid(current_user_ns(), result.uint_32);
+		if (!uid_valid(opts->fs_uid))
+			return invalf(fc, "ntfs3: Invalid value for uid.");
+		opts->uid = 1;
+		break;
+	case Opt_gid:
+		opts->fs_gid = make_kgid(current_user_ns(), result.uint_32);
+		if (!gid_valid(opts->fs_gid))
+			return invalf(fc, "ntfs3: Invalid value for gid.");
+		opts->gid = 1;
+		break;
+	case Opt_umask:
+		if (result.uint_32 & ~07777)
+			return invalf(fc, "ntfs3: Invalid value for umask.");
+		opts->fs_fmask_inv = ~result.uint_32;
+		opts->fs_dmask_inv = ~result.uint_32;
+		opts->fmask = 1;
+		opts->dmask = 1;
+		break;
+	case Opt_dmask:
+		if (result.uint_32 & ~07777)
+			return invalf(fc, "ntfs3: Invalid value for dmask.");
+		opts->fs_dmask_inv = ~result.uint_32;
+		opts->dmask = 1;
+		break;
+	case Opt_fmask:
+		if (result.uint_32 & ~07777)
+			return invalf(fc, "ntfs3: Invalid value for fmask.");
+		opts->fs_fmask_inv = ~result.uint_32;
+		opts->fmask = 1;
+		break;
+	case Opt_immutable:
+		opts->sys_immutable = result.negated ? 0 : 1;
+		break;
+	case Opt_discard:
+		opts->discard = result.negated ? 0 : 1;
+		break;
+	case Opt_force:
+		opts->force = result.negated ? 0 : 1;
+		break;
+	case Opt_sparse:
+		opts->sparse = result.negated ? 0 : 1;
+		break;
+	case Opt_nohidden:
+		opts->nohidden = 1;
+		break;
+	case Opt_acl:
+		if (!result.negated)
 #ifdef CONFIG_NTFS3_FS_POSIX_ACL
-			sb->s_flags |= SB_POSIXACL;
-			break;
+			fc->sb_flags |= SB_POSIXACL;
 #else
-			ntfs_err(sb, "support for ACL not compiled in!");
-			return -EINVAL;
+			return invalf(fc, "ntfs3: Support for ACL not compiled in!");
 #endif
-		case Opt_showmeta:
-			opts->showmeta = 1;
-			break;
-		case Opt_nls:
-			match_strlcpy(nls_name, &args[0], sizeof(nls_name));
-			break;
-		case Opt_prealloc:
-			opts->prealloc = 1;
-			break;
-		case Opt_no_acs_rules:
-			opts->no_acs_rules = 1;
-			break;
-		default:
-			if (!silent)
-				ntfs_err(
-					sb,
-					"Unrecognized mount option \"%s\" or missing value",
-					p);
-			//return -EINVAL;
-		}
-	}
-
-out:
-	if (!strcmp(nls_name[0] ? nls_name : CONFIG_NLS_DEFAULT, "utf8")) {
-		/* For UTF-8 use utf16s_to_utf8s/utf8s_to_utf16s instead of nls */
-		nls = NULL;
-	} else if (nls_name[0]) {
-		nls = load_nls(nls_name);
-		if (!nls) {
-			ntfs_err(sb, "failed to load \"%s\"", nls_name);
-			return -EINVAL;
-		}
-	} else {
-		nls = load_nls_default();
-		if (!nls) {
-			ntfs_err(sb, "failed to load default nls");
-			return -EINVAL;
-		}
+		else
+			fc->sb_flags &= ~SB_POSIXACL;
+		break;
+	case Opt_showmeta:
+		opts->showmeta = result.negated ? 0 : 1;
+		break;
+	case Opt_nls:
+		kfree(opts->nls_name);
+		opts->nls_name = param->string;
+		param->string = NULL;
+		break;
+	case Opt_prealloc:
+		opts->prealloc = result.negated ? 0 : 1;
+		break;
+	case Opt_no_acs_rules:
+		opts->no_acs_rules = 1;
+		break;
+	default:
+		/* Should not be here unless we forget add case. */
+		return -EINVAL;
 	}
-	opts->nls = nls;
-
 	return 0;
 }
 
-static int ntfs_remount(struct super_block *sb, int *flags, char *data)
+static int ntfs_fs_reconfigure(struct fs_context *fc)
 {
-	int err, ro_rw;
+	struct super_block *sb = fc->root->d_sb;
 	struct ntfs_sb_info *sbi = sb->s_fs_info;
-	struct ntfs_mount_options old_opts;
-	char *orig_data = kstrdup(data, GFP_KERNEL);
-
-	if (data && !orig_data)
-		return -ENOMEM;
-
-	/* Store  original options */
-	memcpy(&old_opts, sbi->options, sizeof(old_opts));
-	clear_mount_options(sbi->options);
-	memset(sbi->options, 0, sizeof(old_opts));
+	struct ntfs_mount_options *new_opts = fc->fs_private;
+	int ro_rw;
 
-	err = ntfs_parse_options(sb, data, 0, sbi->options);
-	if (err)
-		goto restore_opts;
-
-	ro_rw = sb_rdonly(sb) && !(*flags & SB_RDONLY);
+	ro_rw = sb_rdonly(sb) && !(fc->sb_flags & SB_RDONLY);
 	if (ro_rw && (sbi->flags & NTFS_FLAGS_NEED_REPLAY)) {
-		ntfs_warn(
-			sb,
-			"Couldn't remount rw because journal is not replayed. Please umount/remount instead\n");
-		err = -EINVAL;
-		goto restore_opts;
+		errorf(fc, "ntfs3: Couldn't remount rw because journal is not replayed. Please umount/remount instead\n");
+		return -EINVAL;
+	}
+
+	new_opts->nls = ntfs_load_nls(new_opts->nls_name);
+	if (IS_ERR(new_opts->nls)) {
+		new_opts->nls = NULL;
+		errorf(fc, "ntfs3: Cannot load nls %s", new_opts->nls_name);
+		return -EINVAL;
 	}
+	if (new_opts->nls != sbi->options->nls)
+		return invalf(fc, "ntfs3: Cannot use different nls when remounting!");
 
 	sync_filesystem(sb);
 
 	if (ro_rw && (sbi->volume.flags & VOLUME_FLAG_DIRTY) &&
-	    !sbi->options->force) {
-		ntfs_warn(sb, "volume is dirty and \"force\" flag is not set!");
-		err = -EINVAL;
-		goto restore_opts;
+	    !new_opts->force) {
+		errorf(fc, "ntfs3: Volume is dirty and \"force\" flag is not set!");
+		return -EINVAL;
 	}
 
-	clear_mount_options(&old_opts);
-
-	ntfs_info(sb, "re-mounted. Opts: %s", orig_data);
-	err = 0;
-	goto out;
-
-restore_opts:
-	clear_mount_options(sbi->options);
-	memcpy(sbi->options, &old_opts, sizeof(old_opts));
+	memcpy(sbi->options, new_opts, sizeof(*new_opts));
 
-out:
-	kfree(orig_data);
-	return err;
+	return 0;
 }
 
 static struct kmem_cache *ntfs_inode_cachep;
@@ -494,9 +466,6 @@ static noinline void put_ntfs(struct ntfs_sb_info *sbi)
 	xpress_free_decompressor(sbi->compress.xpress);
 	lzx_free_decompressor(sbi->compress.lzx);
 #endif
-	clear_mount_options(sbi->options);
-	kfree(sbi->options);
-
 	kfree(sbi);
 }
 
@@ -507,7 +476,9 @@ static void ntfs_put_super(struct super_block *sb)
 	/*mark rw ntfs as clear, if possible*/
 	ntfs_set_state(sbi, NTFS_DIRTY_CLEAR);
 
+	put_mount_options(sbi->options);
 	put_ntfs(sbi);
+	sb->s_fs_info = NULL;
 
 	sync_blockdev(sb->s_bdev);
 }
@@ -621,7 +592,6 @@ static const struct super_operations ntfs_sops = {
 	.statfs = ntfs_statfs,
 	.show_options = ntfs_show_options,
 	.sync_fs = ntfs_sync_fs,
-	.remount_fs = ntfs_remount,
 	.write_inode = ntfs3_write_inode,
 };
 
@@ -885,10 +855,10 @@ static int ntfs_init_from_boot(struct super_block *sb, u32 sector_size,
 }
 
 /* try to mount*/
-static int ntfs_fill_super(struct super_block *sb, void *data, int silent)
+static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	int err;
-	struct ntfs_sb_info *sbi;
+	struct ntfs_sb_info *sbi = sb->s_fs_info;
 	struct block_device *bdev = sb->s_bdev;
 	struct inode *bd_inode = bdev->bd_inode;
 	struct request_queue *rq = bdev_get_queue(bdev);
@@ -907,17 +877,6 @@ static int ntfs_fill_super(struct super_block *sb, void *data, int silent)
 
 	ref.high = 0;
 
-	sbi = kzalloc(sizeof(struct ntfs_sb_info), GFP_NOFS);
-	if (!sbi)
-		return -ENOMEM;
-
-	sbi->options = kzalloc(sizeof(struct ntfs_mount_options), GFP_NOFS);
-	if (!sbi->options) {
-		kfree(sbi);
-		return -ENOMEM;
-	}
-
-	sb->s_fs_info = sbi;
 	sbi->sb = sb;
 	sb->s_flags |= SB_NODIRATIME;
 	sb->s_magic = 0x7366746e; // "ntfs"
@@ -929,9 +888,12 @@ static int ntfs_fill_super(struct super_block *sb, void *data, int silent)
 	ratelimit_state_init(&sbi->msg_ratelimit, DEFAULT_RATELIMIT_INTERVAL,
 			     DEFAULT_RATELIMIT_BURST);
 
-	err = ntfs_parse_options(sb, data, silent, sbi->options);
-	if (err)
-		goto out;
+	sbi->options->nls = ntfs_load_nls(sbi->options->nls_name);
+	if (IS_ERR(sbi->options->nls)) {
+		sbi->options->nls = NULL;
+		errorf(fc, "Cannot load nls %s", sbi->options->nls_name);
+		return -EINVAL;
+	}
 
 	if (!rq || !blk_queue_discard(rq) || !rq->limits.discard_granularity) {
 		;
@@ -1324,6 +1286,9 @@ static int ntfs_fill_super(struct super_block *sb, void *data, int silent)
 		goto out;
 	}
 
+	fc->fs_private = NULL;
+	fc->s_fs_info = NULL;
+
 	return 0;
 
 out:
@@ -1334,9 +1299,6 @@ static int ntfs_fill_super(struct super_block *sb, void *data, int silent)
 		sb->s_root = NULL;
 	}
 
-	put_ntfs(sbi);
-
-	sb->s_fs_info = NULL;
 	return err;
 }
 
@@ -1408,19 +1370,79 @@ int ntfs_discard(struct ntfs_sb_info *sbi, CLST lcn, CLST len)
 	return err;
 }
 
-static struct dentry *ntfs_mount(struct file_system_type *fs_type, int flags,
-				 const char *dev_name, void *data)
+static int ntfs_fs_get_tree(struct fs_context *fc)
+{
+	return get_tree_bdev(fc, ntfs_fill_super);
+}
+
+/*
+ * Note that this will be called after fill_super and reconfigure
+ * even when they pass. So they have to take pointers if they pass.
+ */
+static void ntfs_fs_free(struct fs_context *fc)
 {
-	return mount_bdev(fs_type, flags, dev_name, data, ntfs_fill_super);
+	struct ntfs_mount_options *opts = fc->fs_private;
+	struct ntfs_sb_info *sbi = fc->s_fs_info;
+
+	if (sbi)
+		put_ntfs(sbi);
+
+	if (opts)
+		put_mount_options(opts);
+}
+
+static const struct fs_context_operations ntfs_context_ops = {
+	.parse_param	= ntfs_fs_parse_param,
+	.get_tree	= ntfs_fs_get_tree,
+	.reconfigure	= ntfs_fs_reconfigure,
+	.free		= ntfs_fs_free,
+};
+
+/*
+ * This will called when mount/remount. We will first initiliaze
+ * options so that if remount we can use just that.
+ */
+static int ntfs_init_fs_context(struct fs_context *fc)
+{
+	struct ntfs_mount_options *opts;
+	struct ntfs_sb_info *sbi;
+
+	opts = kzalloc(sizeof(struct ntfs_mount_options), GFP_NOFS);
+	if (!opts)
+		return -ENOMEM;
+
+	/* Default options. */
+	opts->fs_uid = current_uid();
+	opts->fs_gid = current_gid();
+	opts->fs_fmask_inv = ~current_umask();
+	opts->fs_dmask_inv = ~current_umask();
+
+	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE)
+		goto ok;
+
+	sbi = kzalloc(sizeof(struct ntfs_sb_info), GFP_NOFS);
+	if (!sbi) {
+		kfree(opts);
+		return -ENOMEM;
+	}
+
+	sbi->options = opts;
+	fc->s_fs_info = sbi;
+ok:
+	fc->fs_private = opts;
+	fc->ops = &ntfs_context_ops;
+
+	return 0;
 }
 
 // clang-format off
 static struct file_system_type ntfs_fs_type = {
-	.owner		= THIS_MODULE,
-	.name		= "ntfs3",
-	.mount		= ntfs_mount,
-	.kill_sb	= kill_block_super,
-	.fs_flags	= FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
+	.owner			= THIS_MODULE,
+	.name			= "ntfs3",
+	.init_fs_context	= ntfs_init_fs_context,
+	.parameters		= ntfs_fs_parameters,
+	.kill_sb		= kill_block_super,
+	.fs_flags		= FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
 };
 // clang-format on
 
-- 
2.25.1

[PATCH v3 5/9] fs/ntfs3: Init spi more in init_fs_context than fill_super

[Kari Argillander]

init_fs_context() is meant to initialize s_fs_info (spi). Move spi
initializing code there which we can initialize before fill_super().

Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 fs/ntfs3/super.c | 41 ++++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 99102a146cf5..b2a3f947542b 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -870,7 +870,7 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	const struct VOLUME_INFO *info;
 	u32 idx, done, bytes;
 	struct ATTR_DEF_ENTRY *t;
-	u16 *upcase = NULL;
+	u16 *upcase;
 	u16 *shared;
 	bool is_ro;
 	struct MFT_REF ref;
@@ -885,9 +885,6 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	sb->s_time_gran = NTFS_TIME_GRAN; // 100 nsec
 	sb->s_xattr = ntfs_xattr_handlers;
 
-	ratelimit_state_init(&sbi->msg_ratelimit, DEFAULT_RATELIMIT_INTERVAL,
-			     DEFAULT_RATELIMIT_BURST);
-
 	sbi->options->nls = ntfs_load_nls(sbi->options->nls_name);
 	if (IS_ERR(sbi->options->nls)) {
 		sbi->options->nls = NULL;
@@ -917,12 +914,6 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 	sb->s_maxbytes = 0xFFFFFFFFull << sbi->cluster_bits;
 #endif
 
-	mutex_init(&sbi->compress.mtx_lznt);
-#ifdef CONFIG_NTFS3_LZX_XPRESS
-	mutex_init(&sbi->compress.mtx_xpress);
-	mutex_init(&sbi->compress.mtx_lzx);
-#endif
-
 	/*
 	 * Load $Volume. This should be done before LogFile
 	 * 'cause 'sbi->volume.ni' is used 'ntfs_set_state'
@@ -1207,11 +1198,7 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
 		goto out;
 	}
 
-	sbi->upcase = upcase = kvmalloc(0x10000 * sizeof(short), GFP_KERNEL);
-	if (!upcase) {
-		err = -ENOMEM;
-		goto out;
-	}
+	upcase = sbi->upcase;
 
 	for (idx = 0; idx < (0x10000 * sizeof(short) >> PAGE_SHIFT); idx++) {
 		const __le16 *src;
@@ -1421,10 +1408,21 @@ static int ntfs_init_fs_context(struct fs_context *fc)
 		goto ok;
 
 	sbi = kzalloc(sizeof(struct ntfs_sb_info), GFP_NOFS);
-	if (!sbi) {
-		kfree(opts);
-		return -ENOMEM;
-	}
+	if (!sbi)
+		goto free_opts;
+
+	sbi->upcase = kvmalloc(0x10000 * sizeof(short), GFP_KERNEL);
+	if (!sbi->upcase)
+		goto free_sbi;
+
+	ratelimit_state_init(&sbi->msg_ratelimit, DEFAULT_RATELIMIT_INTERVAL,
+			     DEFAULT_RATELIMIT_BURST);
+
+	mutex_init(&sbi->compress.mtx_lznt);
+#ifdef CONFIG_NTFS3_LZX_XPRESS
+	mutex_init(&sbi->compress.mtx_xpress);
+	mutex_init(&sbi->compress.mtx_lzx);
+#endif
 
 	sbi->options = opts;
 	fc->s_fs_info = sbi;
@@ -1433,6 +1431,11 @@ static int ntfs_init_fs_context(struct fs_context *fc)
 	fc->ops = &ntfs_context_ops;
 
 	return 0;
+free_opts:
+	kfree(opts);
+free_sbi:
+	kfree(sbi);
+	return -ENOMEM;
 }
 
 // clang-format off
-- 
2.25.1

[PATCH v3 6/9] fs/ntfs3: Make mount option nohidden more universal

[Kari Argillander]

If we call Opt_nohidden with just keyword hidden, then we can use
hidden/nohidden when mounting. We already use this method for almoust
all other parameters so it is just logical that this will use same
method.

Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 fs/ntfs3/super.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index b2a3f947542b..52e0dc45e060 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -235,7 +235,7 @@ static const struct fs_parameter_spec ntfs_fs_parameters[] = {
 	fsparam_flag_no("discard",		Opt_discard),
 	fsparam_flag_no("force",		Opt_force),
 	fsparam_flag_no("sparse",		Opt_sparse),
-	fsparam_flag("nohidden",		Opt_nohidden),
+	fsparam_flag_no("hidden",		Opt_nohidden),
 	fsparam_flag_no("acl",			Opt_acl),
 	fsparam_flag_no("showmeta",		Opt_showmeta),
 	fsparam_string("nls",			Opt_nls),
@@ -324,7 +324,7 @@ static int ntfs_fs_parse_param(struct fs_context *fc,
 		opts->sparse = result.negated ? 0 : 1;
 		break;
 	case Opt_nohidden:
-		opts->nohidden = 1;
+		opts->nohidden = result.negated ? 1 : 0;
 		break;
 	case Opt_acl:
 		if (!result.negated)
-- 
2.25.1

[PATCH v3 7/9] fs/ntfs3: Add iocharset= mount option as alias for nls=

[Kari Argillander]

Other fs drivers are using iocharset= mount option for specifying charset.
So add it also for ntfs3 and mark old nls= mount option as deprecated.

Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 Documentation/filesystems/ntfs3.rst |  4 ++--
 fs/ntfs3/super.c                    | 18 +++++++++++-------
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/Documentation/filesystems/ntfs3.rst b/Documentation/filesystems/ntfs3.rst
index af7158de6fde..ded706474825 100644
--- a/Documentation/filesystems/ntfs3.rst
+++ b/Documentation/filesystems/ntfs3.rst
@@ -32,12 +32,12 @@ generic ones.
 
 ===============================================================================
 
-nls=name		This option informs the driver how to interpret path
+iocharset=name		This option informs the driver how to interpret path
 			strings and translate them to Unicode and back. If
 			this option is not set, the default codepage will be
 			used (CONFIG_NLS_DEFAULT).
 			Examples:
-				'nls=utf8'
+				'iocharset=utf8'
 
 uid=
 gid=
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 52e0dc45e060..e5c319604c4d 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -219,7 +219,7 @@ enum Opt {
 	Opt_nohidden,
 	Opt_showmeta,
 	Opt_acl,
-	Opt_nls,
+	Opt_iocharset,
 	Opt_prealloc,
 	Opt_no_acs_rules,
 	Opt_err,
@@ -238,9 +238,13 @@ static const struct fs_parameter_spec ntfs_fs_parameters[] = {
 	fsparam_flag_no("hidden",		Opt_nohidden),
 	fsparam_flag_no("acl",			Opt_acl),
 	fsparam_flag_no("showmeta",		Opt_showmeta),
-	fsparam_string("nls",			Opt_nls),
 	fsparam_flag_no("prealloc",		Opt_prealloc),
 	fsparam_flag("no_acs_rules",		Opt_no_acs_rules),
+	fsparam_string("iocharset",		Opt_iocharset),
+
+	__fsparam(fs_param_is_string,
+		  "nls", Opt_iocharset,
+		  fs_param_deprecated, NULL),
 	{}
 };
 
@@ -339,7 +343,7 @@ static int ntfs_fs_parse_param(struct fs_context *fc,
 	case Opt_showmeta:
 		opts->showmeta = result.negated ? 0 : 1;
 		break;
-	case Opt_nls:
+	case Opt_iocharset:
 		kfree(opts->nls_name);
 		opts->nls_name = param->string;
 		param->string = NULL;
@@ -373,11 +377,11 @@ static int ntfs_fs_reconfigure(struct fs_context *fc)
 	new_opts->nls = ntfs_load_nls(new_opts->nls_name);
 	if (IS_ERR(new_opts->nls)) {
 		new_opts->nls = NULL;
-		errorf(fc, "ntfs3: Cannot load nls %s", new_opts->nls_name);
+		errorf(fc, "ntfs3: Cannot load iocharset %s", new_opts->nls_name);
 		return -EINVAL;
 	}
 	if (new_opts->nls != sbi->options->nls)
-		return invalf(fc, "ntfs3: Cannot use different nls when remounting!");
+		return invalf(fc, "ntfs3: Cannot use different iocharset when remounting!");
 
 	sync_filesystem(sb);
 
@@ -519,9 +523,9 @@ static int ntfs_show_options(struct seq_file *m, struct dentry *root)
 	if (opts->dmask)
 		seq_printf(m, ",dmask=%04o", ~opts->fs_dmask_inv);
 	if (opts->nls)
-		seq_printf(m, ",nls=%s", opts->nls->charset);
+		seq_printf(m, ",iocharset=%s", opts->nls->charset);
 	else
-		seq_puts(m, ",nls=utf8");
+		seq_puts(m, ",iocharset=utf8");
 	if (opts->sys_immutable)
 		seq_puts(m, ",sys_immutable");
 	if (opts->discard)
-- 
2.25.1

[PATCH v3 8/9] fs/ntfs3: Rename mount option no_acl_rules > (no)acl_rules

[Kari Argillander]

Rename mount option no_acl_rules to noacl_rules. This allow us to use
possibility to mount with options noacl_rules or acl_rules.

Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 Documentation/filesystems/ntfs3.rst |  2 +-
 fs/ntfs3/file.c                     |  2 +-
 fs/ntfs3/ntfs_fs.h                  |  2 +-
 fs/ntfs3/super.c                    | 12 ++++++------
 fs/ntfs3/xattr.c                    |  2 +-
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/Documentation/filesystems/ntfs3.rst b/Documentation/filesystems/ntfs3.rst
index ded706474825..bdc9dd5a9185 100644
--- a/Documentation/filesystems/ntfs3.rst
+++ b/Documentation/filesystems/ntfs3.rst
@@ -73,7 +73,7 @@ prealloc		Preallocate space for files excessively when file size is
 			increasing on writes. Decreases fragmentation in case of
 			parallel write operations to different files.
 
-no_acs_rules		"No access rules" mount option sets access rights for
+noacs_rules		"No access rules" mount option sets access rights for
 			files/folders to 777 and owner/group to root. This mount
 			option absorbs all other permissions:
 			- permissions change for files/folders will be reported
diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c
index c79e4aff7a19..4c9ff7fcf0b1 100644
--- a/fs/ntfs3/file.c
+++ b/fs/ntfs3/file.c
@@ -743,7 +743,7 @@ int ntfs3_setattr(struct user_namespace *mnt_userns, struct dentry *dentry,
 	umode_t mode = inode->i_mode;
 	int err;
 
-	if (sbi->options->no_acs_rules) {
+	if (sbi->options->noacs_rules) {
 		/* "no access rules" - force any changes of time etc. */
 		attr->ia_valid |= ATTR_FORCE;
 		/* and disable for editing some attributes */
diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index 45d6f4f91222..5df55bc733bd 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -70,7 +70,7 @@ struct ntfs_mount_options {
 		showmeta : 1, /*show meta files*/
 		nohidden : 1, /*do not show hidden files*/
 		force : 1, /*rw mount dirty volume*/
-		no_acs_rules : 1, /*exclude acs rules*/
+		noacs_rules : 1, /*exclude acs rules*/
 		prealloc : 1 /*preallocate space when file is growing*/
 		;
 };
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index e5c319604c4d..d7408b4f6813 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -221,7 +221,7 @@ enum Opt {
 	Opt_acl,
 	Opt_iocharset,
 	Opt_prealloc,
-	Opt_no_acs_rules,
+	Opt_noacs_rules,
 	Opt_err,
 };
 
@@ -239,7 +239,7 @@ static const struct fs_parameter_spec ntfs_fs_parameters[] = {
 	fsparam_flag_no("acl",			Opt_acl),
 	fsparam_flag_no("showmeta",		Opt_showmeta),
 	fsparam_flag_no("prealloc",		Opt_prealloc),
-	fsparam_flag("no_acs_rules",		Opt_no_acs_rules),
+	fsparam_flag_no("acs_rules",		Opt_noacs_rules),
 	fsparam_string("iocharset",		Opt_iocharset),
 
 	__fsparam(fs_param_is_string,
@@ -351,8 +351,8 @@ static int ntfs_fs_parse_param(struct fs_context *fc,
 	case Opt_prealloc:
 		opts->prealloc = result.negated ? 0 : 1;
 		break;
-	case Opt_no_acs_rules:
-		opts->no_acs_rules = 1;
+	case Opt_noacs_rules:
+		opts->noacs_rules = result.negated ? 1 : 0;
 		break;
 	default:
 		/* Should not be here unless we forget add case. */
@@ -538,8 +538,8 @@ static int ntfs_show_options(struct seq_file *m, struct dentry *root)
 		seq_puts(m, ",nohidden");
 	if (opts->force)
 		seq_puts(m, ",force");
-	if (opts->no_acs_rules)
-		seq_puts(m, ",no_acs_rules");
+	if (opts->noacs_rules)
+		seq_puts(m, ",noacs_rules");
 	if (opts->prealloc)
 		seq_puts(m, ",prealloc");
 	if (sb->s_flags & SB_POSIXACL)
diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
index a17d48735b99..4b37ed239579 100644
--- a/fs/ntfs3/xattr.c
+++ b/fs/ntfs3/xattr.c
@@ -774,7 +774,7 @@ int ntfs_acl_chmod(struct user_namespace *mnt_userns, struct inode *inode)
 int ntfs_permission(struct user_namespace *mnt_userns, struct inode *inode,
 		    int mask)
 {
-	if (ntfs_sb(inode->i_sb)->options->no_acs_rules) {
+	if (ntfs_sb(inode->i_sb)->options->noacs_rules) {
 		/* "no access rules" mode - allow all changes */
 		return 0;
 	}
-- 
2.25.1

Re: [PATCH v3 8/9] fs/ntfs3: Rename mount option no_acl_rules > (no)acl_rules

[Pali Rohár]

Hello!

On Sunday 29 August 2021 12:56:13 Kari Argillander wrote:
> Rename mount option no_acl_rules to noacl_rules. This allow us to use
> possibility to mount with options noacl_rules or acl_rules.

$commit_message =~ s/acl/acs/g;

Anyway, for me "noacs_rules" name looks strange. Underline is used as a
word separator and so original name "no_acs_rules" looks better. But if
you are going to remove first underline, why not then remove also the
second one? So name would be "noacsrules" and better matches naming
convention?

And I see that other filesystems have option 'mode' (e.g. iso9660, udf)
whicha is basically superset of this no_acs_rules as it supports to set
permission to also any other mode than 0777.

Maybe this could be a good thing to unify across all filesystems in
future...

> Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
> ---
>  Documentation/filesystems/ntfs3.rst |  2 +-
>  fs/ntfs3/file.c                     |  2 +-
>  fs/ntfs3/ntfs_fs.h                  |  2 +-
>  fs/ntfs3/super.c                    | 12 ++++++------
>  fs/ntfs3/xattr.c                    |  2 +-
>  5 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/filesystems/ntfs3.rst b/Documentation/filesystems/ntfs3.rst
> index ded706474825..bdc9dd5a9185 100644
> --- a/Documentation/filesystems/ntfs3.rst
> +++ b/Documentation/filesystems/ntfs3.rst
> @@ -73,7 +73,7 @@ prealloc		Preallocate space for files excessively when file size is
>  			increasing on writes. Decreases fragmentation in case of
>  			parallel write operations to different files.
>  
> -no_acs_rules		"No access rules" mount option sets access rights for
> +noacs_rules		"No access rules" mount option sets access rights for
>  			files/folders to 777 and owner/group to root. This mount
>  			option absorbs all other permissions:
>  			- permissions change for files/folders will be reported
> diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c
> index c79e4aff7a19..4c9ff7fcf0b1 100644
> --- a/fs/ntfs3/file.c
> +++ b/fs/ntfs3/file.c
> @@ -743,7 +743,7 @@ int ntfs3_setattr(struct user_namespace *mnt_userns, struct dentry *dentry,
>  	umode_t mode = inode->i_mode;
>  	int err;
>  
> -	if (sbi->options->no_acs_rules) {
> +	if (sbi->options->noacs_rules) {
>  		/* "no access rules" - force any changes of time etc. */
>  		attr->ia_valid |= ATTR_FORCE;
>  		/* and disable for editing some attributes */
> diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
> index 45d6f4f91222..5df55bc733bd 100644
> --- a/fs/ntfs3/ntfs_fs.h
> +++ b/fs/ntfs3/ntfs_fs.h
> @@ -70,7 +70,7 @@ struct ntfs_mount_options {
>  		showmeta : 1, /*show meta files*/
>  		nohidden : 1, /*do not show hidden files*/
>  		force : 1, /*rw mount dirty volume*/
> -		no_acs_rules : 1, /*exclude acs rules*/
> +		noacs_rules : 1, /*exclude acs rules*/
>  		prealloc : 1 /*preallocate space when file is growing*/
>  		;
>  };
> diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
> index e5c319604c4d..d7408b4f6813 100644
> --- a/fs/ntfs3/super.c
> +++ b/fs/ntfs3/super.c
> @@ -221,7 +221,7 @@ enum Opt {
>  	Opt_acl,
>  	Opt_iocharset,
>  	Opt_prealloc,
> -	Opt_no_acs_rules,
> +	Opt_noacs_rules,
>  	Opt_err,
>  };
>  
> @@ -239,7 +239,7 @@ static const struct fs_parameter_spec ntfs_fs_parameters[] = {
>  	fsparam_flag_no("acl",			Opt_acl),
>  	fsparam_flag_no("showmeta",		Opt_showmeta),
>  	fsparam_flag_no("prealloc",		Opt_prealloc),
> -	fsparam_flag("no_acs_rules",		Opt_no_acs_rules),
> +	fsparam_flag_no("acs_rules",		Opt_noacs_rules),
>  	fsparam_string("iocharset",		Opt_iocharset),
>  
>  	__fsparam(fs_param_is_string,
> @@ -351,8 +351,8 @@ static int ntfs_fs_parse_param(struct fs_context *fc,
>  	case Opt_prealloc:
>  		opts->prealloc = result.negated ? 0 : 1;
>  		break;
> -	case Opt_no_acs_rules:
> -		opts->no_acs_rules = 1;
> +	case Opt_noacs_rules:
> +		opts->noacs_rules = result.negated ? 1 : 0;
>  		break;
>  	default:
>  		/* Should not be here unless we forget add case. */
> @@ -538,8 +538,8 @@ static int ntfs_show_options(struct seq_file *m, struct dentry *root)
>  		seq_puts(m, ",nohidden");
>  	if (opts->force)
>  		seq_puts(m, ",force");
> -	if (opts->no_acs_rules)
> -		seq_puts(m, ",no_acs_rules");
> +	if (opts->noacs_rules)
> +		seq_puts(m, ",noacs_rules");
>  	if (opts->prealloc)
>  		seq_puts(m, ",prealloc");
>  	if (sb->s_flags & SB_POSIXACL)
> diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
> index a17d48735b99..4b37ed239579 100644
> --- a/fs/ntfs3/xattr.c
> +++ b/fs/ntfs3/xattr.c
> @@ -774,7 +774,7 @@ int ntfs_acl_chmod(struct user_namespace *mnt_userns, struct inode *inode)
>  int ntfs_permission(struct user_namespace *mnt_userns, struct inode *inode,
>  		    int mask)
>  {
> -	if (ntfs_sb(inode->i_sb)->options->no_acs_rules) {
> +	if (ntfs_sb(inode->i_sb)->options->noacs_rules) {
>  		/* "no access rules" mode - allow all changes */
>  		return 0;
>  	}
> -- 
> 2.25.1
> 

Re: [PATCH v3 8/9] fs/ntfs3: Rename mount option no_acl_rules > (no)acl_rules

[Kari Argillander]

On Sun, Aug 29, 2021 at 12:16:37PM +0200, Pali Rohár wrote:
> Hello!
> 
> On Sunday 29 August 2021 12:56:13 Kari Argillander wrote:
> > Rename mount option no_acl_rules to noacl_rules. This allow us to use
> > possibility to mount with options noacl_rules or acl_rules.
> 
> $commit_message =~ s/acl/acs/g;

Thanks.

> 
> Anyway, for me "noacs_rules" name looks strange. Underline is used as a
> word separator and so original name "no_acs_rules" looks better. But if
> you are going to remove first underline, why not then remove also the
> second one? So name would be "noacsrules" and better matches naming
> convention?

I agree. Now that you wrote it like that I see that is definitely
better.

> And I see that other filesystems have option 'mode' (e.g. iso9660, udf)
> whicha is basically superset of this no_acs_rules as it supports to set
> permission to also any other mode than 0777.

We also have umask, fmask and dmask. Isn't fmask=mode and dmask=dmode?

I have not tested these really, but my impression is that noacsrules
will kinda overwrite everything else. It can also lie to user because
user can change file permission, but it will not change in reality.

I'm not even sure when do we need this option. Konstantin can probably
enlighten us or at least me.

> Maybe this could be a good thing to unify across all filesystems in
> future...

Hopefully.

> 
> > Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
> > ---
> >  Documentation/filesystems/ntfs3.rst |  2 +-
> >  fs/ntfs3/file.c                     |  2 +-
> >  fs/ntfs3/ntfs_fs.h                  |  2 +-
> >  fs/ntfs3/super.c                    | 12 ++++++------
> >  fs/ntfs3/xattr.c                    |  2 +-
> >  5 files changed, 10 insertions(+), 10 deletions(-)
> > 
> > diff --git a/Documentation/filesystems/ntfs3.rst b/Documentation/filesystems/ntfs3.rst
> > index ded706474825..bdc9dd5a9185 100644
> > --- a/Documentation/filesystems/ntfs3.rst
> > +++ b/Documentation/filesystems/ntfs3.rst
> > @@ -73,7 +73,7 @@ prealloc		Preallocate space for files excessively when file size is
> >  			increasing on writes. Decreases fragmentation in case of
> >  			parallel write operations to different files.
> >  
> > -no_acs_rules		"No access rules" mount option sets access rights for
> > +noacs_rules		"No access rules" mount option sets access rights for
> >  			files/folders to 777 and owner/group to root. This mount
> >  			option absorbs all other permissions:
> >  			- permissions change for files/folders will be reported
> > diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c
> > index c79e4aff7a19..4c9ff7fcf0b1 100644
> > --- a/fs/ntfs3/file.c
> > +++ b/fs/ntfs3/file.c
> > @@ -743,7 +743,7 @@ int ntfs3_setattr(struct user_namespace *mnt_userns, struct dentry *dentry,
> >  	umode_t mode = inode->i_mode;
> >  	int err;
> >  
> > -	if (sbi->options->no_acs_rules) {
> > +	if (sbi->options->noacs_rules) {
> >  		/* "no access rules" - force any changes of time etc. */
> >  		attr->ia_valid |= ATTR_FORCE;
> >  		/* and disable for editing some attributes */
> > diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
> > index 45d6f4f91222..5df55bc733bd 100644
> > --- a/fs/ntfs3/ntfs_fs.h
> > +++ b/fs/ntfs3/ntfs_fs.h
> > @@ -70,7 +70,7 @@ struct ntfs_mount_options {
> >  		showmeta : 1, /*show meta files*/
> >  		nohidden : 1, /*do not show hidden files*/
> >  		force : 1, /*rw mount dirty volume*/
> > -		no_acs_rules : 1, /*exclude acs rules*/
> > +		noacs_rules : 1, /*exclude acs rules*/
> >  		prealloc : 1 /*preallocate space when file is growing*/
> >  		;
> >  };
> > diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
> > index e5c319604c4d..d7408b4f6813 100644
> > --- a/fs/ntfs3/super.c
> > +++ b/fs/ntfs3/super.c
> > @@ -221,7 +221,7 @@ enum Opt {
> >  	Opt_acl,
> >  	Opt_iocharset,
> >  	Opt_prealloc,
> > -	Opt_no_acs_rules,
> > +	Opt_noacs_rules,
> >  	Opt_err,
> >  };
> >  
> > @@ -239,7 +239,7 @@ static const struct fs_parameter_spec ntfs_fs_parameters[] = {
> >  	fsparam_flag_no("acl",			Opt_acl),
> >  	fsparam_flag_no("showmeta",		Opt_showmeta),
> >  	fsparam_flag_no("prealloc",		Opt_prealloc),
> > -	fsparam_flag("no_acs_rules",		Opt_no_acs_rules),
> > +	fsparam_flag_no("acs_rules",		Opt_noacs_rules),
> >  	fsparam_string("iocharset",		Opt_iocharset),
> >  
> >  	__fsparam(fs_param_is_string,
> > @@ -351,8 +351,8 @@ static int ntfs_fs_parse_param(struct fs_context *fc,
> >  	case Opt_prealloc:
> >  		opts->prealloc = result.negated ? 0 : 1;
> >  		break;
> > -	case Opt_no_acs_rules:
> > -		opts->no_acs_rules = 1;
> > +	case Opt_noacs_rules:
> > +		opts->noacs_rules = result.negated ? 1 : 0;
> >  		break;
> >  	default:
> >  		/* Should not be here unless we forget add case. */
> > @@ -538,8 +538,8 @@ static int ntfs_show_options(struct seq_file *m, struct dentry *root)
> >  		seq_puts(m, ",nohidden");
> >  	if (opts->force)
> >  		seq_puts(m, ",force");
> > -	if (opts->no_acs_rules)
> > -		seq_puts(m, ",no_acs_rules");
> > +	if (opts->noacs_rules)
> > +		seq_puts(m, ",noacs_rules");
> >  	if (opts->prealloc)
> >  		seq_puts(m, ",prealloc");
> >  	if (sb->s_flags & SB_POSIXACL)
> > diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
> > index a17d48735b99..4b37ed239579 100644
> > --- a/fs/ntfs3/xattr.c
> > +++ b/fs/ntfs3/xattr.c
> > @@ -774,7 +774,7 @@ int ntfs_acl_chmod(struct user_namespace *mnt_userns, struct inode *inode)
> >  int ntfs_permission(struct user_namespace *mnt_userns, struct inode *inode,
> >  		    int mask)
> >  {
> > -	if (ntfs_sb(inode->i_sb)->options->no_acs_rules) {
> > +	if (ntfs_sb(inode->i_sb)->options->noacs_rules) {
> >  		/* "no access rules" mode - allow all changes */
> >  		return 0;
> >  	}
> > -- 
> > 2.25.1
> > 
> 

[PATCH v3 9/9] fs/ntfs3: Show uid/gid always in show_options()

[Kari Argillander]

Show options should show option according documentation when some value
is not default or when ever coder wants. Uid/gid are problematic because
it is hard to know which are defaults. In file system there is many
different implementation for this problem.

Some file systems show uid/gid when they are different than root, some
when user has set them and some show them always. There is also problem
that what if root uid/gid change. This code just choose to show them
always. This way we do not need to think this any more.

Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 fs/ntfs3/ntfs_fs.h | 23 ++++++++++-------------
 fs/ntfs3/super.c   | 12 ++++--------
 2 files changed, 14 insertions(+), 21 deletions(-)

diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index 5df55bc733bd..a3a7d10de7cb 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -60,19 +60,16 @@ struct ntfs_mount_options {
 	u16 fs_fmask_inv;
 	u16 fs_dmask_inv;
 
-	unsigned uid : 1, /* uid was set */
-		gid : 1, /* gid was set */
-		fmask : 1, /* fmask was set */
-		dmask : 1, /*dmask was set*/
-		sys_immutable : 1, /* immutable system files */
-		discard : 1, /* issue discard requests on deletions */
-		sparse : 1, /*create sparse files*/
-		showmeta : 1, /*show meta files*/
-		nohidden : 1, /*do not show hidden files*/
-		force : 1, /*rw mount dirty volume*/
-		noacs_rules : 1, /*exclude acs rules*/
-		prealloc : 1 /*preallocate space when file is growing*/
-		;
+	unsigned fmask : 1; /* fmask was set */
+	unsigned dmask : 1; /*dmask was set*/
+	unsigned sys_immutable : 1; /* immutable system files */
+	unsigned discard : 1; /* issue discard requests on deletions */
+	unsigned sparse : 1; /*create sparse files*/
+	unsigned showmeta : 1; /*show meta files*/
+	unsigned nohidden : 1; /*do not show hidden files*/
+	unsigned force : 1; /*rw mount dirty volume*/
+	unsigned noacs_rules : 1; /*exclude acs rules*/
+	unsigned prealloc : 1; /*preallocate space when file is growing*/
 };
 
 /* special value to unpack and deallocate*/
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index d7408b4f6813..d28fab6c2297 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -287,13 +287,11 @@ static int ntfs_fs_parse_param(struct fs_context *fc,
 		opts->fs_uid = make_kuid(current_user_ns(), result.uint_32);
 		if (!uid_valid(opts->fs_uid))
 			return invalf(fc, "ntfs3: Invalid value for uid.");
-		opts->uid = 1;
 		break;
 	case Opt_gid:
 		opts->fs_gid = make_kgid(current_user_ns(), result.uint_32);
 		if (!gid_valid(opts->fs_gid))
 			return invalf(fc, "ntfs3: Invalid value for gid.");
-		opts->gid = 1;
 		break;
 	case Opt_umask:
 		if (result.uint_32 & ~07777)
@@ -512,12 +510,10 @@ static int ntfs_show_options(struct seq_file *m, struct dentry *root)
 	struct ntfs_mount_options *opts = sbi->options;
 	struct user_namespace *user_ns = seq_user_ns(m);
 
-	if (opts->uid)
-		seq_printf(m, ",uid=%u",
-			   from_kuid_munged(user_ns, opts->fs_uid));
-	if (opts->gid)
-		seq_printf(m, ",gid=%u",
-			   from_kgid_munged(user_ns, opts->fs_gid));
+	seq_printf(m, ",uid=%u",
+		  from_kuid_munged(user_ns, opts->fs_uid));
+	seq_printf(m, ",gid=%u",
+		  from_kgid_munged(user_ns, opts->fs_gid));
 	if (opts->fmask)
 		seq_printf(m, ",fmask=%04o", ~opts->fs_fmask_inv);
 	if (opts->dmask)
-- 
2.25.1

Re: [PATCH v3 9/9] fs/ntfs3: Show uid/gid always in show_options()

[Pali Rohár]

On Sunday 29 August 2021 12:56:14 Kari Argillander wrote:
> Show options should show option according documentation when some value
> is not default or when ever coder wants. Uid/gid are problematic because
> it is hard to know which are defaults. In file system there is many
> different implementation for this problem.
> 
> Some file systems show uid/gid when they are different than root, some
> when user has set them and some show them always. There is also problem
> that what if root uid/gid change. This code just choose to show them
> always. This way we do not need to think this any more.

Hello! IIRC ntfs disk storage supports POSIX permissions and uid/gid for
files and directories. But from ntfs3 documentation it is not clear if
this ntfs3 implementation supports it or not.

Currently ntfs3 documentation says:
> https://github.com/Paragon-Software-Group/linux-ntfs3/blob/master/Documentation/filesystems/ntfs3.rst
> uid= gid= umask= Controls the default permissions for
> files/directories created after the NTFS volume is mounted.
(and looks that rst formatting is broken)

And from this description I'm not really sure what these option
controls.

For example udf filesystem also supports storing POSIX uid/gid and
supports also additional extension per file to "do not store uid" and
"do not store gid". Moreover kernel implementation has mount option
(uid= and gid=) which overrides disk's uid/gid to mount option value.
And also has mount option to allow storing new files "without uid/gid".
So there does not have to be any default for uid=/gid= like it is for
"native" POSIX filesystems (e.g. ext4).

And so interpretation of uid/gid options is not always easy; specially
if filesystem has extensions to POSIX permissions. And NTFS has it too
as it by default has in its storage (only?) NT permissions and NT SIDs.

> Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
> ---
>  fs/ntfs3/ntfs_fs.h | 23 ++++++++++-------------
>  fs/ntfs3/super.c   | 12 ++++--------
>  2 files changed, 14 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
> index 5df55bc733bd..a3a7d10de7cb 100644
> --- a/fs/ntfs3/ntfs_fs.h
> +++ b/fs/ntfs3/ntfs_fs.h
> @@ -60,19 +60,16 @@ struct ntfs_mount_options {
>  	u16 fs_fmask_inv;
>  	u16 fs_dmask_inv;
>  
> -	unsigned uid : 1, /* uid was set */
> -		gid : 1, /* gid was set */
> -		fmask : 1, /* fmask was set */
> -		dmask : 1, /*dmask was set*/
> -		sys_immutable : 1, /* immutable system files */
> -		discard : 1, /* issue discard requests on deletions */
> -		sparse : 1, /*create sparse files*/
> -		showmeta : 1, /*show meta files*/
> -		nohidden : 1, /*do not show hidden files*/
> -		force : 1, /*rw mount dirty volume*/
> -		noacs_rules : 1, /*exclude acs rules*/
> -		prealloc : 1 /*preallocate space when file is growing*/
> -		;
> +	unsigned fmask : 1; /* fmask was set */
> +	unsigned dmask : 1; /*dmask was set*/
> +	unsigned sys_immutable : 1; /* immutable system files */
> +	unsigned discard : 1; /* issue discard requests on deletions */
> +	unsigned sparse : 1; /*create sparse files*/
> +	unsigned showmeta : 1; /*show meta files*/
> +	unsigned nohidden : 1; /*do not show hidden files*/
> +	unsigned force : 1; /*rw mount dirty volume*/
> +	unsigned noacs_rules : 1; /*exclude acs rules*/
> +	unsigned prealloc : 1; /*preallocate space when file is growing*/
>  };
>  
>  /* special value to unpack and deallocate*/
> diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
> index d7408b4f6813..d28fab6c2297 100644
> --- a/fs/ntfs3/super.c
> +++ b/fs/ntfs3/super.c
> @@ -287,13 +287,11 @@ static int ntfs_fs_parse_param(struct fs_context *fc,
>  		opts->fs_uid = make_kuid(current_user_ns(), result.uint_32);
>  		if (!uid_valid(opts->fs_uid))
>  			return invalf(fc, "ntfs3: Invalid value for uid.");
> -		opts->uid = 1;
>  		break;
>  	case Opt_gid:
>  		opts->fs_gid = make_kgid(current_user_ns(), result.uint_32);
>  		if (!gid_valid(opts->fs_gid))
>  			return invalf(fc, "ntfs3: Invalid value for gid.");
> -		opts->gid = 1;
>  		break;
>  	case Opt_umask:
>  		if (result.uint_32 & ~07777)
> @@ -512,12 +510,10 @@ static int ntfs_show_options(struct seq_file *m, struct dentry *root)
>  	struct ntfs_mount_options *opts = sbi->options;
>  	struct user_namespace *user_ns = seq_user_ns(m);
>  
> -	if (opts->uid)
> -		seq_printf(m, ",uid=%u",
> -			   from_kuid_munged(user_ns, opts->fs_uid));
> -	if (opts->gid)
> -		seq_printf(m, ",gid=%u",
> -			   from_kgid_munged(user_ns, opts->fs_gid));
> +	seq_printf(m, ",uid=%u",
> +		  from_kuid_munged(user_ns, opts->fs_uid));
> +	seq_printf(m, ",gid=%u",
> +		  from_kgid_munged(user_ns, opts->fs_gid));
>  	if (opts->fmask)
>  		seq_printf(m, ",fmask=%04o", ~opts->fs_fmask_inv);
>  	if (opts->dmask)
> -- 
> 2.25.1
> 

Re: [PATCH v3 0/9] fs/ntfs3: Use new mount api and change some opts

[Kari Argillander]

On Sun, Aug 29, 2021 at 12:56:05PM +0300, Kari Argillander wrote:
> See V2 if you want:
> lore.kernel.org/ntfs3/20210819002633.689831-1-kari.argillander@gmail.com
> 
> NLS change is now blocked when remounting. Christoph also suggest that
> we block all other mount options, but I have tested a couple and they
> seem to work. I wish that we do not block any other than NLS because
> in theory they should work. Also Konstantin can comment about this.
> 
> I have not include reviewed/acked to patch "Use new api for mounting"
> because it change so much. I have also included three new patch to this
> series:
> 	- Convert mount options to pointer in sbi
> 		So that we do not need to initiliaze whole spi in 
> 		remount.
> 	- Init spi more in init_fs_context than fill_super
> 		This is just refactoring. (Series does not depend on this)
> 	- Show uid/gid always in show_options()
> 		Christian Brauner kinda ask this. (Series does not depend
> 		on this)
> 
> Series is ones again tested with kvm-xfstests. Every commit is build
> tested.

I will send v4 within couple of days. It will address issues what Pali
says in patch 8/9. Everything else should be same at least for now. Is
everything else looking ok?

> 
> v3:
> 	- Add patch "Convert mount options to pointer in sbi"
> 	- Add patch "Init spi more in init_fs_context than fill_super"
> 	- Add patch "Show uid/gid always in show_options"
> 	- Patch "Use new api for mounting" has make over
> 	- NLS loading is not anymore possible when remounting
> 	- show_options() iocharset printing is fixed
> 	- Delete comment that testing should be done with other
> 	  mount options.
> 	- Add reviewed/acked-tags to 1,2,6,8 
> 	- Rewrite this cover
> v2:
> 	- Rewrite this cover leter
> 	- Reorder noatime to first patch
> 	- NLS loading with string
> 	- Delete default_options function
> 	- Remove remount flags
> 	- Rename no_acl_rules mount option
> 	- Making code cleaner
> 	- Add comment that mount options should be tested
> 
> Kari Argillander (9):
>   fs/ntfs3: Remove unnecesarry mount option noatime
>   fs/ntfs3: Remove unnecesarry remount flag handling
>   fs/ntfs3: Convert mount options to pointer in sbi
>   fs/ntfs3: Use new api for mounting
>   fs/ntfs3: Init spi more in init_fs_context than fill_super
>   fs/ntfs3: Make mount option nohidden more universal
>   fs/ntfs3: Add iocharset= mount option as alias for nls=
>   fs/ntfs3: Rename mount option no_acl_rules > (no)acl_rules
>   fs/ntfs3: Show uid/gid always in show_options()
> 
>  Documentation/filesystems/ntfs3.rst |  10 +-
>  fs/ntfs3/attrib.c                   |   2 +-
>  fs/ntfs3/dir.c                      |   8 +-
>  fs/ntfs3/file.c                     |   4 +-
>  fs/ntfs3/inode.c                    |  12 +-
>  fs/ntfs3/ntfs_fs.h                  |  26 +-
>  fs/ntfs3/super.c                    | 486 +++++++++++++++-------------
>  fs/ntfs3/xattr.c                    |   2 +-
>  8 files changed, 284 insertions(+), 266 deletions(-)
> 
> -- 
> 2.25.1
> 
> 

Re: [PATCH v3 0/9] fs/ntfs3: Use new mount api and change some opts

[Konstantin Komarov]

On 07.09.2021 10:36, Kari Argillander wrote:
> On Sun, Aug 29, 2021 at 12:56:05PM +0300, Kari Argillander wrote:
>> See V2 if you want:
>> lore.kernel.org/ntfs3/20210819002633.689831-1-kari.argillander@gmail.com
>>
>> NLS change is now blocked when remounting. Christoph also suggest that
>> we block all other mount options, but I have tested a couple and they
>> seem to work. I wish that we do not block any other than NLS because
>> in theory they should work. Also Konstantin can comment about this.
>>
>> I have not include reviewed/acked to patch "Use new api for mounting"
>> because it change so much. I have also included three new patch to this
>> series:
>> 	- Convert mount options to pointer in sbi
>> 		So that we do not need to initiliaze whole spi in 
>> 		remount.
>> 	- Init spi more in init_fs_context than fill_super
>> 		This is just refactoring. (Series does not depend on this)
>> 	- Show uid/gid always in show_options()
>> 		Christian Brauner kinda ask this. (Series does not depend
>> 		on this)
>>
>> Series is ones again tested with kvm-xfstests. Every commit is build
>> tested.
> 
> I will send v4 within couple of days. It will address issues what Pali
> says in patch 8/9. Everything else should be same at least for now. Is
> everything else looking ok?
> 

Yes, everything else seems good. 
We tested patches locally - no regression was found.

>>
>> v3:
>> 	- Add patch "Convert mount options to pointer in sbi"
>> 	- Add patch "Init spi more in init_fs_context than fill_super"
>> 	- Add patch "Show uid/gid always in show_options"
>> 	- Patch "Use new api for mounting" has make over
>> 	- NLS loading is not anymore possible when remounting
>> 	- show_options() iocharset printing is fixed
>> 	- Delete comment that testing should be done with other
>> 	  mount options.
>> 	- Add reviewed/acked-tags to 1,2,6,8 
>> 	- Rewrite this cover
>> v2:
>> 	- Rewrite this cover leter
>> 	- Reorder noatime to first patch
>> 	- NLS loading with string
>> 	- Delete default_options function
>> 	- Remove remount flags
>> 	- Rename no_acl_rules mount option
>> 	- Making code cleaner
>> 	- Add comment that mount options should be tested
>>
>> Kari Argillander (9):
>>   fs/ntfs3: Remove unnecesarry mount option noatime
>>   fs/ntfs3: Remove unnecesarry remount flag handling
>>   fs/ntfs3: Convert mount options to pointer in sbi
>>   fs/ntfs3: Use new api for mounting
>>   fs/ntfs3: Init spi more in init_fs_context than fill_super
>>   fs/ntfs3: Make mount option nohidden more universal
>>   fs/ntfs3: Add iocharset= mount option as alias for nls=
>>   fs/ntfs3: Rename mount option no_acl_rules > (no)acl_rules
>>   fs/ntfs3: Show uid/gid always in show_options()
>>
>>  Documentation/filesystems/ntfs3.rst |  10 +-
>>  fs/ntfs3/attrib.c                   |   2 +-
>>  fs/ntfs3/dir.c                      |   8 +-
>>  fs/ntfs3/file.c                     |   4 +-
>>  fs/ntfs3/inode.c                    |  12 +-
>>  fs/ntfs3/ntfs_fs.h                  |  26 +-
>>  fs/ntfs3/super.c                    | 486 +++++++++++++++-------------
>>  fs/ntfs3/xattr.c                    |   2 +-
>>  8 files changed, 284 insertions(+), 266 deletions(-)
>>
>> -- 
>> 2.25.1
>>
>>

Re: [PATCH v3 0/9] fs/ntfs3: Use new mount api and change some opts

[Kari Argillander]

On Tuesday, September 7, 2021, Andy Shevchenko
(andy.shevchenko@gmail.com) wrote:
> On Tuesday, September 7, 2021, Konstantin Komarov <almaz.alexandrovich@paragon-software.com> wrote:
>> On 07.09.2021 10:36, Kari Argillander wrote:
>> > On Sun, Aug 29, 2021 at 12:56:05PM +0300, Kari Argillander wrote:
>> >> See V2 if you want:
>> >> lore.kernel.org/ntfs3/20210819002633.689831-1-kari.argillander@gmail.com
>> >>
>> >> NLS change is now blocked when remounting. Christoph also suggest that
>> >> we block all other mount options, but I have tested a couple and they
>> >> seem to work. I wish that we do not block any other than NLS because
>> >> in theory they should work. Also Konstantin can comment about this.
>> >>
>> >> I have not include reviewed/acked to patch "Use new api for mounting"
>> >> because it change so much. I have also included three new patch to this
>> >> series:
>> >>      - Convert mount options to pointer in sbi
>> >>              So that we do not need to initiliaze whole spi in
>> >>              remount.
>> >>      - Init spi more in init_fs_context than fill_super
>> >>              This is just refactoring. (Series does not depend on this)
>> >>      - Show uid/gid always in show_options()
>> >>              Christian Brauner kinda ask this. (Series does not depend
>> >>              on this)
>> >>
>> >> Series is ones again tested with kvm-xfstests. Every commit is build
>> >> tested.
>> >
>> > I will send v4 within couple of days. It will address issues what Pali
>> > says in patch 8/9. Everything else should be same at least for now. Is
>> > everything else looking ok?
>> >
>>
>> Yes, everything else seems good.
>> We tested patches locally - no regression was
>
> The formal answer in such case should also contain the Tested-by tag. I would suggest you to read the Submitting Patches document (available in the Linux kernel source tree).

He is a maintainer so he can add tags when he picks this up. This is not
really relevant here. Yes it should be good to include that but I have already
sended v4 which he has not tested. So I really cannot put this tag for him.
So at the end he really should not even put it here.

Also usually the maintainers will always make their own tests and usually
they will not even bother with a tested-by tag. Or do you say to me that I
should go read Submitting Patches document as I'm the one who submit
this?

  Argillander

>> >>
>> >> v3:
>> >>      - Add patch "Convert mount options to pointer in sbi"
>> >>      - Add patch "Init spi more in init_fs_context than fill_super"
>> >>      - Add patch "Show uid/gid always in show_options"
>> >>      - Patch "Use new api for mounting" has make over
>> >>      - NLS loading is not anymore possible when remounting
>> >>      - show_options() iocharset printing is fixed
>> >>      - Delete comment that testing should be done with other
>> >>        mount options.
>> >>      - Add reviewed/acked-tags to 1,2,6,8
>> >>      - Rewrite this cover
>> >> v2:
>> >>      - Rewrite this cover leter
>> >>      - Reorder noatime to first patch
>> >>      - NLS loading with string
>> >>      - Delete default_options function
>> >>      - Remove remount flags
>> >>      - Rename no_acl_rules mount option
>> >>      - Making code cleaner
>> >>      - Add comment that mount options should be tested
>> >>
>> >> Kari Argillander (9):
>> >>   fs/ntfs3: Remove unnecesarry mount option noatime
>> >>   fs/ntfs3: Remove unnecesarry remount flag handling
>> >>   fs/ntfs3: Convert mount options to pointer in sbi
>> >>   fs/ntfs3: Use new api for mounting
>> >>   fs/ntfs3: Init spi more in init_fs_context than fill_super
>> >>   fs/ntfs3: Make mount option nohidden more universal
>> >>   fs/ntfs3: Add iocharset= mount option as alias for nls=
>> >>   fs/ntfs3: Rename mount option no_acl_rules > (no)acl_rules
>> >>   fs/ntfs3: Show uid/gid always in show_options()
>> >>
>> >>  Documentation/filesystems/ntfs3.rst |  10 +-
>> >>  fs/ntfs3/attrib.c                   |   2 +-
>> >>  fs/ntfs3/dir.c                      |   8 +-
>> >>  fs/ntfs3/file.c                     |   4 +-
>> >>  fs/ntfs3/inode.c                    |  12 +-
>> >>  fs/ntfs3/ntfs_fs.h                  |  26 +-
>> >>  fs/ntfs3/super.c                    | 486 +++++++++++++++-------------
>> >>  fs/ntfs3/xattr.c                    |   2 +-
>> >>  8 files changed, 284 insertions(+), 266 deletions(-)
>> >>
>> >> --
>> >> 2.25.1
>> >>
>> >>
>
>
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Re: [PATCH v3 0/9] fs/ntfs3: Use new mount api and change some opts

[Andy Shevchenko]

On Tue, Sep 7, 2021 at 11:47 PM Kari Argillander
<kari.argillander@gmail.com> wrote:
> On Tuesday, September 7, 2021, Andy Shevchenko
> (andy.shevchenko@gmail.com) wrote:
> > On Tuesday, September 7, 2021, Konstantin Komarov <almaz.alexandrovich@paragon-software.com> wrote:
> >> On 07.09.2021 10:36, Kari Argillander wrote:

...

> >> Yes, everything else seems good.
> >> We tested patches locally - no regression was
> >
> > The formal answer in such case should also contain the Tested-by tag. I would suggest you to read the Submitting Patches document (available in the Linux kernel source tree).
>
> He is a maintainer so he can add tags when he picks this up.

It's a good practice to do so. Moreover, it's better to do it
patch-by-patch, so tools like `b4` can cope with tags for *anybody*
who will use it in automated way.

> This is not
> really relevant here.

Why not?

> Yes it should be good to include that but I have already
> sended v4 which he has not tested. So I really cannot put this tag for him.
> So at the end he really should not even put it here.

For v4 I agree with you.

> Also usually the maintainers will always make their own tests and usually
> they will not even bother with a tested-by tag.

If it's their own code, yes, if it's others', why not? See above as well.

> Or do you say to me that I
> should go read Submitting Patches document as I'm the one who submit
> this?

It's always good to refresh memory, so why not? :-)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 0/9] fs/ntfs3: Use new mount api and change some opts

[Konstantin Komarov]

On 08.09.2021 12:00, Andy Shevchenko wrote:
> On Tue, Sep 7, 2021 at 11:47 PM Kari Argillander
> <kari.argillander@gmail.com> wrote:
>> On Tuesday, September 7, 2021, Andy Shevchenko
>> (andy.shevchenko@gmail.com) wrote:
>>> On Tuesday, September 7, 2021, Konstantin Komarov <almaz.alexandrovich@paragon-software.com> wrote:
>>>> On 07.09.2021 10:36, Kari Argillander wrote:
> 
> ...
> 
>>>> Yes, everything else seems good.
>>>> We tested patches locally - no regression was
>>>
>>> The formal answer in such case should also contain the Tested-by tag. I would suggest you to read the Submitting Patches document (available in the Linux kernel source tree).
>>
>> He is a maintainer so he can add tags when he picks this up.
> 
> It's a good practice to do so. Moreover, it's better to do it
> patch-by-patch, so tools like `b4` can cope with tags for *anybody*
> who will use it in automated way.
> 
>> This is not
>> really relevant here.
> 
> Why not?
> 
>> Yes it should be good to include that but I have already
>> sended v4 which he has not tested. So I really cannot put this tag for him.
>> So at the end he really should not even put it here.
> 
> For v4 I agree with you.

My answer doesn't contain Tested-by tag because author of patch already said
that there will be next version of patch.
Thanks for Submitting Patches document suggestion.

> 
>> Also usually the maintainers will always make their own tests and usually
>> they will not even bother with a tested-by tag.
> 
> If it's their own code, yes, if it's others', why not? See above as well.
> 
>> Or do you say to me that I
>> should go read Submitting Patches document as I'm the one who submit
>> this?
> 
> It's always good to refresh memory, so why not? :-)
>