On 2018/2/28 20:09, Junling Zheng wrote:
> Commit "0a007b97aad6"(f2fs: recover directory operations by fsync)
> fixed xfstest generic/342 case, but it also increased the written
> data and caused the performance degradation. In most cases, there's
> no need to do so heavy fsync actually.
>
> So we introduce new mount option "fsync_mode={posix,strict}" to
> control the policy of fsync. "fsync_mode=posix" is set by default,
> and means that f2fs uses a light fsync, which follows POSIX semantics.
> And "fsync_mode=strict" means that it's a heavy fsync, which behaves
> in line with xfs, ext4 and btrfs, where generic/342 will pass, but
> the performance will regress.
Let's handle to recover old fsync_mode in error path of ->remount_fs.
Thanks,
>
> Signed-off-by: Junling Zheng <[email protected]>
> ---
> Changes from v3:
> - add fsync_mode in sbi to record fsync mode
> - keep the extensibility for fsync_mode
> Changes from v2:
> - Change to "fsync={posix,strict}" format
> - Set "fsync=posix" default
> Changes from v1:
> - Add document modify
> - Add reviewer
> Documentation/filesystems/f2fs.txt | 7 +++++++
> fs/f2fs/dir.c | 3 ++-
> fs/f2fs/f2fs.h | 8 ++++++++
> fs/f2fs/file.c | 3 ++-
> fs/f2fs/namei.c | 9 ++++++---
> fs/f2fs/super.c | 24 ++++++++++++++++++++++++
> 6 files changed, 49 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/filesystems/f2fs.txt
> b/Documentation/filesystems/f2fs.txt
> index 0409c47584ea..514e44983a8e 100644
> --- a/Documentation/filesystems/f2fs.txt
> +++ b/Documentation/filesystems/f2fs.txt
> @@ -182,6 +182,13 @@ whint_mode=%s Control which write hints are
> passed down to block
> passes down hints with its policy.
> alloc_mode=%s Adjust block allocation policy, which supports "reuse"
> and "default".
> +fsync_mode=%s Control the policy of fsync. Currently supports
> "posix"
> + and "strict". In "posix" mode, which is default, fsync
> + will follow POSIX semantics and does a light operation
> + to improve the filesystem performance. In "strict"
> mode,
> + fsync will be heavy and behaves in line with xfs, ext4
> + and btrfs, where xfstest generic/342 will pass, but
> the
> + performance will regress.
>
>
> ================================================================================
> DEBUGFS ENTRIES
> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> index f00b5ed8c011..cce8ce11c2cc 100644
> --- a/fs/f2fs/dir.c
> +++ b/fs/f2fs/dir.c
> @@ -713,7 +713,8 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry,
> struct page *page,
>
> f2fs_update_time(F2FS_I_SB(dir), REQ_TIME);
>
> - add_ino_entry(F2FS_I_SB(dir), dir->i_ino, TRANS_DIR_INO);
> + if (F2FS_I_SB(dir)->fsync_mode == FSYNC_MODE_STRICT)
> + add_ino_entry(F2FS_I_SB(dir), dir->i_ino, TRANS_DIR_INO);
>
> if (f2fs_has_inline_dentry(dir))
> return f2fs_delete_inline_entry(dentry, page, dir, inode);
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index e4a782b23e66..b85dcfb30bac 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1052,6 +1052,11 @@ enum {
> ALLOC_MODE_REUSE, /* reuse segments as much as possible */
> };
>
> +enum fsync_mode {
> + FSYNC_MODE_POSIX, /* fsync follows posix semantics */
> + FSYNC_MODE_STRICT, /* fsync behaves in line with ext4 */
> +};
> +
> struct f2fs_sb_info {
> struct super_block *sb; /* pointer to VFS super block */
> struct proc_dir_entry *s_proc; /* proc entry */
> @@ -1241,6 +1246,9 @@ struct f2fs_sb_info {
>
> /* segment allocation policy */
> int alloc_mode;
> +
> + /* fsync policy */
> + int fsync_mode;
> };
>
> #ifdef CONFIG_F2FS_FAULT_INJECTION
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 6a202e5a6386..99fa207fc310 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -165,7 +165,8 @@ static inline enum cp_reason_type
> need_do_checkpoint(struct inode *inode)
> cp_reason = CP_FASTBOOT_MODE;
> else if (sbi->active_logs == 2)
> cp_reason = CP_SPEC_LOG_NUM;
> - else if (need_dentry_mark(sbi, inode->i_ino) &&
> + else if (sbi->fsync_mode == FSYNC_MODE_STRICT &&
> + need_dentry_mark(sbi, inode->i_ino) &&
> exist_written_data(sbi, F2FS_I(inode)->i_pino, TRANS_DIR_INO))
> cp_reason = CP_RECOVER_DIR;
>
> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> index 75135191a73d..010fc9a4a589 100644
> --- a/fs/f2fs/namei.c
> +++ b/fs/f2fs/namei.c
> @@ -977,7 +977,8 @@ static int f2fs_rename(struct inode *old_dir, struct
> dentry *old_dentry,
> }
> f2fs_i_links_write(old_dir, false);
> }
> - add_ino_entry(sbi, new_dir->i_ino, TRANS_DIR_INO);
> + if (sbi->fsync_mode == FSYNC_MODE_STRICT)
> + add_ino_entry(sbi, new_dir->i_ino, TRANS_DIR_INO);
>
> f2fs_unlock_op(sbi);
>
> @@ -1132,8 +1133,10 @@ static int f2fs_cross_rename(struct inode *old_dir,
> struct dentry *old_dentry,
> }
> f2fs_mark_inode_dirty_sync(new_dir, false);
>
> - add_ino_entry(sbi, old_dir->i_ino, TRANS_DIR_INO);
> - add_ino_entry(sbi, new_dir->i_ino, TRANS_DIR_INO);
> + if (sbi->fsync_mode == FSYNC_MODE_STRICT) {
> + add_ino_entry(sbi, old_dir->i_ino, TRANS_DIR_INO);
> + add_ino_entry(sbi, new_dir->i_ino, TRANS_DIR_INO);
> + }
>
> f2fs_unlock_op(sbi);
>
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 359dc031fb8c..67c07de87dad 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -131,6 +131,7 @@ enum {
> Opt_jqfmt_vfsv1,
> Opt_whint,
> Opt_alloc,
> + Opt_fsync,
> Opt_err,
> };
>
> @@ -186,6 +187,7 @@ static match_table_t f2fs_tokens = {
> {Opt_jqfmt_vfsv1, "jqfmt=vfsv1"},
> {Opt_whint, "whint_mode=%s"},
> {Opt_alloc, "alloc_mode=%s"},
> + {Opt_fsync, "fsync_mode=%s"},
> {Opt_err, NULL},
> };
>
> @@ -719,6 +721,22 @@ static int parse_options(struct super_block *sb, char
> *options)
> }
> kfree(name);
> break;
> + case Opt_fsync:
> + name = match_strdup(&args[0]);
> + if (!name)
> + return -ENOMEM;
> + if (strlen(name) == 5 &&
> + !strncmp(name, "posix", 5)) {
> + sbi->fsync_mode = FSYNC_MODE_POSIX;
> + } else if (strlen(name) == 6 &&
> + !strncmp(name, "strict", 6)) {
> + sbi->fsync_mode = FSYNC_MODE_STRICT;
> + } else {
> + kfree(name);
> + return -EINVAL;
> + }
> + kfree(name);
> + break;
> default:
> f2fs_msg(sb, KERN_ERR,
> "Unrecognized mount option \"%s\" or missing
> value",
> @@ -1287,6 +1305,11 @@ static int f2fs_show_options(struct seq_file *seq,
> struct dentry *root)
> seq_printf(seq, ",alloc_mode=%s", "default");
> else if (sbi->alloc_mode == ALLOC_MODE_REUSE)
> seq_printf(seq, ",alloc_mode=%s", "reuse");
> +
> + if (sbi->fsync_mode == FSYNC_MODE_POSIX)
> + seq_printf(seq, ",fsync_mode=%s", "posix");
> + else if (sbi->fsync_mode == FSYNC_MODE_STRICT)
> + seq_printf(seq, ",fsync_mode=%s", "strict");
> return 0;
> }
>
> @@ -1297,6 +1320,7 @@ static void default_options(struct f2fs_sb_info *sbi)
> sbi->inline_xattr_size = DEFAULT_INLINE_XATTR_ADDRS;
> sbi->whint_mode = WHINT_MODE_OFF;
> sbi->alloc_mode = ALLOC_MODE_DEFAULT;
> + sbi->fsync_mode = FSYNC_MODE_POSIX;
> sbi->readdir_ra = 1;
>
> set_opt(sbi, BG_GC);
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
