| 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: <746edfbc-ae29-b84b-f83f-4093a398804b@kernel.org>
Date: Fri, 13 Aug 2021 21:42:15 +0800
From: Chao Yu <chao@...nel.org>
To: Yangtao Li <frank.li@...o.com>, jaegeuk@...nel.org
Cc: linux-f2fs-devel@...ts.sourceforge.net,
linux-kernel@...r.kernel.org, Fengnan Chang <changfengnan@...o.com>
Subject: Re: [PATCH v2] f2fs: Don't create discard thread when device not
support realtime discard
On 2021/8/13 18:11, Yangtao Li wrote:
> From: Fengnan Chang <changfengnan@...o.com>
>
> Don't create discard thread when device not support realtime discard.
>
> Signed-off-by: Fengnan Chang <changfengnan@...o.com>
> Signed-off-by: Yangtao Li <frank.li@...o.com>
> ---
> fs/f2fs/f2fs.h | 1 +
> fs/f2fs/segment.c | 29 +++++++++++++++++++++--------
> fs/f2fs/super.c | 34 ++++++++++++++++++++++++++++++++--
> 3 files changed, 54 insertions(+), 10 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index d24fd5045712..60a408af53a3 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3483,6 +3483,7 @@ int f2fs_flush_device_cache(struct f2fs_sb_info *sbi);
> void f2fs_destroy_flush_cmd_control(struct f2fs_sb_info *sbi, bool free);
> void f2fs_invalidate_blocks(struct f2fs_sb_info *sbi, block_t addr);
> bool f2fs_is_checkpointed_data(struct f2fs_sb_info *sbi, block_t blkaddr);
> +int f2fs_start_discard_thread(struct f2fs_sb_info *sbi);
> void f2fs_drop_discard_cmd(struct f2fs_sb_info *sbi);
> void f2fs_stop_discard_thread(struct f2fs_sb_info *sbi);
> bool f2fs_issue_discard_timeout(struct f2fs_sb_info *sbi);
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index ca9876a6d396..b83a4a1e5023 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -2112,7 +2112,27 @@ void f2fs_clear_prefree_segments(struct f2fs_sb_info *sbi,
> wakeup:
> wake_up_discard_thread(sbi, false);
> }
Need a blank line here.
> +int f2fs_start_discard_thread(struct f2fs_sb_info *sbi)
> +{
> + dev_t dev = sbi->sb->s_bdev->bd_dev;
> + struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
> + int err = 0;
>
> + if (!dcc)
> + return -EINVAL;
> + if (!f2fs_realtime_discard_enable(sbi))
> + return 0;
> +
> + dcc->f2fs_issue_discard = kthread_run(issue_discard_thread, sbi,
> + "f2fs_discard-%u:%u", MAJOR(dev), MINOR(dev));
> + if (IS_ERR(dcc->f2fs_issue_discard)) {
> + err = PTR_ERR(dcc->f2fs_issue_discard);
> + kfree(dcc);
> + SM_I(sbi)->dcc_info = NULL;
> + return err;
> + }
> + return err;
> +}
Ditto,
> static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
> {
> dev_t dev = sbi->sb->s_bdev->bd_dev;
Need to remove unused dev.
> @@ -2153,14 +2173,7 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
> init_waitqueue_head(&dcc->discard_wait_queue);
> SM_I(sbi)->dcc_info = dcc;
> init_thread:
> - dcc->f2fs_issue_discard = kthread_run(issue_discard_thread, sbi,
> - "f2fs_discard-%u:%u", MAJOR(dev), MINOR(dev));
> - if (IS_ERR(dcc->f2fs_issue_discard)) {
> - err = PTR_ERR(dcc->f2fs_issue_discard);
> - kfree(dcc);
> - SM_I(sbi)->dcc_info = NULL;
> - return err;
> - }
> + err = f2fs_start_discard_thread(sbi);
>
> return err;
return f2fs_start_discard_thread(sbi);
err becomes unused.
> }
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 84cd085020cd..ff19c30cd6a1 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -2101,12 +2101,15 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
> bool need_restart_gc = false, need_stop_gc = false;
> bool need_restart_ckpt = false, need_stop_ckpt = false;
> bool need_restart_flush = false, need_stop_flush = false;
> + bool need_enable_ckpt = false, need_disable_ckpt = false;
> bool no_extent_cache = !test_opt(sbi, EXTENT_CACHE);
> bool enable_checkpoint = !test_opt(sbi, DISABLE_CHECKPOINT);
> bool no_io_align = !F2FS_IO_ALIGNED(sbi);
> bool no_atgc = !test_opt(sbi, ATGC);
> + bool no_discard = !test_opt(sbi, DISCARD);
> bool no_compress_cache = !test_opt(sbi, COMPRESS_CACHE);
> bool block_unit_discard = f2fs_block_unit_discard(sbi);
> + struct discard_cmd_control *dcc;
> #ifdef CONFIG_QUOTA
> int i, j;
> #endif
> @@ -2274,7 +2277,7 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
> } else {
> err = f2fs_create_flush_cmd_control(sbi);
> if (err)
> - goto restore_ckpt;
> + goto restore_ckpt_thread;
> need_stop_flush = true;
> }
>
> @@ -2283,8 +2286,28 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
> err = f2fs_disable_checkpoint(sbi);
> if (err)
> goto restore_flush;
> + need_enable_ckpt = true;
> } else {
> f2fs_enable_checkpoint(sbi);
> + need_disable_ckpt = true;
> + }
> + }
> +
> + if (no_discard == !!test_opt(sbi, DISCARD)) {
> + if (test_opt(sbi, DISCARD)) {
> + err = f2fs_start_discard_thread(sbi);
> + if (err)
> + goto restore_ckpt;
> +
Unneeded blank line.
> + } else {
> + dcc = SM_I(sbi)->dcc_info;
> + if (!dcc) {
> + err = -EINVAL;
> + goto restore_ckpt;
> + }
> + f2fs_stop_discard_thread(sbi);
> + if (unlikely(atomic_read(&dcc->discard_cmd_cnt)))
I don't think this is an unlikely case.
> + f2fs_issue_discard_timeout(sbi);
How about starting/stopping discard thread after flush thread status update,
leaving complicated checkpoint disabling status change in the last stage of
remount().
Thanks,
> }
> }
>
> @@ -2302,6 +2325,13 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
> adjust_unusable_cap_perc(sbi);
> *flags = (*flags & ~SB_LAZYTIME) | (sb->s_flags & SB_LAZYTIME);
> return 0;
> +restore_ckpt:
> + if (need_enable_ckpt) {
> + f2fs_enable_checkpoint(sbi);
> + } else if (need_disable_ckpt) {
> + if (f2fs_disable_checkpoint(sbi))
> + f2fs_warn(sbi, "checkpoint has been enable");
> + }
> restore_flush:
> if (need_restart_flush) {
> if (f2fs_create_flush_cmd_control(sbi))
> @@ -2310,7 +2340,7 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
> clear_opt(sbi, FLUSH_MERGE);
> f2fs_destroy_flush_cmd_control(sbi, false);
> }
> -restore_ckpt:
> +restore_ckpt_thread:
> if (need_restart_ckpt) {
> if (f2fs_start_ckpt_thread(sbi))
> f2fs_warn(sbi, "background ckpt thread has stopped");
>
Powered by blists - more mailing lists