[f2fs-dev] [PATCH 0/9] f2fs: first steps towards mount API conversion

[f2fs-dev] [PATCH 0/9] f2fs: first steps towards mount API conversion

[Eric Sandeen]

I have been struggling to get to a good series to convert f2fs to the
new mount API. f2fs is more complex, because much of the option parsing
assumes that the superblock has already been read from disk, and uses
that to test various on-disk features, etc. All of those tests will need
to be moved to after parsing is complete, and this series is just a
start.

The first two patches in this series are incidental, just things I
noticed when working on this. They are not critical to the conversion,
but they may be desirable anyway.

The rest of the patches move towards removal of explicit references to
*sb in parse_options(), using *sbi instead. (The full conversion may use
a private context structure instead of *sbi, since the *sbi is rather
large.)

It's up to you if you want to merge these now or not, but I thought I'd
share the direction I was moving, to get some feedback about whether
this seems to make sense. Next steps would be moving more of the feature
checks to later in the mount process, after parsing is complete.

This has been tested with random combinations of valid and invalid mount
options, but it has not been tested with a wide range of on-disk
features. My testing did not turn up any differences in behavior.
(I also did explicit testing of direct mount syscalls with "lazytime" as
an option string, keeping in mind the earlier regression there.)

Thanks,
-Eric



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH 1/9] f2fs: use f2fs_sb_has_device_alias during option parsing

[Eric Sandeen]

Rather than using F2FS_HAS_FEATURE directly, use f2fs_sb_has_device_alias
macro during option parsing for consistency.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/f2fs/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 19b67828ae32..dd35d199775a 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -838,7 +838,7 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
 			set_opt(sbi, READ_EXTENT_CACHE);
 			break;
 		case Opt_noextent_cache:
-			if (F2FS_HAS_FEATURE(sbi, F2FS_FEATURE_DEVICE_ALIAS)) {
+			if (f2fs_sb_has_device_alias(sbi)) {
 				f2fs_err(sbi, "device aliasing requires extent cache");
 				return -EINVAL;
 			}
-- 
2.48.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH 2/9] f2fs: consolidate unsupported option handling errors

[Eric Sandeen]

When certain build-time options are disabled, some mount options are not
accepted. For quota and compression, all related options are dismissed
with a single error message. For xattr, acl, and fault injection, each
option is handled individually. In addition, inline_xattr_size was missed
when CONFIG_F2FS_FS_XATTR was disabled.

Collapse xattr, acl, and fault injection errors into a single string, for
simplicity, and handle the missing inline_xattr_size case.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/f2fs/super.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index dd35d199775a..29b3aa1ee99c 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -772,16 +772,11 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
 			break;
 #else
 		case Opt_user_xattr:
-			f2fs_info(sbi, "user_xattr options not supported");
-			break;
 		case Opt_nouser_xattr:
-			f2fs_info(sbi, "nouser_xattr options not supported");
-			break;
 		case Opt_inline_xattr:
-			f2fs_info(sbi, "inline_xattr options not supported");
-			break;
 		case Opt_noinline_xattr:
-			f2fs_info(sbi, "noinline_xattr options not supported");
+		case Opt_inline_xattr_size:
+			f2fs_info(sbi, "xattr options not supported");
 			break;
 #endif
 #ifdef CONFIG_F2FS_FS_POSIX_ACL
@@ -793,10 +788,8 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
 			break;
 #else
 		case Opt_acl:
-			f2fs_info(sbi, "acl options not supported");
-			break;
 		case Opt_noacl:
-			f2fs_info(sbi, "noacl options not supported");
+			f2fs_info(sbi, "acl options not supported");
 			break;
 #endif
 		case Opt_active_logs:
@@ -919,11 +912,8 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
 			break;
 #else
 		case Opt_fault_injection:
-			f2fs_info(sbi, "fault_injection options not supported");
-			break;
-
 		case Opt_fault_type:
-			f2fs_info(sbi, "fault_type options not supported");
+			f2fs_info(sbi, "fault injection options not supported");
 			break;
 #endif
 		case Opt_lazytime:
-- 
2.48.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH 3/9] f2fs: factor out an f2fs_default_check function

[Eric Sandeen]

From: Eric Sandeen <sandeen@sandeen.net>

The current options parsing function both parses options and validates
them - factor the validation out to reduce the size of the function and
make transition to the new mount API possible, because under the new mount
API, options are parsed one at a time, and cannot all be tested at the end
of the parsing function.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/f2fs/super.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 29b3aa1ee99c..7cfd5e4e806e 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -687,7 +687,7 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
 	int ret;
 
 	if (!options)
-		goto default_check;
+		return 0;
 
 	while ((p = strsep(&options, ",")) != NULL) {
 		int token;
@@ -1318,7 +1318,11 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
 			return -EINVAL;
 		}
 	}
-default_check:
+	return 0;
+}
+
+static int f2fs_default_check(struct f2fs_sb_info *sbi)
+{
 #ifdef CONFIG_QUOTA
 	if (f2fs_check_quota_options(sbi))
 		return -EINVAL;
@@ -2364,6 +2368,10 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
 	}
 #endif
 
+	err = f2fs_default_check(sbi);
+	if (err)
+		goto restore_opts;
+
 	/* flush outstanding errors before changing fs state */
 	flush_work(&sbi->s_error_work);
 
@@ -4489,6 +4497,10 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 	if (err)
 		goto free_options;
 
+	err = f2fs_default_check(sbi);
+	if (err)
+		goto free_options;
+
 	sb->s_maxbytes = max_file_blocks(NULL) <<
 				le32_to_cpu(raw_super->log_blocksize);
 	sb->s_max_links = F2FS_LINK_MAX;
-- 
2.48.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH 4/9] f2fs: make INLINECRYPT a mount option flag

[Eric Sandeen]

From: Eric Sandeen <sandeen@sandeen.net>

Set INLINECRYPT into sbi during parsing, and transfer it to the sb in
fill_super, so that an sb is not required during option parsing.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/f2fs/f2fs.h  | 1 +
 fs/f2fs/super.c | 5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 1afa7be16e7d..15e4f5a77eb5 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -114,6 +114,7 @@ extern const char *f2fs_fault_name[FAULT_MAX];
 #define	F2FS_MOUNT_GC_MERGE		0x02000000
 #define F2FS_MOUNT_COMPRESS_CACHE	0x04000000
 #define F2FS_MOUNT_AGE_EXTENT_CACHE	0x08000000
+#define F2FS_MOUNT_INLINECRYPT		0x10000000
 
 #define F2FS_OPTION(sbi)	((sbi)->mount_opt)
 #define clear_opt(sbi, option)	(F2FS_OPTION(sbi).opt &= ~F2FS_MOUNT_##option)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 7cfd5e4e806e..643d19bbc156 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1036,7 +1036,7 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
 			break;
 		case Opt_inlinecrypt:
 #ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT
-			sb->s_flags |= SB_INLINECRYPT;
+			set_opt(sbi, INLINECRYPT);
 #else
 			f2fs_info(sbi, "inline encryption not supported");
 #endif
@@ -4535,6 +4535,9 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_time_gran = 1;
 	sb->s_flags = (sb->s_flags & ~SB_POSIXACL) |
 		(test_opt(sbi, POSIX_ACL) ? SB_POSIXACL : 0);
+	if (test_opt(sbi, INLINECRYPT))
+		sb->s_flags |= SB_INLINECRYPT;
+
 	super_set_uuid(sb, (void *) raw_super->uuid, sizeof(raw_super->uuid));
 	super_set_sysfs_name_bdev(sb);
 	sb->s_iflags |= SB_I_CGROUPWB;
-- 
2.48.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH 5/9] f2fs: make LAZYTIME a mount option flag

[Eric Sandeen]

Set LAZYTIME into sbi during parsing, and transfer it to the sb in
fill_super, so that an sb is not required during option parsing.

(Note: While lazytime is normally handled via mount flag in the vfs,
some f2fs users do expect to be able to use it as an explicit mount
option string via the mount syscall, so this option must remain.)

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/f2fs/f2fs.h  |  5 +++++
 fs/f2fs/super.c | 11 ++++++++---
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 15e4f5a77eb5..5c83e3a558f9 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -115,6 +115,11 @@ extern const char *f2fs_fault_name[FAULT_MAX];
 #define F2FS_MOUNT_COMPRESS_CACHE	0x04000000
 #define F2FS_MOUNT_AGE_EXTENT_CACHE	0x08000000
 #define F2FS_MOUNT_INLINECRYPT		0x10000000
+/*
+ * Some f2fs environments expect to be able to pass the "lazytime" option
+ * string rather than using the MS_LAZYTIME flag, so this must remain.
+ */
+#define F2FS_MOUNT_LAZYTIME		0x20000000
 
 #define F2FS_OPTION(sbi)	((sbi)->mount_opt)
 #define clear_opt(sbi, option)	(F2FS_OPTION(sbi).opt &= ~F2FS_MOUNT_##option)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 643d19bbc156..e63b3bd75f85 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -917,10 +917,10 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
 			break;
 #endif
 		case Opt_lazytime:
-			sb->s_flags |= SB_LAZYTIME;
+			set_opt(sbi, LAZYTIME);
 			break;
 		case Opt_nolazytime:
-			sb->s_flags &= ~SB_LAZYTIME;
+			clear_opt(sbi, LAZYTIME);
 			break;
 #ifdef CONFIG_QUOTA
 		case Opt_quota:
@@ -2169,8 +2169,8 @@ static void default_options(struct f2fs_sb_info *sbi, bool remount)
 	set_opt(sbi, INLINE_DATA);
 	set_opt(sbi, INLINE_DENTRY);
 	set_opt(sbi, MERGE_CHECKPOINT);
+	set_opt(sbi, LAZYTIME);
 	F2FS_OPTION(sbi).unusable_cap = 0;
-	sbi->sb->s_flags |= SB_LAZYTIME;
 	if (!f2fs_is_readonly(sbi))
 		set_opt(sbi, FLUSH_MERGE);
 	if (f2fs_sb_has_blkzoned(sbi))
@@ -4538,6 +4538,11 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 	if (test_opt(sbi, INLINECRYPT))
 		sb->s_flags |= SB_INLINECRYPT;
 
+	if (test_opt(sbi, LAZYTIME))
+		sb->s_flags |= SB_LAZYTIME;
+	else
+		sb->s_flags &= ~SB_LAZYTIME;
+
 	super_set_uuid(sb, (void *) raw_super->uuid, sizeof(raw_super->uuid));
 	super_set_sysfs_name_bdev(sb);
 	sb->s_iflags |= SB_I_CGROUPWB;
-- 
2.48.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH 6/9] f2fs: Pass sbi rather than sb to f2fs_set_test_dummy_encryption

[Eric Sandeen]

This removes another sb instance from parse_options()

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/f2fs/super.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index e63b3bd75f85..8866a74ce6aa 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -483,12 +483,11 @@ static int f2fs_check_quota_options(struct f2fs_sb_info *sbi)
 }
 #endif
 
-static int f2fs_set_test_dummy_encryption(struct super_block *sb,
+static int f2fs_set_test_dummy_encryption(struct f2fs_sb_info *sbi,
 					  const char *opt,
 					  const substring_t *arg,
 					  bool is_remount)
 {
-	struct f2fs_sb_info *sbi = F2FS_SB(sb);
 	struct fs_parameter param = {
 		.type = fs_value_is_string,
 		.string = arg->from ? arg->from : "",
@@ -1029,7 +1028,7 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
 			kfree(name);
 			break;
 		case Opt_test_dummy_encryption:
-			ret = f2fs_set_test_dummy_encryption(sb, p, &args[0],
+			ret = f2fs_set_test_dummy_encryption(sbi, p, &args[0],
 							     is_remount);
 			if (ret)
 				return ret;
-- 
2.48.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH 7/9] f2fs: defer readonly check vs norecovery

[Eric Sandeen]

Defer the readonly-vs-norecovery check until after option parsing is done
so that option parsing does not require an active superblock for the test.
Add a helpful message, while we're at it.

(I think could be moved back into parsing after we switch to the new mount
API if desired, as the fs context will have RO state available.)

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/f2fs/super.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 8866a74ce6aa..bc1aab749689 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -727,10 +727,8 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
 			set_opt(sbi, DISABLE_ROLL_FORWARD);
 			break;
 		case Opt_norecovery:
-			/* this option mounts f2fs with ro */
+			/* requires ro mount, checked in f2fs_default_check */
 			set_opt(sbi, NORECOVERY);
-			if (!f2fs_readonly(sb))
-				return -EINVAL;
 			break;
 		case Opt_discard:
 			if (!f2fs_hw_support_discard(sbi)) {
@@ -1411,6 +1409,12 @@ static int f2fs_default_check(struct f2fs_sb_info *sbi)
 		f2fs_err(sbi, "Allow to mount readonly mode only");
 		return -EROFS;
 	}
+
+	if (test_opt(sbi, NORECOVERY) && !f2fs_readonly(sbi->sb)) {
+		f2fs_err(sbi, "norecovery requires readonly mount");
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
-- 
2.48.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH 8/9] f2fs: pass sbi rather than sb to quota qf_name helpers

[Eric Sandeen]

With the new mount api we will not have the superblock available during
option parsing. Prepare for this by passing *sbi rather than *sb.

For now, we are parsing after fill_super has been done, so sbi->sb will
exist. Under the new mount API this will require more care, but do the
simple change for now.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/f2fs/super.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index bc1aab749689..9edb200caae7 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -383,10 +383,10 @@ static void init_once(void *foo)
 #ifdef CONFIG_QUOTA
 static const char * const quotatypes[] = INITQFNAMES;
 #define QTYPE2NAME(t) (quotatypes[t])
-static int f2fs_set_qf_name(struct super_block *sb, int qtype,
+static int f2fs_set_qf_name(struct f2fs_sb_info *sbi, int qtype,
 							substring_t *args)
 {
-	struct f2fs_sb_info *sbi = F2FS_SB(sb);
+	struct super_block *sb = sbi->sb;
 	char *qname;
 	int ret = -EINVAL;
 
@@ -424,9 +424,9 @@ static int f2fs_set_qf_name(struct super_block *sb, int qtype,
 	return ret;
 }
 
-static int f2fs_clear_qf_name(struct super_block *sb, int qtype)
+static int f2fs_clear_qf_name(struct f2fs_sb_info *sbi, int qtype)
 {
-	struct f2fs_sb_info *sbi = F2FS_SB(sb);
+	struct super_block *sb = sbi->sb;
 
 	if (sb_any_quota_loaded(sb) && F2FS_OPTION(sbi).s_qf_names[qtype]) {
 		f2fs_err(sbi, "Cannot change journaled quota options when quota turned on");
@@ -931,32 +931,32 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
 			set_opt(sbi, PRJQUOTA);
 			break;
 		case Opt_usrjquota:
-			ret = f2fs_set_qf_name(sb, USRQUOTA, &args[0]);
+			ret = f2fs_set_qf_name(sbi, USRQUOTA, &args[0]);
 			if (ret)
 				return ret;
 			break;
 		case Opt_grpjquota:
-			ret = f2fs_set_qf_name(sb, GRPQUOTA, &args[0]);
+			ret = f2fs_set_qf_name(sbi, GRPQUOTA, &args[0]);
 			if (ret)
 				return ret;
 			break;
 		case Opt_prjjquota:
-			ret = f2fs_set_qf_name(sb, PRJQUOTA, &args[0]);
+			ret = f2fs_set_qf_name(sbi, PRJQUOTA, &args[0]);
 			if (ret)
 				return ret;
 			break;
 		case Opt_offusrjquota:
-			ret = f2fs_clear_qf_name(sb, USRQUOTA);
+			ret = f2fs_clear_qf_name(sbi, USRQUOTA);
 			if (ret)
 				return ret;
 			break;
 		case Opt_offgrpjquota:
-			ret = f2fs_clear_qf_name(sb, GRPQUOTA);
+			ret = f2fs_clear_qf_name(sbi, GRPQUOTA);
 			if (ret)
 				return ret;
 			break;
 		case Opt_offprjjquota:
-			ret = f2fs_clear_qf_name(sb, PRJQUOTA);
+			ret = f2fs_clear_qf_name(sbi, PRJQUOTA);
 			if (ret)
 				return ret;
 			break;
-- 
2.48.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH 9/9] f2fs: pass sbi rather than sb to parse_options()

[Eric Sandeen]

With the new mount API the sb will not be available during initial option
parsing, which will happen before fill_super reads sb from disk.

Now that the sb is no longer directly referenced in parse_options, switch
it to use sbi.

(Note that all calls to f2fs_sb_has_* originating from parse_options will
need to be deferred to later before we can use the new mount API.)

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/f2fs/super.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 9edb200caae7..579c96a80fe2 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -670,9 +670,8 @@ static int f2fs_set_zstd_level(struct f2fs_sb_info *sbi, const char *str)
 #endif
 #endif
 
-static int parse_options(struct super_block *sb, char *options, bool is_remount)
+static int parse_options(struct f2fs_sb_info *sbi, char *options, bool is_remount)
 {
-	struct f2fs_sb_info *sbi = F2FS_SB(sb);
 	substring_t args[MAX_OPT_ARGS];
 #ifdef CONFIG_F2FS_FS_COMPRESSION
 	unsigned char (*ext)[F2FS_EXTENSION_LEN];
@@ -2356,7 +2355,7 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
 	default_options(sbi, true);
 
 	/* parse mount options */
-	err = parse_options(sb, data, true);
+	err = parse_options(sbi, data, true);
 	if (err)
 		goto restore_opts;
 
@@ -4496,7 +4495,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 		goto free_sb_buf;
 	}
 
-	err = parse_options(sb, options, false);
+	err = parse_options(sbi, options, false);
 	if (err)
 		goto free_options;
 
-- 
2.48.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 1/9] f2fs: use f2fs_sb_has_device_alias during option parsing

[Chao Yu via Linux-f2fs-devel]

On 3/4/25 01:12, Eric Sandeen wrote:
> Rather than using F2FS_HAS_FEATURE directly, use f2fs_sb_has_device_alias
> macro during option parsing for consistency.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 2/9] f2fs: consolidate unsupported option handling errors

[Chao Yu via Linux-f2fs-devel]

On 3/4/25 01:12, Eric Sandeen wrote:
> When certain build-time options are disabled, some mount options are not
> accepted. For quota and compression, all related options are dismissed
> with a single error message. For xattr, acl, and fault injection, each
> option is handled individually. In addition, inline_xattr_size was missed
> when CONFIG_F2FS_FS_XATTR was disabled.
> 
> Collapse xattr, acl, and fault injection errors into a single string, for
> simplicity, and handle the missing inline_xattr_size case.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 3/9] f2fs: factor out an f2fs_default_check function

[Chao Yu via Linux-f2fs-devel]

On 3/4/25 01:12, Eric Sandeen wrote:
> From: Eric Sandeen <sandeen@sandeen.net>
> 
> The current options parsing function both parses options and validates
> them - factor the validation out to reduce the size of the function and
> make transition to the new mount API possible, because under the new mount
> API, options are parsed one at a time, and cannot all be tested at the end
> of the parsing function.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>  fs/f2fs/super.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 29b3aa1ee99c..7cfd5e4e806e 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -687,7 +687,7 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
>  	int ret;
>  
>  	if (!options)
> -		goto default_check;

Eric, do you know in which condition options can be NULL, mount w/o any
specified options?

If so, maybe we'd keep this in order to chech whether default options
generated from default_options() is conflicted or not? What do you think?

Thanks,

> +		return 0;
>  
>  	while ((p = strsep(&options, ",")) != NULL) {
>  		int token;
> @@ -1318,7 +1318,11 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
>  			return -EINVAL;
>  		}
>  	}
> -default_check:
> +	return 0;
> +}
> +
> +static int f2fs_default_check(struct f2fs_sb_info *sbi)
> +{
>  #ifdef CONFIG_QUOTA
>  	if (f2fs_check_quota_options(sbi))
>  		return -EINVAL;
> @@ -2364,6 +2368,10 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
>  	}
>  #endif
>  
> +	err = f2fs_default_check(sbi);
> +	if (err)
> +		goto restore_opts;
> +
>  	/* flush outstanding errors before changing fs state */
>  	flush_work(&sbi->s_error_work);
>  
> @@ -4489,6 +4497,10 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>  	if (err)
>  		goto free_options;
>  
> +	err = f2fs_default_check(sbi);
> +	if (err)
> +		goto free_options;
> +
>  	sb->s_maxbytes = max_file_blocks(NULL) <<
>  				le32_to_cpu(raw_super->log_blocksize);
>  	sb->s_max_links = F2FS_LINK_MAX;



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 4/9] f2fs: make INLINECRYPT a mount option flag

[Chao Yu via Linux-f2fs-devel]

On 3/4/25 01:12, Eric Sandeen wrote:
> From: Eric Sandeen <sandeen@sandeen.net>
> 
> Set INLINECRYPT into sbi during parsing, and transfer it to the sb in
> fill_super, so that an sb is not required during option parsing.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 5/9] f2fs: make LAZYTIME a mount option flag

[Chao Yu via Linux-f2fs-devel]

On 3/4/25 01:12, Eric Sandeen wrote:
> Set LAZYTIME into sbi during parsing, and transfer it to the sb in
> fill_super, so that an sb is not required during option parsing.
> 
> (Note: While lazytime is normally handled via mount flag in the vfs,
> some f2fs users do expect to be able to use it as an explicit mount
> option string via the mount syscall, so this option must remain.)
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 6/9] f2fs: Pass sbi rather than sb to f2fs_set_test_dummy_encryption

[Chao Yu via Linux-f2fs-devel]

On 3/4/25 01:12, Eric Sandeen wrote:
> This removes another sb instance from parse_options()
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 7/9] f2fs: defer readonly check vs norecovery

[Chao Yu via Linux-f2fs-devel]

On 3/4/25 01:12, Eric Sandeen wrote:
> Defer the readonly-vs-norecovery check until after option parsing is done
> so that option parsing does not require an active superblock for the test.
> Add a helpful message, while we're at it.
> 
> (I think could be moved back into parsing after we switch to the new mount
> API if desired, as the fs context will have RO state available.)
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 8/9] f2fs: pass sbi rather than sb to quota qf_name helpers

[Chao Yu via Linux-f2fs-devel]

On 3/4/25 01:12, Eric Sandeen wrote:
> With the new mount api we will not have the superblock available during
> option parsing. Prepare for this by passing *sbi rather than *sb.
> 
> For now, we are parsing after fill_super has been done, so sbi->sb will
> exist. Under the new mount API this will require more care, but do the
> simple change for now.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 9/9] f2fs: pass sbi rather than sb to parse_options()

[Chao Yu via Linux-f2fs-devel]

On 3/4/25 01:12, Eric Sandeen wrote:
> With the new mount API the sb will not be available during initial option
> parsing, which will happen before fill_super reads sb from disk.
> 
> Now that the sb is no longer directly referenced in parse_options, switch
> it to use sbi.
> 
> (Note that all calls to f2fs_sb_has_* originating from parse_options will
> need to be deferred to later before we can use the new mount API.)
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 3/9] f2fs: factor out an f2fs_default_check function

[Eric Sandeen]

On 3/11/25 10:10 PM, Chao Yu wrote:
> On 3/4/25 01:12, Eric Sandeen wrote:
>> From: Eric Sandeen <sandeen@sandeen.net>
>>
>> The current options parsing function both parses options and validates
>> them - factor the validation out to reduce the size of the function and
>> make transition to the new mount API possible, because under the new mount
>> API, options are parsed one at a time, and cannot all be tested at the end
>> of the parsing function.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>  fs/f2fs/super.c | 16 ++++++++++++++--
>>  1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index 29b3aa1ee99c..7cfd5e4e806e 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -687,7 +687,7 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
>>  	int ret;
>>  
>>  	if (!options)
>> -		goto default_check;
> 
> Eric, do you know in which condition options can be NULL, mount w/o any
> specified options?
> 
> If so, maybe we'd keep this in order to chech whether default options
> generated from default_options() is conflicted or not? What do you think?
> 
> Thanks,

Yes, that's I think that is correct - (!options) is true when no f2fs-specific
options are present for parsing.

However, I think that we do still check default options with my patch in this
case, because both calls to parse_options() still call f2fs_default_check()
when parse_options() completes.

Or am I misunderstanding your question?

I added printks to check:

# mount -o loop,ro  f2fsfile.img mnt
[root@fedora-rawhide f2fs-test]# dmesg 
[847946.326384] loop2: detected capacity change from 0 to 819200
[847946.337625] parse_options: (!options) is true
[847946.337637] enter f2fs_default_check()

Thank you for reviewing this series. I think at least the first 2 or 3 patches
are suitable for merge now, if you agree. I am fairly certain that the rest of
them are proper steps towards mount API conversion as well, but as I have not
yet finished the work, I can't guarantee it yet. :)

Thanks,
-Eric



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 3/9] f2fs: factor out an f2fs_default_check function

[Chao Yu via Linux-f2fs-devel]

On 3/12/25 21:29, Eric Sandeen wrote:
> On 3/11/25 10:10 PM, Chao Yu wrote:
>> On 3/4/25 01:12, Eric Sandeen wrote:
>>> From: Eric Sandeen <sandeen@sandeen.net>
>>>
>>> The current options parsing function both parses options and validates
>>> them - factor the validation out to reduce the size of the function and
>>> make transition to the new mount API possible, because under the new mount
>>> API, options are parsed one at a time, and cannot all be tested at the end
>>> of the parsing function.
>>>
>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>> ---
>>>  fs/f2fs/super.c | 16 ++++++++++++++--
>>>  1 file changed, 14 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>> index 29b3aa1ee99c..7cfd5e4e806e 100644
>>> --- a/fs/f2fs/super.c
>>> +++ b/fs/f2fs/super.c
>>> @@ -687,7 +687,7 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
>>>  	int ret;
>>>  
>>>  	if (!options)
>>> -		goto default_check;
>>
>> Eric, do you know in which condition options can be NULL, mount w/o any
>> specified options?
>>
>> If so, maybe we'd keep this in order to chech whether default options
>> generated from default_options() is conflicted or not? What do you think?
>>
>> Thanks,
> 
> Yes, that's I think that is correct - (!options) is true when no f2fs-specific
> options are present for parsing.
> 
> However, I think that we do still check default options with my patch in this
> case, because both calls to parse_options() still call f2fs_default_check()
> when parse_options() completes.
> 
> Or am I misunderstanding your question?
> 
> I added printks to check:
> 
> # mount -o loop,ro  f2fsfile.img mnt
> [root@fedora-rawhide f2fs-test]# dmesg 
> [847946.326384] loop2: detected capacity change from 0 to 819200
> [847946.337625] parse_options: (!options) is true
> [847946.337637] enter f2fs_default_check()

Oh, I missed that we will call f2fs_default_check() after default_options()
anyway, so I guess it's fine here.

Thanks for your confirmation.

> 
> Thank you for reviewing this series. I think at least the first 2 or 3 patches
> are suitable for merge now, if you agree. I am fairly certain that the rest of

I've reviewed other patches, they look clean to me, Jaegeuk has merged them into
dev-test branch, let's test them for a while.

> them are proper steps towards mount API conversion as well, but as I have not
> yet finished the work, I can't guarantee it yet. :)

Thanks for the effect, and good to see progress on changing to use new
mount API. :)

Thanks,

> 
> Thanks,
> -Eric
> 



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 3/9] f2fs: factor out an f2fs_default_check function

[Chao Yu via Linux-f2fs-devel]

On 3/4/25 01:12, Eric Sandeen wrote:
> From: Eric Sandeen <sandeen@sandeen.net>
> 
> The current options parsing function both parses options and validates
> them - factor the validation out to reduce the size of the function and
> make transition to the new mount API possible, because under the new mount
> API, options are parsed one at a time, and cannot all be tested at the end
> of the parsing function.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 0/9] f2fs: first steps towards mount API conversion

[patchwork-bot+f2fs--- via Linux-f2fs-devel]

Hello:

This series was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <jaegeuk@kernel.org>:

On Mon,  3 Mar 2025 11:12:10 -0600 you wrote:
> I have been struggling to get to a good series to convert f2fs to the
> new mount API. f2fs is more complex, because much of the option parsing
> assumes that the superblock has already been read from disk, and uses
> that to test various on-disk features, etc. All of those tests will need
> to be moved to after parsing is complete, and this series is just a
> start.
> 
> [...]

Here is the summary with links:
  - [f2fs-dev,1/9] f2fs: use f2fs_sb_has_device_alias during option parsing
    https://git.kernel.org/jaegeuk/f2fs/c/64ee7503cbf6
  - [f2fs-dev,2/9] f2fs: consolidate unsupported option handling errors
    https://git.kernel.org/jaegeuk/f2fs/c/277352b6cbed
  - [f2fs-dev,3/9] f2fs: factor out an f2fs_default_check function
    https://git.kernel.org/jaegeuk/f2fs/c/abd0e040e9a5
  - [f2fs-dev,4/9] f2fs: make INLINECRYPT a mount option flag
    (no matching commit)
  - [f2fs-dev,5/9] f2fs: make LAZYTIME a mount option flag
    (no matching commit)
  - [f2fs-dev,6/9] f2fs: Pass sbi rather than sb to f2fs_set_test_dummy_encryption
    https://git.kernel.org/jaegeuk/f2fs/c/0edcb2197e76
  - [f2fs-dev,7/9] f2fs: defer readonly check vs norecovery
    https://git.kernel.org/jaegeuk/f2fs/c/9cca49875997
  - [f2fs-dev,8/9] f2fs: pass sbi rather than sb to quota qf_name helpers
    https://git.kernel.org/jaegeuk/f2fs/c/b7de231b9df4
  - [f2fs-dev,9/9] f2fs: pass sbi rather than sb to parse_options()
    https://git.kernel.org/jaegeuk/f2fs/c/71e9bd3d5c04

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html




_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 0/9] f2fs: first steps towards mount API conversion

[Eric Sandeen via Linux-f2fs-devel]

I was working on next steps for this, and I have a followup question.

Today, several mount options are simply ignored if the on-disk format
does not support them. For example:

                case Opt_compress_mode:
                        if (!f2fs_sb_has_compression(sbi)) {
                                f2fs_info(sbi, "Image doesn't support compression");
                                break;
                        }
                        name = match_strdup(&args[0]);
                        if (!name)
                                return -ENOMEM;
                        if (!strcmp(name, "fs")) {
                                F2FS_OPTION(sbi).compress_mode = COMPR_MODE_FS;
                        } else if (!strcmp(name, "user")) {
                                F2FS_OPTION(sbi).compress_mode = COMPR_MODE_USER;
                        } else {
                                kfree(name);
                                return -EINVAL;
                        }
                        kfree(name);
                        break;

so if f2fs_sb_has_compression() is not true, then the option is ignored without
any validation.

in other words, "mount -o compress_mode=nope ..." will succeed if the feature
is disabled on the filesystem.

If I move the f2fs_sb_has_compression() check to later for the new mount API,
then "mount -o compress_mode=nope ..."  will start failing for all images. Is
this acceptable? It seems wise to reject invalid options rather than ignore them,
even if they are incompatible with the format, but this would be a behavior
change.

The above would be simple enough to defer (maybe set to COMPR_MODE_INVAL and
reject it later) but I think other options such as compress/nocompress extensions
would be very messy to approach as "accept all options given during parsing,
and validate them later only if the corresponding feature is present."

So I wonder if a behavior change (stricter option validation) would be
acceptable here?

Thanks,
-Eric



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 0/9] f2fs: first steps towards mount API conversion

[Chao Yu via Linux-f2fs-devel]

On 3/29/25 12:18, Eric Sandeen wrote:
> I was working on next steps for this, and I have a followup question.
> 
> Today, several mount options are simply ignored if the on-disk format
> does not support them. For example:
> 
>                 case Opt_compress_mode:
>                         if (!f2fs_sb_has_compression(sbi)) {
>                                 f2fs_info(sbi, "Image doesn't support compression");
>                                 break;
>                         }
>                         name = match_strdup(&args[0]);
>                         if (!name)
>                                 return -ENOMEM;
>                         if (!strcmp(name, "fs")) {
>                                 F2FS_OPTION(sbi).compress_mode = COMPR_MODE_FS;
>                         } else if (!strcmp(name, "user")) {
>                                 F2FS_OPTION(sbi).compress_mode = COMPR_MODE_USER;
>                         } else {
>                                 kfree(name);
>                                 return -EINVAL;
>                         }
>                         kfree(name);
>                         break;
> 
> so if f2fs_sb_has_compression() is not true, then the option is ignored without
> any validation.
> 
> in other words, "mount -o compress_mode=nope ..." will succeed if the feature
> is disabled on the filesystem.
> 
> If I move the f2fs_sb_has_compression() check to later for the new mount API,
> then "mount -o compress_mode=nope ..."  will start failing for all images. Is
> this acceptable? It seems wise to reject invalid options rather than ignore them,
> even if they are incompatible with the format, but this would be a behavior
> change.

I'm fine w/ this change. IIRC, I haven't saw above use case, otherwise user
should stop passing invalid mount option to f2fs.

Thanks,

> 
> The above would be simple enough to defer (maybe set to COMPR_MODE_INVAL and
> reject it later) but I think other options such as compress/nocompress extensions
> would be very messy to approach as "accept all options given during parsing,
> and validate them later only if the corresponding feature is present."
> 
> So I wonder if a behavior change (stricter option validation) would be
> acceptable here?
> 
> Thanks,
> -Eric
> 



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 0/9] f2fs: first steps towards mount API conversion

[Eric Sandeen via Linux-f2fs-devel]

On 3/31/25 3:31 AM, Chao Yu wrote:
> On 3/29/25 12:18, Eric Sandeen wrote:
>> I was working on next steps for this, and I have a followup question.
>>
>> Today, several mount options are simply ignored if the on-disk format
>> does not support them. For example:
>>
>>                 case Opt_compress_mode:
>>                         if (!f2fs_sb_has_compression(sbi)) {
>>                                 f2fs_info(sbi, "Image doesn't support compression");
>>                                 break;
>>                         }
>>                         name = match_strdup(&args[0]);
>>                         if (!name)
>>                                 return -ENOMEM;
>>                         if (!strcmp(name, "fs")) {
>>                                 F2FS_OPTION(sbi).compress_mode = COMPR_MODE_FS;
>>                         } else if (!strcmp(name, "user")) {
>>                                 F2FS_OPTION(sbi).compress_mode = COMPR_MODE_USER;
>>                         } else {
>>                                 kfree(name);
>>                                 return -EINVAL;
>>                         }
>>                         kfree(name);
>>                         break;
>>
>> so if f2fs_sb_has_compression() is not true, then the option is ignored without
>> any validation.
>>
>> in other words, "mount -o compress_mode=nope ..." will succeed if the feature
>> is disabled on the filesystem.
>>
>> If I move the f2fs_sb_has_compression() check to later for the new mount API,
>> then "mount -o compress_mode=nope ..."  will start failing for all images. Is
>> this acceptable? It seems wise to reject invalid options rather than ignore them,
>> even if they are incompatible with the format, but this would be a behavior
>> change.
> 
> I'm fine w/ this change. IIRC, I haven't saw above use case, otherwise user
> should stop passing invalid mount option to f2fs.

Great, I will proceed with this. It will make the conversion simpler (but may
make testing/validation more difficult, as behavior will change with invalid input).

-Eric

> Thanks,



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 0/9] f2fs: first steps towards mount API conversion

[Eric Sandeen via Linux-f2fs-devel]

On 4/1/25 3:33 PM, Eric Sandeen wrote:
> On 3/31/25 3:31 AM, Chao Yu wrote:
>> On 3/29/25 12:18, Eric Sandeen wrote:
>>> I was working on next steps for this, and I have a followup question.
>>>
>>> Today, several mount options are simply ignored if the on-disk format
>>> does not support them. For example:
>>>
>>>                 case Opt_compress_mode:
>>>                         if (!f2fs_sb_has_compression(sbi)) {
>>>                                 f2fs_info(sbi, "Image doesn't support compression");
>>>                                 break;
>>>                         }
>>>                         name = match_strdup(&args[0]);
>>>                         if (!name)
>>>                                 return -ENOMEM;
>>>                         if (!strcmp(name, "fs")) {
>>>                                 F2FS_OPTION(sbi).compress_mode = COMPR_MODE_FS;
>>>                         } else if (!strcmp(name, "user")) {
>>>                                 F2FS_OPTION(sbi).compress_mode = COMPR_MODE_USER;
>>>                         } else {
>>>                                 kfree(name);
>>>                                 return -EINVAL;
>>>                         }
>>>                         kfree(name);
>>>                         break;
>>>
>>> so if f2fs_sb_has_compression() is not true, then the option is ignored without
>>> any validation.
>>>
>>> in other words, "mount -o compress_mode=nope ..." will succeed if the feature
>>> is disabled on the filesystem.
>>>
>>> If I move the f2fs_sb_has_compression() check to later for the new mount API,
>>> then "mount -o compress_mode=nope ..."  will start failing for all images. Is
>>> this acceptable? It seems wise to reject invalid options rather than ignore them,
>>> even if they are incompatible with the format, but this would be a behavior
>>> change.
>>
>> I'm fine w/ this change. IIRC, I haven't saw above use case, otherwise user
>> should stop passing invalid mount option to f2fs.
> 
> Great, I will proceed with this. It will make the conversion simpler (but may
> make testing/validation more difficult, as behavior will change with invalid input).

FYI - I don't think I will be able to complete this conversion task myself - f2fs is
by far the most difficult conversion I've encountered, and my time for these sorts of
projects is sadly limited. I do have one more patch series that moves a lot of the
on-disk feature checking out of option parsing, and perhaps I will send it as an example
at least.

But I think it may be time to ask the f2fs experts to take over this effort, because
I'm just not getting through it on my own.

(We are down to only a small handful of filesystems left - in fact, I think only
bfs, 9p, and f2fs, that don't have patches anywhere. So it would be really great to
get some help on this.)

Thanks,
-Eric
 
> -Eric
> 
>> Thanks,



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 0/9] f2fs: first steps towards mount API conversion

[Eric Sandeen via Linux-f2fs-devel]

On 4/12/25 12:17 PM, Eric Sandeen wrote:

...

> FYI - I don't think I will be able to complete this conversion task myself - f2fs is
> by far the most difficult conversion I've encountered, and my time for these sorts of
> projects is sadly limited. I do have one more patch series that moves a lot of the
> on-disk feature checking out of option parsing, and perhaps I will send it as an example
> at least.
> 
> But I think it may be time to ask the f2fs experts to take over this effort, because
> I'm just not getting through it on my own.
> 
> (We are down to only a small handful of filesystems left - in fact, I think only
> bfs, 9p, and f2fs, that don't have patches anywhere. So it would be really great to
> get some help on this.)

Sorry for replying to myself.

Actually, I will take one more try at this - when I first looked at Hongo Li's patches
to do the conversion, it seemed like there were a few problems, and I didn't review
fully. I realize now that although it was a different approach than I had been taking,
perhaps it was on the right track after all.

I'll try to evaluate that patch series more carefully and see if it solved the problems
I am struggling with now.

If it does, I'd be happy to work with you, Hongbo, to get this completed. Your series
will take a bit of forward-porting, as the codebase has moved forward quite a lot
since then. But I'll go back to an older kernel and evaluate the net result of your
changes, since I have some custom mount-option-testing framework here, and see how
things look.

Thanks,
-Eric



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel