| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ed3eecaf9e2f4c8fae2fd01241aa097e@BJMBX02.spreadtrum.com>
Date: Mon, 26 Feb 2024 01:21:44 +0000
From: 牛志国 (Zhiguo Niu) <Zhiguo.Niu@...soc.com>
To: "jaegeuk@...nel.org" <jaegeuk@...nel.org>
CC: "linux-f2fs-devel@...ts.sourceforge.net"
<linux-f2fs-devel@...ts.sourceforge.net>,
"linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>,
Chao Yu <chao@...nel.org>,
"niuzhiguo84@...il.com" <niuzhiguo84@...il.com>,
金红宇 (Hongyu Jin) <hongyu.jin@...soc.com>
Subject: 答复: [PATCH v3] f2fs: reduce expensive checkpoint trigger frequency
Hi Jaegeuk
Sorry for disturbing you, Do you have any comments about this patch from Chao, I’ve met this issue several times on our platform when do monkey test.
Thanks!
-----邮件原件-----
发件人: Chao Yu <chao@...nel.org>
发送时间: 2024年2月19日 15:19
收件人: jaegeuk@...nel.org
抄送: linux-f2fs-devel@...ts.sourceforge.net; linux-kernel@...r.kernel.org; 牛志国 (Zhiguo Niu) <Zhiguo.Niu@...soc.com>
主题: Re: [PATCH v3] 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.
Jaegeuk,
Any comments?
On 2024/1/11 16:17, 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.
>
> 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().
>
> Reported-by: Zhiguo Niu <zhiguo.niu@...soc.com>
> Tested-by: Zhiguo Niu <zhiguo.niu@...soc.com>
> Reported-by: Yunlei He <heyunlei@...onor.com>
> Signed-off-by: Chao Yu <chao@...nel.org>
> ---
> v3:
> - Recently, Zhiguo Niu reported the same issue, so I repost this patch
> for comments.
> fs/f2fs/f2fs.h | 2 ++
> fs/f2fs/file.c | 3 +++
> fs/f2fs/xattr.c | 16 ++++++++++++++--
> include/trace/events/f2fs.h | 3 ++-
> 4 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index
> e2e0ca45f881..0094a8c85f4a 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -279,6 +279,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 */
> };
> @@ -1147,6 +1148,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
> 8198afb5fb9c..18b33b1f0c83 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
> f290fe9327c4..cbd1b88297fe 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,8 +773,19 @@ 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)) {
> + /*
> + * In restrict mode, fsync() always tries triggering checkpoint
> + * for all metadata consistency, in other mode, it only triggers
> + * checkpoint when parent's encryption metadata updates.
> + */
> + if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_STRICT)
> + set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_CP);
> + else if (IS_ENCRYPTED(inode) &&
> + f2fs_is_checkpointed_node(sbi, inode->i_ino))
> + f2fs_add_ino_entry(sbi, inode->i_ino, ENC_DIR_INO);
> + }
>
> 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
> 7ed0fc430dc6..48f2e399e184 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, \
Powered by blists - more mailing lists