-----邮件原件-----
发件人: Chao Yu <c...@kernel.org>
发送时间: 2024年6月25日 14:55
收件人: jaeg...@kernel.org
抄送: linux-f2fs-devel@lists.sourceforge.net; linux-ker...@vger.kernel.org; Chao
Yu <c...@kernel.org>; wangzijie <wangzij...@honor.com>; 牛志国 (Zhiguo Niu)
<zhiguo....@unisoc.com>; Yunlei He <heyun...@hihonor.com>
主题: [PATCH v4] f2fs: reduce expensive checkpoint trigger frequency
注意: 这封邮件来自于外部。除非你确定邮件内容安全，否则不要点击任何链接和附件。
CAUTION: This email originated from outside of the organization. Do not click
links or open attachments unless you recognize the sender and know the content
is safe.
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 <wangzij...@honor.com>
Reported-by: Zhiguo Niu <zhiguo....@unisoc.com>
Tested-by: Zhiguo Niu <zhiguo....@unisoc.com>
Reported-by: Yunlei He <heyun...@hihonor.com>
Signed-off-by: Chao Yu <c...@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 f1d65ee3addf..f3c910b8983b
100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -284,6 +284,7 @@ enum {
APPEND_INO, /* for append ino list */
UPDATE_INO, /* for update ino list */
TRANS_DIR_INO, /* for transactions dir ino list */
+ ENC_DIR_INO, /* for encrypted dir ino list */
FLUSH_INO, /* for multiple device flushing */
MAX_INO_ENTRY, /* max. list */
};
@@ -1150,6 +1151,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 a527de1e7a2f..278573974db4
100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -217,6 +217,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 f290fe9327c4..d04c0b47a4e4
100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -629,6 +629,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;
@@ -772,9 +773,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);
Hi Chao,
For this case, will it also cause the same issue with original issue when
fsync_mode == FSYNC_MODE_STRICT ?
if ckpt thread is blocked by some reasons and SBI_NEED_CP is not cleared in
time, Subsequent fsync will trigger cp?
+ else
+ f2fs_add_ino_entry(sbi, inode->i_ino, ENC_DIR_INO);
This patch version regardless of whether dir is encrypted or not, so this
name(ENC_DIR_INO) can be rename other for more accurate?
Thanks！
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
ed794b5fefbe..e4a94995e9a8 100644
--- a/include/trace/events/f2fs.h
+++ b/include/trace/events/f2fs.h
@@ -139,7 +139,8 @@ TRACE_DEFINE_ENUM(EX_BLOCK_AGE);
{ 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.40.1
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
