On 05/07, Eric Sandeen wrote:
> On 5/7/25 3:28 PM, Jaegeuk Kim wrote:
> >> But as far as I can tell, at least for the extent cache, remount is handled
> >> properly already (with the hunk above):
> >>
> >> # mkfs/mkfs.f2fs -c /dev/v...@vdc.file /dev/vdb
> >> # mount /dev/vdb mnt
> >> # mount -o remount,noextent_cache mnt
> >> mount: /root/mnt: mount point not mounted or bad option.
> >>        dmesg(1) may have more information after failed mount system call.
> >> # dmesg | tail -n 1
> >> [60012.364941] F2FS-fs (vdb): device aliasing requires extent cache
> >> #
> >>
> >> I haven't tested with i.e. blkzoned devices though, is there a testcase
> >> that fails for you?
> > I'm worrying about any missing case to check options enabled by 
> > default_options.
> > For example, in the case of device_aliasing, we rely on enabling 
> > extent_cache
> > by default_options, which was not caught by f2fs_check_opt_consistency.
> > 
> > I was thinking that we'd need a post sanity check.
> 
> I see. If you want a "belt and suspenders" approach and it works for
> you, no argument from me :)

Thanks. :)

I just found that I had to check it's from remount or not. And, this change does
not break my setup having a specific options. Let me queue the series back and
wait for further review from Chao.

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

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index d89b9ede221e..0ee783224953 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,33 @@ 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, bool remount)
+{
+       if (f2fs_sb_has_device_alias(sbi) &&
+           !test_opt(sbi, READ_EXTENT_CACHE)) {
+               f2fs_err(sbi, "device aliasing requires extent cache");
+               return -EINVAL;
+       }
+
+       if (!remount)
+               return 0;
+
+#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 +2644,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, true);
+       if (err)
                goto restore_opts;
-       }
-#endif
+
        /* flush outstanding errors before changing fs state */
        flush_work(&sbi->s_error_work);
 
@@ -2663,12 +2685,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 +4824,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, false);
+       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

Reply via email to