Re: [RFC PATCH] f2fs: reduce expensive checkpoint trigger frequency
From: heyunlei 00015531
Date: Thu Apr 22 2021 - 08:35:15 EST
在 2021/4/22 15:14, Chao Yu 写道:
On 2021/4/22 12:06, Jaegeuk Kim wrote:
On 04/16, Chao Yu wrote:
We may trigger high frequent checkpoint for below case:
1. mkdir /mnt/dir1; set dir1 encrypted
2. touch /mnt/file1; fsync /mnt/file1
3. mkdir /mnt/dir2; set dir2 encrypted
4. touch /mnt/file2; fsync /mnt/file2
...
Although, newly created dir and file are not related, due to
commit bbf156f7afa7 ("f2fs: fix lost xattrs of directories"), we will
trigger checkpoint whenever fsync() comes after a new encrypted dir
created.
It'll happen once? How much impact will we hit due to this?
Yunlei reports me this issue, the problems here in Honer device's
specified
environment, most fsync() on regular file triggers a checkpoint() when
mkdir()
happened concurrently, result in causing the performance issue.
Yunlei could explain more about details of this issue. @Yunlei
Hi all，
Many apps launch will create new directories，which will set need
checkpoint flag because
inherited crypto info from their parent. During this time, any
fsynced will write a new cp.
For the worst case:
new dir1
fsync file a (with no relations with dir1), and
write cp n
new dir 2
fsync file b (with no relations with dir2), and
write cp n + 1
... ...
Thanks.
In order to avoid such condition, let's record an entry including
directory's ino into global cache when we initialize encryption policy
in a checkpointed directory, and then only trigger checkpoint() when
target file's parent has non-persisted encryption policy, for the case
its parent is not checkpointed, need_do_checkpoint() has cover that
by verifying it with f2fs_is_checkpointed_node().
Signed-off-by: Chao Yu <yuchao0@xxxxxxxxxx>
---
fs/f2fs/f2fs.h | 2 ++
fs/f2fs/file.c | 3 +++
fs/f2fs/xattr.c | 6 ++++--
include/trace/events/f2fs.h | 3 ++-
4 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 87d734f5589d..34487e527d12 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -246,6 +246,7 @@ enum {
APPEND_INO, /* for append ino list */
UPDATE_INO, /* for update ino list */
TRANS_DIR_INO, /* for trasactions dir ino list */
+ ENC_DIR_INO, /* for encrypted dir ino list */
FLUSH_INO, /* for multiple device flushing */
MAX_INO_ENTRY, /* max. list */
};
@@ -1090,6 +1091,7 @@ enum cp_reason_type {
CP_FASTBOOT_MODE,
CP_SPEC_LOG_NUM,
CP_RECOVER_DIR,
+ CP_ENC_DIR,
};
enum iostat_type {
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 6284b2f4a60b..a6c38d8b1ec3 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -218,6 +218,9 @@ static inline enum cp_reason_type
need_do_checkpoint(struct inode *inode)
f2fs_exist_written_data(sbi, F2FS_I(inode)->i_pino,
TRANS_DIR_INO))
cp_reason = CP_RECOVER_DIR;
+ else if (f2fs_exist_written_data(sbi, F2FS_I(inode)->i_pino,
+ ENC_DIR_INO))
+ cp_reason = CP_ENC_DIR;
return cp_reason;
}
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index c8f34decbf8e..38796d488d15 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -630,6 +630,7 @@ static int __f2fs_setxattr(struct inode *inode,
int index,
const char *name, const void *value, size_t size,
struct page *ipage, int flags)
{
+ struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
struct f2fs_xattr_entry *here, *last;
void *base_addr, *last_base_addr;
int found, newsize;
@@ -745,8 +746,9 @@ static int __f2fs_setxattr(struct inode *inode,
int index,
!strcmp(name, F2FS_XATTR_NAME_ENCRYPTION_CONTEXT))
f2fs_set_encrypted_inode(inode);
f2fs_mark_inode_dirty_sync(inode, true);
- if (!error && S_ISDIR(inode->i_mode))
- set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_CP);
+ if (!error && S_ISDIR(inode->i_mode) &&
+ f2fs_is_checkpointed_node(sbi, inode->i_ino))
+ f2fs_add_ino_entry(sbi, inode->i_ino, ENC_DIR_INO);
Is it right to say ENC_DIR_INO in this case?
Sorry, I didn't get it.
Thanks,
same:
if (is_inode_flag_set(inode, FI_ACL_MODE)) {
diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
index 56b113e3cd6a..ca0cf12226e9 100644
--- a/include/trace/events/f2fs.h
+++ b/include/trace/events/f2fs.h
@@ -145,7 +145,8 @@ TRACE_DEFINE_ENUM(CP_RESIZE);
{ CP_NODE_NEED_CP, "node needs cp" }, \
{ CP_FASTBOOT_MODE, "fastboot mode" }, \
{ CP_SPEC_LOG_NUM, "log type is 2" }, \
- { CP_RECOVER_DIR, "dir needs recovery" })
+ { CP_RECOVER_DIR, "dir needs recovery" }, \
+ { CP_ENC_DIR, "persist encryption policy" })
#define show_shutdown_mode(type) \
__print_symbolic(type, \
--
2.29.2
.