[f2fs-dev] [PATCH v2] f2fs: introduce "strict_fsync" for posix standard fsync
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
Reviewed-by: Chao Yu
---
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 0x0040
#define F2FS_MOUNT_INLINE_XATTR_SIZE 0x0080
#define F2FS_MOUNT_RESERVE_ROOT0x0100
+#define F2FS_MOUNT_STRICT_FSYNC0x0200
#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_to
Re: [f2fs-dev] [PATCH v2] f2fs: introduce "strict_fsync" for posix standard fsync
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
> Reviewed-by: Chao Yu
> ---
> 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 0x0040
> #define F2FS_MOUNT_INLINE_XATTR_SIZE 0x0080
> #define F2FS_MOUNT_RESERVE_ROOT 0x0100
> +#define F2FS_MOUNT_STRICT_FSYNC 0x0200
>
> #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 7966cf7bfb8
Re: [f2fs-dev] [PATCH v2] f2fs: introduce "strict_fsync" for posix standard fsync
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 >> Reviewed-by: Chao Yu >> --- >> 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 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v2] f2fs: introduce "strict_fsync" for posix standard fsync
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 > >> Reviewed-by: Chao Yu > >> --- > >> 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 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v2] f2fs: introduce "strict_fsync" for posix standard fsync
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 Reviewed-by: Chao Yu --- 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 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v2] f2fs: introduce "strict_fsync" for posix standard fsync
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 > Reviewed-by: Chao Yu > --- > 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 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v2] f2fs: introduce "strict_fsync" for posix standard fsync
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 >> Reviewed-by: Chao Yu >> --- >> 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 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
