From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Chao Yu <yuchao0@huawei.com>
Cc: linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH 4/4] f2fs: clean up bggc mount option
Date: Fri, 14 Feb 2020 10:42:24 -0800 [thread overview]
Message-ID: <20200214184224.GC60913@google.com> (raw)
In-Reply-To: <20200214094413.12784-4-yuchao0@huawei.com>
On 02/14, Chao Yu wrote:
> There are three status for background gc: on, off and sync, it's
> a little bit confused to use test_opt(BG_GC) and test_opt(FORCE_FG_GC)
> combinations to indicate status of background gc.
>
> So let's remove F2FS_MOUNT_BG_GC and F2FS_MOUNT_FORCE_FG_GC mount
I don't think we can do as well.
> options, and add F2FS_OPTION().bggc_mode with below three status
> to clean up codes and enhance bggc mode's scalability.
>
> enum {
> BGGC_MODE_ON, /* background gc is on */
> BGGC_MODE_OFF, /* background gc is off */
> BGGC_MODE_SYNC, /*
> * background gc is on, migrating blocks
> * like foreground gc
> */
> };
>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
> fs/f2fs/f2fs.h | 12 ++++++++++--
> fs/f2fs/gc.c | 6 +++++-
> fs/f2fs/super.c | 29 +++++++++++++----------------
> 3 files changed, 28 insertions(+), 19 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index d2d50827772c..9f65ba8057ad 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -74,7 +74,6 @@ extern const char *f2fs_fault_name[FAULT_MAX];
> /*
> * For mount options
> */
> -#define F2FS_MOUNT_BG_GC 0x00000001
> #define F2FS_MOUNT_DISABLE_ROLL_FORWARD 0x00000002
> #define F2FS_MOUNT_DISCARD 0x00000004
> #define F2FS_MOUNT_NOHEAP 0x00000008
> @@ -88,7 +87,6 @@ extern const char *f2fs_fault_name[FAULT_MAX];
> #define F2FS_MOUNT_NOBARRIER 0x00000800
> #define F2FS_MOUNT_FASTBOOT 0x00001000
> #define F2FS_MOUNT_EXTENT_CACHE 0x00002000
> -#define F2FS_MOUNT_FORCE_FG_GC 0x00004000
> #define F2FS_MOUNT_DATA_FLUSH 0x00008000
> #define F2FS_MOUNT_FAULT_INJECTION 0x00010000
> #define F2FS_MOUNT_USRQUOTA 0x00080000
> @@ -137,6 +135,7 @@ struct f2fs_mount_info {
> int alloc_mode; /* segment allocation policy */
> int fsync_mode; /* fsync policy */
> int fs_mode; /* fs mode: LFS or ADAPTIVE */
> + int bggc_mode; /* bggc mode: off, on or sync */
> bool test_dummy_encryption; /* test dummy encryption */
> block_t unusable_cap; /* Amount of space allowed to be
> * unusable when disabling checkpoint
> @@ -1170,6 +1169,15 @@ enum {
> GC_URGENT,
> };
>
> +enum {
> + BGGC_MODE_ON, /* background gc is on */
> + BGGC_MODE_OFF, /* background gc is off */
> + BGGC_MODE_SYNC, /*
> + * background gc is on, migrating blocks
> + * like foreground gc
> + */
> +};
> +
> enum {
> FS_MODE_ADAPTIVE, /* use both lfs/ssr allocation */
> FS_MODE_LFS, /* use lfs allocation only */
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 8aebe2b9c655..897de003e423 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -31,6 +31,8 @@ static int gc_thread_func(void *data)
>
> set_freezable();
> do {
> + bool sync_mode;
> +
> wait_event_interruptible_timeout(*wq,
> kthread_should_stop() || freezing(current) ||
> gc_th->gc_wake,
> @@ -101,8 +103,10 @@ static int gc_thread_func(void *data)
> do_gc:
> stat_inc_bggc_count(sbi->stat_info);
>
> + sync_mode = F2FS_OPTION(sbi).bggc_mode == BGGC_MODE_SYNC;
> +
> /* if return value is not zero, no victim was selected */
> - if (f2fs_gc(sbi, test_opt(sbi, FORCE_FG_GC), true, NULL_SEGNO))
> + if (f2fs_gc(sbi, sync_mode, true, NULL_SEGNO))
> wait_ms = gc_th->no_gc_sleep_time;
>
> trace_f2fs_background_gc(sbi->sb, wait_ms,
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 427409eff354..4ef7e6eb37da 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -427,14 +427,11 @@ static int parse_options(struct super_block *sb, char *options)
> if (!name)
> return -ENOMEM;
> if (strlen(name) == 2 && !strncmp(name, "on", 2)) {
> - set_opt(sbi, BG_GC);
> - clear_opt(sbi, FORCE_FG_GC);
> + F2FS_OPTION(sbi).bggc_mode = BGGC_MODE_ON;
> } else if (strlen(name) == 3 && !strncmp(name, "off", 3)) {
> - clear_opt(sbi, BG_GC);
> - clear_opt(sbi, FORCE_FG_GC);
> + F2FS_OPTION(sbi).bggc_mode = BGGC_MODE_OFF;
> } else if (strlen(name) == 4 && !strncmp(name, "sync", 4)) {
> - set_opt(sbi, BG_GC);
> - set_opt(sbi, FORCE_FG_GC);
> + F2FS_OPTION(sbi).bggc_mode = BGGC_MODE_SYNC;
> } else {
> kvfree(name);
> return -EINVAL;
> @@ -1436,14 +1433,13 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
> {
> struct f2fs_sb_info *sbi = F2FS_SB(root->d_sb);
>
> - if (!f2fs_readonly(sbi->sb) && test_opt(sbi, BG_GC)) {
> - if (test_opt(sbi, FORCE_FG_GC))
> - seq_printf(seq, ",background_gc=%s", "sync");
> - else
> - seq_printf(seq, ",background_gc=%s", "on");
> - } else {
> + if (F2FS_OPTION(sbi).bggc_mode == BGGC_MODE_SYNC)
> + seq_printf(seq, ",background_gc=%s", "sync");
> + else if (F2FS_OPTION(sbi).bggc_mode == BGGC_MODE_ON)
> + seq_printf(seq, ",background_gc=%s", "on");
> + else if (F2FS_OPTION(sbi).bggc_mode == BGGC_MODE_OFF)
> seq_printf(seq, ",background_gc=%s", "off");
> - }
> +
> if (test_opt(sbi, DISABLE_ROLL_FORWARD))
> seq_puts(seq, ",disable_roll_forward");
> if (test_opt(sbi, DISCARD))
> @@ -1573,8 +1569,8 @@ static void default_options(struct f2fs_sb_info *sbi)
> F2FS_OPTION(sbi).compress_algorithm = COMPRESS_LZO;
> F2FS_OPTION(sbi).compress_log_size = MIN_COMPRESS_LOG_SIZE;
> F2FS_OPTION(sbi).compress_ext_cnt = 0;
> + F2FS_OPTION(sbi).bggc_mode = BGGC_MODE_ON;
>
> - set_opt(sbi, BG_GC);
> set_opt(sbi, INLINE_XATTR);
> set_opt(sbi, INLINE_DATA);
> set_opt(sbi, INLINE_DENTRY);
> @@ -1780,7 +1776,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
> * or if background_gc = off is passed in mount
> * option. Also sync the filesystem.
> */
> - if ((*flags & SB_RDONLY) || !test_opt(sbi, BG_GC)) {
> + if ((*flags & SB_RDONLY) ||
> + F2FS_OPTION(sbi).bggc_mode == BGGC_MODE_OFF) {
> if (sbi->gc_thread) {
> f2fs_stop_gc_thread(sbi);
> need_restart_gc = true;
> @@ -3664,7 +3661,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
> * If filesystem is not mounted as read-only then
> * do start the gc_thread.
> */
> - if (test_opt(sbi, BG_GC) && !f2fs_readonly(sb)) {
> + if (F2FS_OPTION(sbi).bggc_mode != BGGC_MODE_OFF && !f2fs_readonly(sb)) {
> /* After POR, we can run background GC thread.*/
> err = f2fs_start_gc_thread(sbi);
> if (err)
> --
> 2.18.0.rc1
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
WARNING: multiple messages have this Message-ID (diff)
From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Chao Yu <yuchao0@huawei.com>
Cc: linux-f2fs-devel@lists.sourceforge.net,
linux-kernel@vger.kernel.org, chao@kernel.org
Subject: Re: [PATCH 4/4] f2fs: clean up bggc mount option
Date: Fri, 14 Feb 2020 10:42:24 -0800 [thread overview]
Message-ID: <20200214184224.GC60913@google.com> (raw)
In-Reply-To: <20200214094413.12784-4-yuchao0@huawei.com>
On 02/14, Chao Yu wrote:
> There are three status for background gc: on, off and sync, it's
> a little bit confused to use test_opt(BG_GC) and test_opt(FORCE_FG_GC)
> combinations to indicate status of background gc.
>
> So let's remove F2FS_MOUNT_BG_GC and F2FS_MOUNT_FORCE_FG_GC mount
I don't think we can do as well.
> options, and add F2FS_OPTION().bggc_mode with below three status
> to clean up codes and enhance bggc mode's scalability.
>
> enum {
> BGGC_MODE_ON, /* background gc is on */
> BGGC_MODE_OFF, /* background gc is off */
> BGGC_MODE_SYNC, /*
> * background gc is on, migrating blocks
> * like foreground gc
> */
> };
>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
> fs/f2fs/f2fs.h | 12 ++++++++++--
> fs/f2fs/gc.c | 6 +++++-
> fs/f2fs/super.c | 29 +++++++++++++----------------
> 3 files changed, 28 insertions(+), 19 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index d2d50827772c..9f65ba8057ad 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -74,7 +74,6 @@ extern const char *f2fs_fault_name[FAULT_MAX];
> /*
> * For mount options
> */
> -#define F2FS_MOUNT_BG_GC 0x00000001
> #define F2FS_MOUNT_DISABLE_ROLL_FORWARD 0x00000002
> #define F2FS_MOUNT_DISCARD 0x00000004
> #define F2FS_MOUNT_NOHEAP 0x00000008
> @@ -88,7 +87,6 @@ extern const char *f2fs_fault_name[FAULT_MAX];
> #define F2FS_MOUNT_NOBARRIER 0x00000800
> #define F2FS_MOUNT_FASTBOOT 0x00001000
> #define F2FS_MOUNT_EXTENT_CACHE 0x00002000
> -#define F2FS_MOUNT_FORCE_FG_GC 0x00004000
> #define F2FS_MOUNT_DATA_FLUSH 0x00008000
> #define F2FS_MOUNT_FAULT_INJECTION 0x00010000
> #define F2FS_MOUNT_USRQUOTA 0x00080000
> @@ -137,6 +135,7 @@ struct f2fs_mount_info {
> int alloc_mode; /* segment allocation policy */
> int fsync_mode; /* fsync policy */
> int fs_mode; /* fs mode: LFS or ADAPTIVE */
> + int bggc_mode; /* bggc mode: off, on or sync */
> bool test_dummy_encryption; /* test dummy encryption */
> block_t unusable_cap; /* Amount of space allowed to be
> * unusable when disabling checkpoint
> @@ -1170,6 +1169,15 @@ enum {
> GC_URGENT,
> };
>
> +enum {
> + BGGC_MODE_ON, /* background gc is on */
> + BGGC_MODE_OFF, /* background gc is off */
> + BGGC_MODE_SYNC, /*
> + * background gc is on, migrating blocks
> + * like foreground gc
> + */
> +};
> +
> enum {
> FS_MODE_ADAPTIVE, /* use both lfs/ssr allocation */
> FS_MODE_LFS, /* use lfs allocation only */
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 8aebe2b9c655..897de003e423 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -31,6 +31,8 @@ static int gc_thread_func(void *data)
>
> set_freezable();
> do {
> + bool sync_mode;
> +
> wait_event_interruptible_timeout(*wq,
> kthread_should_stop() || freezing(current) ||
> gc_th->gc_wake,
> @@ -101,8 +103,10 @@ static int gc_thread_func(void *data)
> do_gc:
> stat_inc_bggc_count(sbi->stat_info);
>
> + sync_mode = F2FS_OPTION(sbi).bggc_mode == BGGC_MODE_SYNC;
> +
> /* if return value is not zero, no victim was selected */
> - if (f2fs_gc(sbi, test_opt(sbi, FORCE_FG_GC), true, NULL_SEGNO))
> + if (f2fs_gc(sbi, sync_mode, true, NULL_SEGNO))
> wait_ms = gc_th->no_gc_sleep_time;
>
> trace_f2fs_background_gc(sbi->sb, wait_ms,
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 427409eff354..4ef7e6eb37da 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -427,14 +427,11 @@ static int parse_options(struct super_block *sb, char *options)
> if (!name)
> return -ENOMEM;
> if (strlen(name) == 2 && !strncmp(name, "on", 2)) {
> - set_opt(sbi, BG_GC);
> - clear_opt(sbi, FORCE_FG_GC);
> + F2FS_OPTION(sbi).bggc_mode = BGGC_MODE_ON;
> } else if (strlen(name) == 3 && !strncmp(name, "off", 3)) {
> - clear_opt(sbi, BG_GC);
> - clear_opt(sbi, FORCE_FG_GC);
> + F2FS_OPTION(sbi).bggc_mode = BGGC_MODE_OFF;
> } else if (strlen(name) == 4 && !strncmp(name, "sync", 4)) {
> - set_opt(sbi, BG_GC);
> - set_opt(sbi, FORCE_FG_GC);
> + F2FS_OPTION(sbi).bggc_mode = BGGC_MODE_SYNC;
> } else {
> kvfree(name);
> return -EINVAL;
> @@ -1436,14 +1433,13 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
> {
> struct f2fs_sb_info *sbi = F2FS_SB(root->d_sb);
>
> - if (!f2fs_readonly(sbi->sb) && test_opt(sbi, BG_GC)) {
> - if (test_opt(sbi, FORCE_FG_GC))
> - seq_printf(seq, ",background_gc=%s", "sync");
> - else
> - seq_printf(seq, ",background_gc=%s", "on");
> - } else {
> + if (F2FS_OPTION(sbi).bggc_mode == BGGC_MODE_SYNC)
> + seq_printf(seq, ",background_gc=%s", "sync");
> + else if (F2FS_OPTION(sbi).bggc_mode == BGGC_MODE_ON)
> + seq_printf(seq, ",background_gc=%s", "on");
> + else if (F2FS_OPTION(sbi).bggc_mode == BGGC_MODE_OFF)
> seq_printf(seq, ",background_gc=%s", "off");
> - }
> +
> if (test_opt(sbi, DISABLE_ROLL_FORWARD))
> seq_puts(seq, ",disable_roll_forward");
> if (test_opt(sbi, DISCARD))
> @@ -1573,8 +1569,8 @@ static void default_options(struct f2fs_sb_info *sbi)
> F2FS_OPTION(sbi).compress_algorithm = COMPRESS_LZO;
> F2FS_OPTION(sbi).compress_log_size = MIN_COMPRESS_LOG_SIZE;
> F2FS_OPTION(sbi).compress_ext_cnt = 0;
> + F2FS_OPTION(sbi).bggc_mode = BGGC_MODE_ON;
>
> - set_opt(sbi, BG_GC);
> set_opt(sbi, INLINE_XATTR);
> set_opt(sbi, INLINE_DATA);
> set_opt(sbi, INLINE_DENTRY);
> @@ -1780,7 +1776,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data)
> * or if background_gc = off is passed in mount
> * option. Also sync the filesystem.
> */
> - if ((*flags & SB_RDONLY) || !test_opt(sbi, BG_GC)) {
> + if ((*flags & SB_RDONLY) ||
> + F2FS_OPTION(sbi).bggc_mode == BGGC_MODE_OFF) {
> if (sbi->gc_thread) {
> f2fs_stop_gc_thread(sbi);
> need_restart_gc = true;
> @@ -3664,7 +3661,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
> * If filesystem is not mounted as read-only then
> * do start the gc_thread.
> */
> - if (test_opt(sbi, BG_GC) && !f2fs_readonly(sb)) {
> + if (F2FS_OPTION(sbi).bggc_mode != BGGC_MODE_OFF && !f2fs_readonly(sb)) {
> /* After POR, we can run background GC thread.*/
> err = f2fs_start_gc_thread(sbi);
> if (err)
> --
> 2.18.0.rc1
next prev parent reply other threads:[~2020-02-14 18:42 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-14 9:44 [f2fs-dev] [PATCH 1/4] f2fs: clean up codes with {f2fs_, }data_blkaddr() Chao Yu
2020-02-14 9:44 ` [PATCH 1/4] f2fs: clean up codes with {f2fs_,}data_blkaddr() Chao Yu
2020-02-14 9:44 ` [f2fs-dev] [PATCH 2/4] f2fs: clean up parameter of macro XATTR_SIZE() Chao Yu
2020-02-14 9:44 ` Chao Yu
2020-02-14 9:44 ` [f2fs-dev] [PATCH 3/4] f2fs: clean up lfs/adaptive mount option Chao Yu
2020-02-14 9:44 ` Chao Yu
2020-02-14 18:41 ` [f2fs-dev] " Jaegeuk Kim
2020-02-14 18:41 ` Jaegeuk Kim
2020-02-17 1:03 ` [f2fs-dev] " Chao Yu
2020-02-17 1:03 ` Chao Yu
2020-02-18 23:24 ` [f2fs-dev] " Jaegeuk Kim
2020-02-18 23:24 ` Jaegeuk Kim
2020-02-14 9:44 ` [f2fs-dev] [PATCH 4/4] f2fs: clean up bggc " Chao Yu
2020-02-14 9:44 ` Chao Yu
2020-02-14 18:42 ` Jaegeuk Kim [this message]
2020-02-14 18:42 ` Jaegeuk Kim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200214184224.GC60913@google.com \
--to=jaegeuk@kernel.org \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
--cc=yuchao0@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.