| From 1cefadb933a98294bb570582cf48776b492c1df0 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Wed, 26 Jun 2024 09:47:27 +0800 |
| Subject: f2fs: reduce expensive checkpoint trigger frequency |
| |
| From: Chao Yu <chao@kernel.org> |
| |
| [ Upstream commit aaf8c0b9ae042494cb4585883b15c1332de77840 ] |
| |
| 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. |
| |
| In order to avoid such performance regression issue, let's record an |
| entry including directory's ino in global cache whenever we update |
| directory's xattr data, and then triggerring checkpoint() only if |
| xattr metadata of target file's parent was updated. |
| |
| This patch updates to cover below no encryption case as well: |
| 1) parent is checkpointed |
| 2) set_xattr(dir) w/ new xnid |
| 3) create(file) |
| 4) fsync(file) |
| |
| Fixes: bbf156f7afa7 ("f2fs: fix lost xattrs of directories") |
| Reported-by: wangzijie <wangzijie1@honor.com> |
| Reported-by: Zhiguo Niu <zhiguo.niu@unisoc.com> |
| Tested-by: Zhiguo Niu <zhiguo.niu@unisoc.com> |
| Reported-by: Yunlei He <heyunlei@hihonor.com> |
| Signed-off-by: Chao Yu <chao@kernel.org> |
| Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| fs/f2fs/f2fs.h | 2 ++ |
| fs/f2fs/file.c | 3 +++ |
| fs/f2fs/xattr.c | 14 ++++++++++++-- |
| include/trace/events/f2fs.h | 3 ++- |
| 4 files changed, 19 insertions(+), 3 deletions(-) |
| |
| diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h |
| index 8126a82b4d26f..f90aaa16bdee6 100644 |
| --- a/fs/f2fs/f2fs.h |
| +++ b/fs/f2fs/f2fs.h |
| @@ -214,6 +214,7 @@ enum { |
| APPEND_INO, /* for append ino list */ |
| UPDATE_INO, /* for update ino list */ |
| TRANS_DIR_INO, /* for transactions dir ino list */ |
| + XATTR_DIR_INO, /* for xattr updated dir ino list */ |
| FLUSH_INO, /* for multiple device flushing */ |
| MAX_INO_ENTRY, /* max. list */ |
| }; |
| @@ -998,6 +999,7 @@ enum cp_reason_type { |
| CP_FASTBOOT_MODE, |
| CP_SPEC_LOG_NUM, |
| CP_RECOVER_DIR, |
| + CP_XATTR_DIR, |
| }; |
| |
| enum iostat_type { |
| diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c |
| index e44cb6bf68b9e..41eec5bfc7b31 100644 |
| --- a/fs/f2fs/file.c |
| +++ b/fs/f2fs/file.c |
| @@ -170,6 +170,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, |
| + XATTR_DIR_INO)) |
| + cp_reason = CP_XATTR_DIR; |
| |
| return cp_reason; |
| } |
| diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c |
| index 5b8ce9c7a5dc2..0b9568480d8f5 100644 |
| --- a/fs/f2fs/xattr.c |
| +++ b/fs/f2fs/xattr.c |
| @@ -607,6 +607,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; |
| nid_t xnid = F2FS_I(inode)->i_xattr_nid; |
| @@ -732,9 +733,18 @@ static int __f2fs_setxattr(struct inode *inode, int index, |
| if (index == F2FS_XATTR_INDEX_ENCRYPTION && |
| !strcmp(name, F2FS_XATTR_NAME_ENCRYPTION_CONTEXT)) |
| f2fs_set_encrypted_inode(inode); |
| - if (S_ISDIR(inode->i_mode)) |
| - set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_CP); |
| |
| + if (!S_ISDIR(inode->i_mode)) |
| + goto same; |
| + /* |
| + * In restrict mode, fsync() always try to trigger checkpoint for all |
| + * metadata consistency, in other mode, it triggers checkpoint when |
| + * parent's xattr metadata was updated. |
| + */ |
| + if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_STRICT) |
| + set_sbi_flag(sbi, SBI_NEED_CP); |
| + else |
| + f2fs_add_ino_entry(sbi, inode->i_ino, XATTR_DIR_INO); |
| same: |
| if (is_inode_flag_set(inode, FI_ACL_MODE)) { |
| inode->i_mode = F2FS_I(inode)->i_acl_mode; |
| diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h |
| index 098d6dff20bef..abffe3a3f39e1 100644 |
| --- a/include/trace/events/f2fs.h |
| +++ b/include/trace/events/f2fs.h |
| @@ -148,7 +148,8 @@ TRACE_DEFINE_ENUM(CP_TRIMMED); |
| { 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_XATTR_DIR, "dir's xattr updated" }) |
| |
| struct victim_sel_policy; |
| struct f2fs_map_blocks; |
| -- |
| 2.43.0 |
| |