Re: [f2fs-dev] [PATCH V3 0/7] f2fs: new mount API conversion

[Jaegeuk Kim via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>]

On 05/07, Eric Sandeen wrote:
> On 5/7/25 9:46 AM, Jaegeuk Kim wrote:
> 
> > I meant:
> > 
> > # mkfs/mkfs.f2fs -c /dev/vdc@vdc.file /dev/vdb
> > # mount /dev/vdb mnt
> > 
> > It's supposed to be successful, since extent_cache is enabled by default.
> 
> I'm sorry, clearly I was too sleepy last night. This fixes it for me.
> 
> We have to test the mask to see if the option was explisitly set (either
> extent_cache or noextent_cache) at mount time.
> 
> If it was not specified at all, it will be set by the default f'n and
> remain in the sbi, and it will pass this consistency check.
> 
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index d89b9ede221e..e178796ce9a7 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -1412,7 +1414,8 @@ static int f2fs_check_opt_consistency(struct fs_context *fc,
>  	}
>  
>  	if (f2fs_sb_has_device_alias(sbi) &&
> -			!ctx_test_opt(ctx, F2FS_MOUNT_READ_EXTENT_CACHE)) {
> +			(ctx->opt_mask & F2FS_MOUNT_READ_EXTENT_CACHE) &&
> +			 !ctx_test_opt(ctx, F2FS_MOUNT_READ_EXTENT_CACHE)) {
>  		f2fs_err(sbi, "device aliasing requires extent cache");
>  		return -EINVAL;
>  	}

I think that will cover the user-given options only, but we'd better check the
final options as well. Can we apply like this?

---
 fs/f2fs/super.c | 50 ++++++++++++++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 17 deletions(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index d89b9ede221e..270a9bf9773d 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1412,6 +1412,7 @@ static int f2fs_check_opt_consistency(struct fs_context *fc,
 	}
 
 	if (f2fs_sb_has_device_alias(sbi) &&
+			(ctx->opt_mask & F2FS_MOUNT_READ_EXTENT_CACHE) &&
 			!ctx_test_opt(ctx, F2FS_MOUNT_READ_EXTENT_CACHE)) {
 		f2fs_err(sbi, "device aliasing requires extent cache");
 		return -EINVAL;
@@ -1657,6 +1658,29 @@ static void f2fs_apply_options(struct fs_context *fc, struct super_block *sb)
 	f2fs_apply_quota_options(fc, sb);
 }
 
+static int f2fs_sanity_check_options(struct f2fs_sb_info *sbi)
+{
+	if (f2fs_sb_has_device_alias(sbi) &&
+	    !test_opt(sbi, READ_EXTENT_CACHE)) {
+		f2fs_err(sbi, "device aliasing requires extent cache");
+		return -EINVAL;
+	}
+#ifdef CONFIG_BLK_DEV_ZONED
+	if (f2fs_sb_has_blkzoned(sbi) &&
+	    sbi->max_open_zones < F2FS_OPTION(sbi).active_logs) {
+		f2fs_err(sbi,
+			"zoned: max open zones %u is too small, need at least %u open zones",
+				 sbi->max_open_zones, F2FS_OPTION(sbi).active_logs);
+		return -EINVAL;
+	}
+#endif
+	if (f2fs_lfs_mode(sbi) && !IS_F2FS_IPU_DISABLE(sbi)) {
+		f2fs_warn(sbi, "LFS is not compatible with IPU");
+		return -EINVAL;
+	}
+	return 0;
+}
+
 static struct inode *f2fs_alloc_inode(struct super_block *sb)
 {
 	struct f2fs_inode_info *fi;
@@ -2616,21 +2640,15 @@ static int __f2fs_remount(struct fs_context *fc, struct super_block *sb)
 	default_options(sbi, true);
 
 	err = f2fs_check_opt_consistency(fc, sb);
-	if (err < 0)
+	if (err)
 		goto restore_opts;
 
 	f2fs_apply_options(fc, sb);
 
-#ifdef CONFIG_BLK_DEV_ZONED
-	if (f2fs_sb_has_blkzoned(sbi) &&
-		sbi->max_open_zones < F2FS_OPTION(sbi).active_logs) {
-		f2fs_err(sbi,
-			"zoned: max open zones %u is too small, need at least %u open zones",
-				 sbi->max_open_zones, F2FS_OPTION(sbi).active_logs);
-		err = -EINVAL;
+	err = f2fs_sanity_check_options(sbi);
+	if (err)
 		goto restore_opts;
-	}
-#endif
+
 	/* flush outstanding errors before changing fs state */
 	flush_work(&sbi->s_error_work);
 
@@ -2663,12 +2681,6 @@ static int __f2fs_remount(struct fs_context *fc, struct super_block *sb)
 		}
 	}
 #endif
-	if (f2fs_lfs_mode(sbi) && !IS_F2FS_IPU_DISABLE(sbi)) {
-		err = -EINVAL;
-		f2fs_warn(sbi, "LFS is not compatible with IPU");
-		goto restore_opts;
-	}
-
 	/* disallow enable atgc dynamically */
 	if (no_atgc == !!test_opt(sbi, ATGC)) {
 		err = -EINVAL;
@@ -4808,11 +4820,15 @@ static int f2fs_fill_super(struct super_block *sb, struct fs_context *fc)
 	default_options(sbi, false);
 
 	err = f2fs_check_opt_consistency(fc, sb);
-	if (err < 0)
+	if (err)
 		goto free_sb_buf;
 
 	f2fs_apply_options(fc, sb);
 
+	err = f2fs_sanity_check_options(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.49.0.1015.ga840276032-goog



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