f2fs: fix to recover quota data correctly

With -O quota mkfs option, xfstests generic/417 fails due to fsck detects
data corruption on quota inodes.

[ASSERT] (fsck_chk_quota_files:2051)  --> Quota file is missing or invalid quota file content found.

The root cause is there is a hole f2fs doesn't hold quota inodes,
so all recovered quota data will be dropped due to SBI_POR_DOING
flag was set.
- f2fs_fill_super
 - f2fs_recover_orphan_inodes
  - f2fs_enable_quota_files
  - f2fs_quota_off_umount
<--- quota inodes were dropped --->
 - f2fs_recover_fsync_data
  - f2fs_enable_quota_files
  - f2fs_quota_off_umount

This patch tries to eliminate the hole by holding quota inodes
during entire recovery flow as below:
- f2fs_fill_super
 - f2fs_recover_quota_begin
 - f2fs_recover_orphan_inodes
 - f2fs_recover_fsync_data
 - f2fs_recover_quota_end

Then, recovered quota data can be persisted after SBI_POR_DOING
is cleared.

Signed-off-by: Chao Yu <chao@kernel.org>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index ec7a0603549070f86b12c17f0644c1b090962a81..19e2cfe3b5bee44471ce1f252dbead2c11f0ecdb 100644 (file)
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -713,11 +713,7 @@ err_out:
 int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi)
 {
        block_t start_blk, orphan_blocks, i, j;
-       unsigned int s_flags = sbi->sb->s_flags;
        int err = 0;
-#ifdef CONFIG_QUOTA
-       int quota_enabled;
-#endif
 
        if (!is_set_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG))
                return 0;
@@ -727,18 +723,8 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi)
                return 0;
        }
 
-       if (s_flags & SB_RDONLY) {
+       if (is_sbi_flag_set(sbi, SBI_IS_WRITABLE))
                f2fs_info(sbi, "orphan cleanup on readonly fs");
-               sbi->sb->s_flags &= ~SB_RDONLY;
-       }
-
-#ifdef CONFIG_QUOTA
-       /*
-        * Turn on quotas which were not enabled for read-only mounts if
-        * filesystem has quota feature, so that they are updated correctly.
-        */
-       quota_enabled = f2fs_enable_quota_files(sbi, s_flags & SB_RDONLY);
-#endif
 
        start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi);
        orphan_blocks = __start_sum_addr(sbi) - 1 - __cp_payload(sbi);
@@ -772,13 +758,6 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi)
 out:
        set_sbi_flag(sbi, SBI_IS_RECOVERED);
 
-#ifdef CONFIG_QUOTA
-       /* Turn quotas off */
-       if (quota_enabled)
-               f2fs_quota_off_umount(sbi->sb);
-#endif
-       sbi->sb->s_flags = s_flags; /* Restore SB_RDONLY status */
-
        return err;
 }
 

diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
index 99c7fc832ec798473ce7c273f4cebd516f545fa0..61c35b59126ecde97f4b020a0ac10a664f3f213e 100644 (file)
--- a/fs/f2fs/debug.c
+++ b/fs/f2fs/debug.c
@@ -352,6 +352,7 @@ static const char *s_flag[MAX_SBI_FLAG] = {
        [SBI_QUOTA_NEED_REPAIR] = "quota_need_repair",
        [SBI_IS_RESIZEFS]       = "resizefs",
        [SBI_IS_FREEZING]       = "freezefs",
+       [SBI_IS_WRITABLE]       = "writable",
 };
 
 static const char *ipu_mode_names[F2FS_IPU_MAX] = {

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 30e31bb5c2cea9982a47a44001e681e8b9174e4a..ede38bcef80e9fa10b81d0132bb0126de3744c6f 100644 (file)
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1294,6 +1294,7 @@ enum {
        SBI_QUOTA_NEED_REPAIR,                  /* quota file may be corrupted */
        SBI_IS_RESIZEFS,                        /* resizefs is in process */
        SBI_IS_FREEZING,                        /* freezefs is in process */
+       SBI_IS_WRITABLE,                        /* remove ro mountoption transiently */
        MAX_SBI_FLAG,
 };
 

diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index dfd41908b12d17fd2fee1c1c75714a3bbc7adfda..58c1a0096f7dedf0bbff0d36243c4aa08f324449 100644 (file)
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -825,19 +825,9 @@ int f2fs_recover_fsync_data(struct f2fs_sb_info *sbi, bool check_only)
        unsigned long s_flags = sbi->sb->s_flags;
        bool need_writecp = false;
        bool fix_curseg_write_pointer = false;
-#ifdef CONFIG_QUOTA
-       int quota_enabled;
-#endif
 
-       if (s_flags & SB_RDONLY) {
+       if (is_sbi_flag_set(sbi, SBI_IS_WRITABLE))
                f2fs_info(sbi, "recover fsync data on readonly fs");
-               sbi->sb->s_flags &= ~SB_RDONLY;
-       }
-
-#ifdef CONFIG_QUOTA
-       /* Turn on quotas so that they are updated correctly */
-       quota_enabled = f2fs_enable_quota_files(sbi, s_flags & SB_RDONLY);
-#endif
 
        INIT_LIST_HEAD(&inode_list);
        INIT_LIST_HEAD(&tmp_inode_list);
@@ -909,11 +899,6 @@ skip:
                }
        }
 
-#ifdef CONFIG_QUOTA
-       /* Turn quotas off */
-       if (quota_enabled)
-               f2fs_quota_off_umount(sbi->sb);
-#endif
        sbi->sb->s_flags = s_flags; /* Restore SB_RDONLY status */
 
        return ret ? ret : err;

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 8f9086d53d520ddbc8521c99b2b6dea89f6df658..4e53b1100b84a6edd7aedd2caa1c73bacfb9c391 100644 (file)
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -2501,6 +2501,54 @@ restore_opts:
 }
 
 #ifdef CONFIG_QUOTA
+static bool f2fs_need_recovery(struct f2fs_sb_info *sbi)
+{
+       /* need to recovery orphan */
+       if (is_set_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG))
+               return true;
+       /* need to recovery data */
+       if (test_opt(sbi, DISABLE_ROLL_FORWARD))
+               return false;
+       if (test_opt(sbi, NORECOVERY))
+               return false;
+       return !is_set_ckpt_flags(sbi, CP_UMOUNT_FLAG);
+}
+
+static bool f2fs_recover_quota_begin(struct f2fs_sb_info *sbi)
+{
+       bool readonly = f2fs_readonly(sbi->sb);
+
+       if (!f2fs_need_recovery(sbi))
+               return false;
+
+       /* it doesn't need to check f2fs_sb_has_readonly() */
+       if (f2fs_hw_is_readonly(sbi))
+               return false;
+
+       if (readonly) {
+               sbi->sb->s_flags &= ~SB_RDONLY;
+               set_sbi_flag(sbi, SBI_IS_WRITABLE);
+       }
+
+       /*
+        * Turn on quotas which were not enabled for read-only mounts if
+        * filesystem has quota feature, so that they are updated correctly.
+        */
+       return f2fs_enable_quota_files(sbi, readonly);
+}
+
+static void f2fs_recover_quota_end(struct f2fs_sb_info *sbi,
+                                               bool quota_enabled)
+{
+       if (quota_enabled)
+               f2fs_quota_off_umount(sbi->sb);
+
+       if (is_sbi_flag_set(sbi, SBI_IS_WRITABLE)) {
+               clear_sbi_flag(sbi, SBI_IS_WRITABLE);
+               sbi->sb->s_flags |= SB_RDONLY;
+       }
+}
+
 /* Read data from quotafile */
 static ssize_t f2fs_quota_read(struct super_block *sb, int type, char *data,
                               size_t len, loff_t off)
@@ -4116,6 +4164,9 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
        int recovery, i, valid_super_block;
        struct curseg_info *seg_i;
        int retry_cnt = 1;
+#ifdef CONFIG_QUOTA
+       bool quota_enabled = false;
+#endif
 
 try_onemore:
        err = -EINVAL;
@@ -4409,6 +4460,8 @@ try_onemore:
                if (err)
                        f2fs_err(sbi, "Cannot turn on quotas: error %d", err);
        }
+
+       quota_enabled = f2fs_recover_quota_begin(sbi);
 #endif
        /* if there are any orphan inodes, free them */
        err = f2fs_recover_orphan_inodes(sbi);
@@ -4466,6 +4519,10 @@ try_onemore:
                }
        }
 
+#ifdef CONFIG_QUOTA
+       f2fs_recover_quota_end(sbi, quota_enabled);
+#endif
+
        /*
         * If the f2fs is not readonly and fsync data recovery succeeds,
         * check zoned block devices' write pointer consistency.