[PATCH v2] f2fs: introduce "strict_fsync" for posix standard fsync

[PATCH v2] f2fs: introduce "strict_fsync" for posix standard fsync

[Junling Zheng]

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 heavily fsync actually.

So we introduce a new mount option "strict_fsync" to control the
policy of fsync. It's set by default, and means that fsync follows
POSIX semantics. And "nostrict_fsync" means that the behaviour is
in line with xfs, ext4 and btrfs, where generic/342 will pass.

Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
Reviewed-by: Chao Yu <yuchao0@huawei.com>
---
 Documentation/filesystems/f2fs.txt |  4 ++++
 fs/f2fs/dir.c                      |  3 ++-
 fs/f2fs/f2fs.h                     |  1 +
 fs/f2fs/file.c                     |  3 ++-
 fs/f2fs/namei.c                    |  9 ++++++---
 fs/f2fs/super.c                    | 13 +++++++++++++
 6 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/Documentation/filesystems/f2fs.txt b/Documentation/filesystems/f2fs.txt
index 0caf7da0a532..c484ce8d1f4c 100644
--- a/Documentation/filesystems/f2fs.txt
+++ b/Documentation/filesystems/f2fs.txt
@@ -180,6 +180,10 @@ whint_mode=%s          Control which write hints are passed down to block
                        down hints. In "user-based" mode, f2fs tries to pass
                        down hints given by users. And in "fs-based" mode, f2fs
                        passes down hints with its policy.
+{,no}strict_fsync      Control the policy of fsync. Set "strict_fsync" by default,
+                       which means that fsync will follow POSIX semantics. Use
+                       "nostrict_fsync" if you expect fsync to behave in line with
+                       xfs, ext4 and btrfs, where xfstest generic/342 will pass.
 
 ================================================================================
 DEBUGFS ENTRIES
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index f00b5ed8c011..7487b7e77a36 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 (!test_opt(F2FS_I_SB(dir), STRICT_FSYNC))
+		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 dbe87c7a266e..8cf914d12f17 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -97,6 +97,7 @@ extern char *fault_name[FAULT_MAX];
 #define F2FS_MOUNT_QUOTA		0x00400000
 #define F2FS_MOUNT_INLINE_XATTR_SIZE	0x00800000
 #define F2FS_MOUNT_RESERVE_ROOT		0x01000000
+#define F2FS_MOUNT_STRICT_FSYNC		0x02000000
 
 #define clear_opt(sbi, option)	((sbi)->mount_opt.opt &= ~F2FS_MOUNT_##option)
 #define set_opt(sbi, option)	((sbi)->mount_opt.opt |= F2FS_MOUNT_##option)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 672a542e5464..9b39254f5b48 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 (!test_opt(sbi, STRICT_FSYNC) &&
+		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 c4c94c7e9f4f..ef86ae327f91 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -936,7 +936,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 (!test_opt(sbi, STRICT_FSYNC))
+		add_ino_entry(sbi, new_dir->i_ino, TRANS_DIR_INO);
 
 	f2fs_unlock_op(sbi);
 
@@ -1091,8 +1092,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 (!test_opt(sbi, STRICT_FSYNC)) {
+		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 7966cf7bfb8e..3066fc9d8985 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -130,6 +130,8 @@ enum {
 	Opt_jqfmt_vfsv0,
 	Opt_jqfmt_vfsv1,
 	Opt_whint,
+	Opt_strict_fsync,
+	Opt_nostrict_fsync,
 	Opt_err,
 };
 
@@ -184,6 +186,8 @@ static match_table_t f2fs_tokens = {
 	{Opt_jqfmt_vfsv0, "jqfmt=vfsv0"},
 	{Opt_jqfmt_vfsv1, "jqfmt=vfsv1"},
 	{Opt_whint, "whint_mode=%s"},
+	{Opt_strict_fsync, "strict_fsync"},
+	{Opt_nostrict_fsync, "nostrict_fsync"},
 	{Opt_err, NULL},
 };
 
@@ -700,6 +704,12 @@ static int parse_options(struct super_block *sb, char *options)
 			}
 			kfree(name);
 			break;
+		case Opt_strict_fsync:
+			set_opt(sbi, STRICT_FSYNC);
+			break;
+		case Opt_nostrict_fsync:
+			clear_opt(sbi, STRICT_FSYNC);
+			break;
 		default:
 			f2fs_msg(sb, KERN_ERR,
 				"Unrecognized mount option \"%s\" or missing value",
@@ -1296,6 +1306,9 @@ static void default_options(struct f2fs_sb_info *sbi)
 	set_opt(sbi, POSIX_ACL);
 #endif
 
+	/* POSIX standard fsync */
+	set_opt(sbi, STRICT_FSYNC);
+
 #ifdef CONFIG_F2FS_FAULT_INJECTION
 	f2fs_build_fault_attr(sbi, 0);
 #endif
-- 
2.15.1


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH v2] f2fs: introduce "strict_fsync" for posix standard fsync

[Jaegeuk Kim]

On 02/02, 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 heavily fsync actually.
> 
> So we introduce a new mount option "strict_fsync" to control the
> policy of fsync. It's set by default, and means that fsync follows
> POSIX semantics. And "nostrict_fsync" means that the behaviour is
> in line with xfs, ext4 and btrfs, where generic/342 will pass.

How about adding "fsync=%s" to give another chance for fsync policies?

Thanks,

> 
> Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
> Reviewed-by: Chao Yu <yuchao0@huawei.com>
> ---
>  Documentation/filesystems/f2fs.txt |  4 ++++
>  fs/f2fs/dir.c                      |  3 ++-
>  fs/f2fs/f2fs.h                     |  1 +
>  fs/f2fs/file.c                     |  3 ++-
>  fs/f2fs/namei.c                    |  9 ++++++---
>  fs/f2fs/super.c                    | 13 +++++++++++++
>  6 files changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/filesystems/f2fs.txt b/Documentation/filesystems/f2fs.txt
> index 0caf7da0a532..c484ce8d1f4c 100644
> --- a/Documentation/filesystems/f2fs.txt
> +++ b/Documentation/filesystems/f2fs.txt
> @@ -180,6 +180,10 @@ whint_mode=%s          Control which write hints are passed down to block
>                         down hints. In "user-based" mode, f2fs tries to pass
>                         down hints given by users. And in "fs-based" mode, f2fs
>                         passes down hints with its policy.
> +{,no}strict_fsync      Control the policy of fsync. Set "strict_fsync" by default,
> +                       which means that fsync will follow POSIX semantics. Use
> +                       "nostrict_fsync" if you expect fsync to behave in line with
> +                       xfs, ext4 and btrfs, where xfstest generic/342 will pass.
>  
>  ================================================================================
>  DEBUGFS ENTRIES
> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> index f00b5ed8c011..7487b7e77a36 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 (!test_opt(F2FS_I_SB(dir), STRICT_FSYNC))
> +		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 dbe87c7a266e..8cf914d12f17 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -97,6 +97,7 @@ extern char *fault_name[FAULT_MAX];
>  #define F2FS_MOUNT_QUOTA		0x00400000
>  #define F2FS_MOUNT_INLINE_XATTR_SIZE	0x00800000
>  #define F2FS_MOUNT_RESERVE_ROOT		0x01000000
> +#define F2FS_MOUNT_STRICT_FSYNC		0x02000000
>  
>  #define clear_opt(sbi, option)	((sbi)->mount_opt.opt &= ~F2FS_MOUNT_##option)
>  #define set_opt(sbi, option)	((sbi)->mount_opt.opt |= F2FS_MOUNT_##option)
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 672a542e5464..9b39254f5b48 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 (!test_opt(sbi, STRICT_FSYNC) &&
> +		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 c4c94c7e9f4f..ef86ae327f91 100644
> --- a/fs/f2fs/namei.c
> +++ b/fs/f2fs/namei.c
> @@ -936,7 +936,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 (!test_opt(sbi, STRICT_FSYNC))
> +		add_ino_entry(sbi, new_dir->i_ino, TRANS_DIR_INO);
>  
>  	f2fs_unlock_op(sbi);
>  
> @@ -1091,8 +1092,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 (!test_opt(sbi, STRICT_FSYNC)) {
> +		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 7966cf7bfb8e..3066fc9d8985 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -130,6 +130,8 @@ enum {
>  	Opt_jqfmt_vfsv0,
>  	Opt_jqfmt_vfsv1,
>  	Opt_whint,
> +	Opt_strict_fsync,
> +	Opt_nostrict_fsync,
>  	Opt_err,
>  };
>  
> @@ -184,6 +186,8 @@ static match_table_t f2fs_tokens = {
>  	{Opt_jqfmt_vfsv0, "jqfmt=vfsv0"},
>  	{Opt_jqfmt_vfsv1, "jqfmt=vfsv1"},
>  	{Opt_whint, "whint_mode=%s"},
> +	{Opt_strict_fsync, "strict_fsync"},
> +	{Opt_nostrict_fsync, "nostrict_fsync"},
>  	{Opt_err, NULL},
>  };
>  
> @@ -700,6 +704,12 @@ static int parse_options(struct super_block *sb, char *options)
>  			}
>  			kfree(name);
>  			break;
> +		case Opt_strict_fsync:
> +			set_opt(sbi, STRICT_FSYNC);
> +			break;
> +		case Opt_nostrict_fsync:
> +			clear_opt(sbi, STRICT_FSYNC);
> +			break;
>  		default:
>  			f2fs_msg(sb, KERN_ERR,
>  				"Unrecognized mount option \"%s\" or missing value",
> @@ -1296,6 +1306,9 @@ static void default_options(struct f2fs_sb_info *sbi)
>  	set_opt(sbi, POSIX_ACL);
>  #endif
>  
> +	/* POSIX standard fsync */
> +	set_opt(sbi, STRICT_FSYNC);
> +
>  #ifdef CONFIG_F2FS_FAULT_INJECTION
>  	f2fs_build_fault_attr(sbi, 0);
>  #endif
> -- 
> 2.15.1

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH v2] f2fs: introduce "strict_fsync" for posix standard fsync

[Junling Zheng]

Hi, Jaegeuk

On 2018/2/10 8:44, Jaegeuk Kim wrote:
> On 02/02, 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 heavily fsync actually.
>>
>> So we introduce a new mount option "strict_fsync" to control the
>> policy of fsync. It's set by default, and means that fsync follows
>> POSIX semantics. And "nostrict_fsync" means that the behaviour is
>> in line with xfs, ext4 and btrfs, where generic/342 will pass.
> 
> How about adding "fsync=%s" to give another chance for fsync policies?
> 

OK, I'll give patch v3 to change to "fsync=%s" format.
BTW, which policy do u think should be the default behavior for f2fs? Posix
or ext4?

Thanks
Junling

> Thanks,
> 
>>
>> Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
>> Reviewed-by: Chao Yu <yuchao0@huawei.com>
>> ---
>>  Documentation/filesystems/f2fs.txt |  4 ++++
>>  fs/f2fs/dir.c                      |  3 ++-
>>  fs/f2fs/f2fs.h                     |  1 +
>>  fs/f2fs/file.c                     |  3 ++-
>>  fs/f2fs/namei.c                    |  9 ++++++---
>>  fs/f2fs/super.c                    | 13 +++++++++++++
>>  6 files changed, 28 insertions(+), 5 deletions(-)
>>


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH v2] f2fs: introduce "strict_fsync" for posix standard fsync

[Jaegeuk Kim]

On 02/11, Junling Zheng wrote:
> Hi, Jaegeuk
> 
> On 2018/2/10 8:44, Jaegeuk Kim wrote:
> > On 02/02, 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 heavily fsync actually.
> >>
> >> So we introduce a new mount option "strict_fsync" to control the
> >> policy of fsync. It's set by default, and means that fsync follows
> >> POSIX semantics. And "nostrict_fsync" means that the behaviour is
> >> in line with xfs, ext4 and btrfs, where generic/342 will pass.
> > 
> > How about adding "fsync=%s" to give another chance for fsync policies?
> > 
> 
> OK, I'll give patch v3 to change to "fsync=%s" format.
> BTW, which policy do u think should be the default behavior for f2fs? Posix
> or ext4?

The default should be like ext4 as fsync=strict. We may add fsync=posix for
this.

Thanks,

> 
> Thanks
> Junling
> 
> > Thanks,
> > 
> >>
> >> Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
> >> Reviewed-by: Chao Yu <yuchao0@huawei.com>
> >> ---
> >>  Documentation/filesystems/f2fs.txt |  4 ++++
> >>  fs/f2fs/dir.c                      |  3 ++-
> >>  fs/f2fs/f2fs.h                     |  1 +
> >>  fs/f2fs/file.c                     |  3 ++-
> >>  fs/f2fs/namei.c                    |  9 ++++++---
> >>  fs/f2fs/super.c                    | 13 +++++++++++++
> >>  6 files changed, 28 insertions(+), 5 deletions(-)
> >>

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH v2] f2fs: introduce "strict_fsync" for posix standard fsync

[Chao Yu]

Hi Jaegeuk,

On 2018/2/12 8:07, Jaegeuk Kim wrote:
> On 02/11, Junling Zheng wrote:
>> Hi, Jaegeuk
>>
>> On 2018/2/10 8:44, Jaegeuk Kim wrote:
>>> On 02/02, 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 heavily fsync actually.
>>>>
>>>> So we introduce a new mount option "strict_fsync" to control the
>>>> policy of fsync. It's set by default, and means that fsync follows
>>>> POSIX semantics. And "nostrict_fsync" means that the behaviour is
>>>> in line with xfs, ext4 and btrfs, where generic/342 will pass.
>>>
>>> How about adding "fsync=%s" to give another chance for fsync policies?

Agreed.

>>>
>>
>> OK, I'll give patch v3 to change to "fsync=%s" format.
>> BTW, which policy do u think should be the default behavior for f2fs? Posix
>> or ext4?
> 
> The default should be like ext4 as fsync=strict. We may add fsync=posix for
> this.

I'd like to suggest using fsync=posix option by default, because in most popular
in-used scenario of f2fs: android, all users of filesystem are posix-compliant,
so there is no such requirement that fs needs do more than posix as generic/342
restricted.

The performance of fsync=strict mode regressed as expected, so if we enable
fsync=strict mode by default, I suspect it may make f2fs losing some kinds of
benchmark competition.

How do you think?

Thanks,

> 
> Thanks,
> 
>>
>> Thanks
>> Junling
>>
>>> Thanks,
>>>
>>>>
>>>> Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
>>>> Reviewed-by: Chao Yu <yuchao0@huawei.com>
>>>> ---
>>>>  Documentation/filesystems/f2fs.txt |  4 ++++
>>>>  fs/f2fs/dir.c                      |  3 ++-
>>>>  fs/f2fs/f2fs.h                     |  1 +
>>>>  fs/f2fs/file.c                     |  3 ++-
>>>>  fs/f2fs/namei.c                    |  9 ++++++---
>>>>  fs/f2fs/super.c                    | 13 +++++++++++++
>>>>  6 files changed, 28 insertions(+), 5 deletions(-)
>>>>
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH v2] f2fs: introduce "strict_fsync" for posix standard fsync

[Jaegeuk Kim]

On 02/13, Chao Yu wrote:
> Hi Jaegeuk,
> 
> On 2018/2/12 8:07, Jaegeuk Kim wrote:
> > On 02/11, Junling Zheng wrote:
> >> Hi, Jaegeuk
> >>
> >> On 2018/2/10 8:44, Jaegeuk Kim wrote:
> >>> On 02/02, 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 heavily fsync actually.
> >>>>
> >>>> So we introduce a new mount option "strict_fsync" to control the
> >>>> policy of fsync. It's set by default, and means that fsync follows
> >>>> POSIX semantics. And "nostrict_fsync" means that the behaviour is
> >>>> in line with xfs, ext4 and btrfs, where generic/342 will pass.
> >>>
> >>> How about adding "fsync=%s" to give another chance for fsync policies?
> 
> Agreed.
> 
> >>>
> >>
> >> OK, I'll give patch v3 to change to "fsync=%s" format.
> >> BTW, which policy do u think should be the default behavior for f2fs? Posix
> >> or ext4?
> > 
> > The default should be like ext4 as fsync=strict. We may add fsync=posix for
> > this.
> 
> I'd like to suggest using fsync=posix option by default, because in most popular
> in-used scenario of f2fs: android, all users of filesystem are posix-compliant,
> so there is no such requirement that fs needs do more than posix as generic/342
> restricted.
> 
> The performance of fsync=strict mode regressed as expected, so if we enable
> fsync=strict mode by default, I suspect it may make f2fs losing some kinds of
> benchmark competition.
> 
> How do you think?

Okay, I have no objection on that.

Thanks,

> 
> Thanks,
> 
> > 
> > Thanks,
> > 
> >>
> >> Thanks
> >> Junling
> >>
> >>> Thanks,
> >>>
> >>>>
> >>>> Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
> >>>> Reviewed-by: Chao Yu <yuchao0@huawei.com>
> >>>> ---
> >>>>  Documentation/filesystems/f2fs.txt |  4 ++++
> >>>>  fs/f2fs/dir.c                      |  3 ++-
> >>>>  fs/f2fs/f2fs.h                     |  1 +
> >>>>  fs/f2fs/file.c                     |  3 ++-
> >>>>  fs/f2fs/namei.c                    |  9 ++++++---
> >>>>  fs/f2fs/super.c                    | 13 +++++++++++++
> >>>>  6 files changed, 28 insertions(+), 5 deletions(-)
> >>>>
> > 
> > .
> > 

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH v2] f2fs: introduce "strict_fsync" for posix standard fsync

[Chao Yu]

On 2018/2/28 12:49, Jaegeuk Kim wrote:
> On 02/13, Chao Yu wrote:
>> Hi Jaegeuk,
>>
>> On 2018/2/12 8:07, Jaegeuk Kim wrote:
>>> On 02/11, Junling Zheng wrote:
>>>> Hi, Jaegeuk
>>>>
>>>> On 2018/2/10 8:44, Jaegeuk Kim wrote:
>>>>> On 02/02, 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 heavily fsync actually.
>>>>>>
>>>>>> So we introduce a new mount option "strict_fsync" to control the
>>>>>> policy of fsync. It's set by default, and means that fsync follows
>>>>>> POSIX semantics. And "nostrict_fsync" means that the behaviour is
>>>>>> in line with xfs, ext4 and btrfs, where generic/342 will pass.
>>>>>
>>>>> How about adding "fsync=%s" to give another chance for fsync policies?
>>
>> Agreed.
>>
>>>>>
>>>>
>>>> OK, I'll give patch v3 to change to "fsync=%s" format.
>>>> BTW, which policy do u think should be the default behavior for f2fs? Posix
>>>> or ext4?
>>>
>>> The default should be like ext4 as fsync=strict. We may add fsync=posix for
>>> this.
>>
>> I'd like to suggest using fsync=posix option by default, because in most popular
>> in-used scenario of f2fs: android, all users of filesystem are posix-compliant,
>> so there is no such requirement that fs needs do more than posix as generic/342
>> restricted.
>>
>> The performance of fsync=strict mode regressed as expected, so if we enable
>> fsync=strict mode by default, I suspect it may make f2fs losing some kinds of
>> benchmark competition.
>>
>> How do you think?
> 
> Okay, I have no objection on that.

Well to hear that, so let's wait Junling to update the patch. ;)

Thanks,

> 
> Thanks,
> 
>>
>> Thanks,
>>
>>>
>>> Thanks,
>>>
>>>>
>>>> Thanks
>>>> Junling
>>>>
>>>>> Thanks,
>>>>>
>>>>>>
>>>>>> Signed-off-by: Junling Zheng <zhengjunling@huawei.com>
>>>>>> Reviewed-by: Chao Yu <yuchao0@huawei.com>
>>>>>> ---
>>>>>>  Documentation/filesystems/f2fs.txt |  4 ++++
>>>>>>  fs/f2fs/dir.c                      |  3 ++-
>>>>>>  fs/f2fs/f2fs.h                     |  1 +
>>>>>>  fs/f2fs/file.c                     |  3 ++-
>>>>>>  fs/f2fs/namei.c                    |  9 ++++++---
>>>>>>  fs/f2fs/super.c                    | 13 +++++++++++++
>>>>>>  6 files changed, 28 insertions(+), 5 deletions(-)
>>>>>>
>>>
>>> .
>>>
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot