Re: [f2fs-dev] [PATCH v3] f2fs: Don't create discard thread when device not support realtime discard

[Chao Yu <chao@kernel.org>]

On 2021/8/14 17:08, Fengnan Chang wrote:
> Don't create discard thread when device not support realtime discard.
> 
> Signed-off-by: Fengnan Chang <changfengnan@vivo.com>
> Signed-off-by: Yangtao Li <frank.li@vivo.com>
> ---
>   fs/f2fs/f2fs.h    |  1 +
>   fs/f2fs/segment.c | 36 ++++++++++++++++++++++++------------
>   fs/f2fs/super.c   | 31 ++++++++++++++++++++++++++++++-
>   3 files changed, 55 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 0c978f934dcc..1e0d8d340546 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3590,6 +3590,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 b4dd22134a73..b7de8ef5ccf1 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -2114,11 +2114,32 @@ void f2fs_clear_prefree_segments(struct f2fs_sb_info *sbi,
>   	wake_up_discard_thread(sbi, false);
>   }
>   
> -static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
> +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;

Any case dcc will be NULL?

> +	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;

Please considering the case if we are in recovery flow of remount(), discard
mount option is tagged, but .dcc_info is NULL...

> +		return err;
> +	}
> +	return err;
> +}
> +
> +static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
> +{
>   	struct discard_cmd_control *dcc;
> -	int err = 0, i;
> +	int i;
>   
>   	if (SM_I(sbi)->dcc_info) {
>   		dcc = SM_I(sbi)->dcc_info;
> @@ -2154,16 +2175,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;
> -	}
> -
> -	return err;
> +	return f2fs_start_discard_thread(sbi);
>   }
>   
>   static void destroy_discard_cmd_control(struct f2fs_sb_info *sbi)
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index f92c582f8008..192af573b09c 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -2103,12 +2103,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_start_discard = false, need_stop_discard = false;

need_restart_discard

>   	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
> @@ -2280,11 +2283,30 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
>   		need_stop_flush = true;
>   	}
>   
> +	if (no_discard == !!test_opt(sbi, DISCARD)) {
> +		if (test_opt(sbi, DISCARD)) {
> +			err = f2fs_start_discard_thread(sbi);
> +			if (err)
> +				goto restore_flush;
> +			need_stop_discard = true;
> +		} else {
> +			dcc = SM_I(sbi)->dcc_info;
> +			if (!dcc) {

Any case dcc will be NULL?

> +				err = -EINVAL;
> +				goto restore_flush;
> +			}
> +			f2fs_stop_discard_thread(sbi);
> +			if (atomic_read(&dcc->discard_cmd_cnt))
> +				f2fs_issue_discard_timeout(sbi);
> +			need_start_discard = true;
> +		}
> +	}
> +
>   	if (enable_checkpoint == !!test_opt(sbi, DISABLE_CHECKPOINT)) {
>   		if (test_opt(sbi, DISABLE_CHECKPOINT)) {
>   			err = f2fs_disable_checkpoint(sbi);
>   			if (err)
> -				goto restore_flush;
> +				goto restore_discard;
>   		} else {
>   			f2fs_enable_checkpoint(sbi);
>   		}
> @@ -2304,6 +2326,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_discard:
> +	if (need_start_discard) {
> +		if (f2fs_start_discard_thread(sbi))
> +			f2fs_warn(sbi, "discard has been stopped");
> +	} else if (need_stop_discard) {
> +		f2fs_stop_discard_thread(sbi);
> +	}
>   restore_flush:
>   	if (need_restart_flush) {
>   		if (f2fs_create_flush_cmd_control(sbi))
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel