* [PATCH v3] f2fs: rename checkpoint=merge mount option to checkpoint_merge @ 2021-02-02 9:23 ` Daeho Jeong 0 siblings, 0 replies; 12+ messages in thread From: Daeho Jeong @ 2021-02-02 9:23 UTC (permalink / raw) To: linux-kernel, linux-f2fs-devel, kernel-team; +Cc: Daeho Jeong From: Daeho Jeong <daehojeong@google.com> As checkpoint=merge comes in, mount option setting related to checkpoint had been mixed up and it became hard to understand. So, I separated this option from "checkpoint=" and made another mount option "checkpoint_merge" for this. Signed-off-by: Daeho Jeong <daehojeong@google.com> --- v2: renamed "checkpoint=merge" to "checkpoint_merge" v3: removed "nocheckpoint_merge" option --- Documentation/filesystems/f2fs.rst | 6 +++--- fs/f2fs/super.c | 21 +++++++++------------ 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst index d0ead45dc706..475994ed8b15 100644 --- a/Documentation/filesystems/f2fs.rst +++ b/Documentation/filesystems/f2fs.rst @@ -247,9 +247,9 @@ checkpoint=%s[:%u[%]] Set to "disable" to turn off checkpointing. Set to "enabl hide up to all remaining free space. The actual space that would be unusable can be viewed at /sys/fs/f2fs/<disk>/unusable This space is reclaimed once checkpoint=enable. - Here is another option "merge", which creates a kernel daemon - and makes it to merge concurrent checkpoint requests as much - as possible to eliminate redundant checkpoint issues. Plus, +checkpoint_merge When checkpoint is enabled, this can be used to create a kernel + daemon and make it to merge concurrent checkpoint requests as + much as possible to eliminate redundant checkpoint issues. Plus, we can eliminate the sluggish issue caused by slow checkpoint operation when the checkpoint is done in a process context in a cgroup having low i/o budget and cpu shares. To make this diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 56696f6cfa86..b60dcef7f9d0 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -215,7 +215,7 @@ static match_table_t f2fs_tokens = { {Opt_checkpoint_disable_cap, "checkpoint=disable:%u"}, {Opt_checkpoint_disable_cap_perc, "checkpoint=disable:%u%%"}, {Opt_checkpoint_enable, "checkpoint=enable"}, - {Opt_checkpoint_merge, "checkpoint=merge"}, + {Opt_checkpoint_merge, "checkpoint_merge"}, {Opt_compress_algorithm, "compress_algorithm=%s"}, {Opt_compress_log_size, "compress_log_size=%u"}, {Opt_compress_extension, "compress_extension=%s"}, @@ -1142,12 +1142,6 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) return -EINVAL; } - if (test_opt(sbi, DISABLE_CHECKPOINT) && - test_opt(sbi, MERGE_CHECKPOINT)) { - f2fs_err(sbi, "checkpoint=merge cannot be used with checkpoint=disable\n"); - return -EINVAL; - } - /* Not pass down write hints if the number of active logs is lesser * than NR_CURSEG_PERSIST_TYPE. */ @@ -1782,7 +1776,7 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root) seq_printf(seq, ",checkpoint=disable:%u", F2FS_OPTION(sbi).unusable_cap); if (test_opt(sbi, MERGE_CHECKPOINT)) - seq_puts(seq, ",checkpoint=merge"); + seq_puts(seq, ",checkpoint_merge"); if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_POSIX) seq_printf(seq, ",fsync_mode=%s", "posix"); else if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_STRICT) @@ -1827,6 +1821,7 @@ static void default_options(struct f2fs_sb_info *sbi) sbi->sb->s_flags |= SB_LAZYTIME; set_opt(sbi, FLUSH_MERGE); set_opt(sbi, DISCARD); + clear_opt(sbi, MERGE_CHECKPOINT); if (f2fs_sb_has_blkzoned(sbi)) F2FS_OPTION(sbi).fs_mode = FS_MODE_LFS; else @@ -2066,9 +2061,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) } } - if (!test_opt(sbi, MERGE_CHECKPOINT)) { - f2fs_stop_ckpt_thread(sbi); - } else { + if (!test_opt(sbi, DISABLE_CHECKPOINT) && + test_opt(sbi, MERGE_CHECKPOINT)) { err = f2fs_start_ckpt_thread(sbi); if (err) { f2fs_err(sbi, @@ -2076,6 +2070,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) err); goto restore_gc; } + } else { + f2fs_stop_ckpt_thread(sbi); } /* @@ -3831,7 +3827,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent) /* setup checkpoint request control and start checkpoint issue thread */ f2fs_init_ckpt_req_control(sbi); - if (test_opt(sbi, MERGE_CHECKPOINT)) { + if (!test_opt(sbi, DISABLE_CHECKPOINT) && + test_opt(sbi, MERGE_CHECKPOINT)) { err = f2fs_start_ckpt_thread(sbi); if (err) { f2fs_err(sbi, -- 2.30.0.365.g02bc693789-goog ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [f2fs-dev] [PATCH v3] f2fs: rename checkpoint=merge mount option to checkpoint_merge @ 2021-02-02 9:23 ` Daeho Jeong 0 siblings, 0 replies; 12+ messages in thread From: Daeho Jeong @ 2021-02-02 9:23 UTC (permalink / raw) To: linux-kernel, linux-f2fs-devel, kernel-team; +Cc: Daeho Jeong From: Daeho Jeong <daehojeong@google.com> As checkpoint=merge comes in, mount option setting related to checkpoint had been mixed up and it became hard to understand. So, I separated this option from "checkpoint=" and made another mount option "checkpoint_merge" for this. Signed-off-by: Daeho Jeong <daehojeong@google.com> --- v2: renamed "checkpoint=merge" to "checkpoint_merge" v3: removed "nocheckpoint_merge" option --- Documentation/filesystems/f2fs.rst | 6 +++--- fs/f2fs/super.c | 21 +++++++++------------ 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst index d0ead45dc706..475994ed8b15 100644 --- a/Documentation/filesystems/f2fs.rst +++ b/Documentation/filesystems/f2fs.rst @@ -247,9 +247,9 @@ checkpoint=%s[:%u[%]] Set to "disable" to turn off checkpointing. Set to "enabl hide up to all remaining free space. The actual space that would be unusable can be viewed at /sys/fs/f2fs/<disk>/unusable This space is reclaimed once checkpoint=enable. - Here is another option "merge", which creates a kernel daemon - and makes it to merge concurrent checkpoint requests as much - as possible to eliminate redundant checkpoint issues. Plus, +checkpoint_merge When checkpoint is enabled, this can be used to create a kernel + daemon and make it to merge concurrent checkpoint requests as + much as possible to eliminate redundant checkpoint issues. Plus, we can eliminate the sluggish issue caused by slow checkpoint operation when the checkpoint is done in a process context in a cgroup having low i/o budget and cpu shares. To make this diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 56696f6cfa86..b60dcef7f9d0 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -215,7 +215,7 @@ static match_table_t f2fs_tokens = { {Opt_checkpoint_disable_cap, "checkpoint=disable:%u"}, {Opt_checkpoint_disable_cap_perc, "checkpoint=disable:%u%%"}, {Opt_checkpoint_enable, "checkpoint=enable"}, - {Opt_checkpoint_merge, "checkpoint=merge"}, + {Opt_checkpoint_merge, "checkpoint_merge"}, {Opt_compress_algorithm, "compress_algorithm=%s"}, {Opt_compress_log_size, "compress_log_size=%u"}, {Opt_compress_extension, "compress_extension=%s"}, @@ -1142,12 +1142,6 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) return -EINVAL; } - if (test_opt(sbi, DISABLE_CHECKPOINT) && - test_opt(sbi, MERGE_CHECKPOINT)) { - f2fs_err(sbi, "checkpoint=merge cannot be used with checkpoint=disable\n"); - return -EINVAL; - } - /* Not pass down write hints if the number of active logs is lesser * than NR_CURSEG_PERSIST_TYPE. */ @@ -1782,7 +1776,7 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root) seq_printf(seq, ",checkpoint=disable:%u", F2FS_OPTION(sbi).unusable_cap); if (test_opt(sbi, MERGE_CHECKPOINT)) - seq_puts(seq, ",checkpoint=merge"); + seq_puts(seq, ",checkpoint_merge"); if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_POSIX) seq_printf(seq, ",fsync_mode=%s", "posix"); else if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_STRICT) @@ -1827,6 +1821,7 @@ static void default_options(struct f2fs_sb_info *sbi) sbi->sb->s_flags |= SB_LAZYTIME; set_opt(sbi, FLUSH_MERGE); set_opt(sbi, DISCARD); + clear_opt(sbi, MERGE_CHECKPOINT); if (f2fs_sb_has_blkzoned(sbi)) F2FS_OPTION(sbi).fs_mode = FS_MODE_LFS; else @@ -2066,9 +2061,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) } } - if (!test_opt(sbi, MERGE_CHECKPOINT)) { - f2fs_stop_ckpt_thread(sbi); - } else { + if (!test_opt(sbi, DISABLE_CHECKPOINT) && + test_opt(sbi, MERGE_CHECKPOINT)) { err = f2fs_start_ckpt_thread(sbi); if (err) { f2fs_err(sbi, @@ -2076,6 +2070,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) err); goto restore_gc; } + } else { + f2fs_stop_ckpt_thread(sbi); } /* @@ -3831,7 +3827,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent) /* setup checkpoint request control and start checkpoint issue thread */ f2fs_init_ckpt_req_control(sbi); - if (test_opt(sbi, MERGE_CHECKPOINT)) { + if (!test_opt(sbi, DISABLE_CHECKPOINT) && + test_opt(sbi, MERGE_CHECKPOINT)) { err = f2fs_start_ckpt_thread(sbi); if (err) { f2fs_err(sbi, -- 2.30.0.365.g02bc693789-goog _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [f2fs-dev] [PATCH v3] f2fs: rename checkpoint=merge mount option to checkpoint_merge 2021-02-02 9:23 ` [f2fs-dev] " Daeho Jeong @ 2021-02-02 9:28 ` Chao Yu -1 siblings, 0 replies; 12+ messages in thread From: Chao Yu @ 2021-02-02 9:28 UTC (permalink / raw) To: Daeho Jeong, linux-kernel, linux-f2fs-devel, kernel-team; +Cc: Daeho Jeong On 2021/2/2 17:23, Daeho Jeong wrote: > From: Daeho Jeong <daehojeong@google.com> > > As checkpoint=merge comes in, mount option setting related to checkpoint > had been mixed up and it became hard to understand. So, I separated > this option from "checkpoint=" and made another mount option > "checkpoint_merge" for this. > > Signed-off-by: Daeho Jeong <daehojeong@google.com> > --- > v2: renamed "checkpoint=merge" to "checkpoint_merge" > v3: removed "nocheckpoint_merge" option > --- > Documentation/filesystems/f2fs.rst | 6 +++--- > fs/f2fs/super.c | 21 +++++++++------------ > 2 files changed, 12 insertions(+), 15 deletions(-) > > diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst > index d0ead45dc706..475994ed8b15 100644 > --- a/Documentation/filesystems/f2fs.rst > +++ b/Documentation/filesystems/f2fs.rst > @@ -247,9 +247,9 @@ checkpoint=%s[:%u[%]] Set to "disable" to turn off checkpointing. Set to "enabl > hide up to all remaining free space. The actual space that > would be unusable can be viewed at /sys/fs/f2fs/<disk>/unusable > This space is reclaimed once checkpoint=enable. > - Here is another option "merge", which creates a kernel daemon > - and makes it to merge concurrent checkpoint requests as much > - as possible to eliminate redundant checkpoint issues. Plus, > +checkpoint_merge When checkpoint is enabled, this can be used to create a kernel > + daemon and make it to merge concurrent checkpoint requests as > + much as possible to eliminate redundant checkpoint issues. Plus, > we can eliminate the sluggish issue caused by slow checkpoint > operation when the checkpoint is done in a process context in > a cgroup having low i/o budget and cpu shares. To make this > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > index 56696f6cfa86..b60dcef7f9d0 100644 > --- a/fs/f2fs/super.c > +++ b/fs/f2fs/super.c > @@ -215,7 +215,7 @@ static match_table_t f2fs_tokens = { > {Opt_checkpoint_disable_cap, "checkpoint=disable:%u"}, > {Opt_checkpoint_disable_cap_perc, "checkpoint=disable:%u%%"}, > {Opt_checkpoint_enable, "checkpoint=enable"}, > - {Opt_checkpoint_merge, "checkpoint=merge"}, > + {Opt_checkpoint_merge, "checkpoint_merge"}, > {Opt_compress_algorithm, "compress_algorithm=%s"}, > {Opt_compress_log_size, "compress_log_size=%u"}, > {Opt_compress_extension, "compress_extension=%s"}, > @@ -1142,12 +1142,6 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) > return -EINVAL; > } > > - if (test_opt(sbi, DISABLE_CHECKPOINT) && > - test_opt(sbi, MERGE_CHECKPOINT)) { > - f2fs_err(sbi, "checkpoint=merge cannot be used with checkpoint=disable\n"); > - return -EINVAL; > - } > - > /* Not pass down write hints if the number of active logs is lesser > * than NR_CURSEG_PERSIST_TYPE. > */ > @@ -1782,7 +1776,7 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root) > seq_printf(seq, ",checkpoint=disable:%u", > F2FS_OPTION(sbi).unusable_cap); > if (test_opt(sbi, MERGE_CHECKPOINT)) > - seq_puts(seq, ",checkpoint=merge"); > + seq_puts(seq, ",checkpoint_merge"); > if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_POSIX) > seq_printf(seq, ",fsync_mode=%s", "posix"); > else if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_STRICT) > @@ -1827,6 +1821,7 @@ static void default_options(struct f2fs_sb_info *sbi) > sbi->sb->s_flags |= SB_LAZYTIME; > set_opt(sbi, FLUSH_MERGE); > set_opt(sbi, DISCARD); > + clear_opt(sbi, MERGE_CHECKPOINT); It's not needed here? Thanks, > if (f2fs_sb_has_blkzoned(sbi)) > F2FS_OPTION(sbi).fs_mode = FS_MODE_LFS; > else > @@ -2066,9 +2061,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) > } > } > > - if (!test_opt(sbi, MERGE_CHECKPOINT)) { > - f2fs_stop_ckpt_thread(sbi); > - } else { > + if (!test_opt(sbi, DISABLE_CHECKPOINT) && > + test_opt(sbi, MERGE_CHECKPOINT)) { > err = f2fs_start_ckpt_thread(sbi); > if (err) { > f2fs_err(sbi, > @@ -2076,6 +2070,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) > err); > goto restore_gc; > } > + } else { > + f2fs_stop_ckpt_thread(sbi); > } > > /* > @@ -3831,7 +3827,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent) > > /* setup checkpoint request control and start checkpoint issue thread */ > f2fs_init_ckpt_req_control(sbi); > - if (test_opt(sbi, MERGE_CHECKPOINT)) { > + if (!test_opt(sbi, DISABLE_CHECKPOINT) && > + test_opt(sbi, MERGE_CHECKPOINT)) { > err = f2fs_start_ckpt_thread(sbi); > if (err) { > f2fs_err(sbi, > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [f2fs-dev] [PATCH v3] f2fs: rename checkpoint=merge mount option to checkpoint_merge @ 2021-02-02 9:28 ` Chao Yu 0 siblings, 0 replies; 12+ messages in thread From: Chao Yu @ 2021-02-02 9:28 UTC (permalink / raw) To: Daeho Jeong, linux-kernel, linux-f2fs-devel, kernel-team; +Cc: Daeho Jeong On 2021/2/2 17:23, Daeho Jeong wrote: > From: Daeho Jeong <daehojeong@google.com> > > As checkpoint=merge comes in, mount option setting related to checkpoint > had been mixed up and it became hard to understand. So, I separated > this option from "checkpoint=" and made another mount option > "checkpoint_merge" for this. > > Signed-off-by: Daeho Jeong <daehojeong@google.com> > --- > v2: renamed "checkpoint=merge" to "checkpoint_merge" > v3: removed "nocheckpoint_merge" option > --- > Documentation/filesystems/f2fs.rst | 6 +++--- > fs/f2fs/super.c | 21 +++++++++------------ > 2 files changed, 12 insertions(+), 15 deletions(-) > > diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst > index d0ead45dc706..475994ed8b15 100644 > --- a/Documentation/filesystems/f2fs.rst > +++ b/Documentation/filesystems/f2fs.rst > @@ -247,9 +247,9 @@ checkpoint=%s[:%u[%]] Set to "disable" to turn off checkpointing. Set to "enabl > hide up to all remaining free space. The actual space that > would be unusable can be viewed at /sys/fs/f2fs/<disk>/unusable > This space is reclaimed once checkpoint=enable. > - Here is another option "merge", which creates a kernel daemon > - and makes it to merge concurrent checkpoint requests as much > - as possible to eliminate redundant checkpoint issues. Plus, > +checkpoint_merge When checkpoint is enabled, this can be used to create a kernel > + daemon and make it to merge concurrent checkpoint requests as > + much as possible to eliminate redundant checkpoint issues. Plus, > we can eliminate the sluggish issue caused by slow checkpoint > operation when the checkpoint is done in a process context in > a cgroup having low i/o budget and cpu shares. To make this > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > index 56696f6cfa86..b60dcef7f9d0 100644 > --- a/fs/f2fs/super.c > +++ b/fs/f2fs/super.c > @@ -215,7 +215,7 @@ static match_table_t f2fs_tokens = { > {Opt_checkpoint_disable_cap, "checkpoint=disable:%u"}, > {Opt_checkpoint_disable_cap_perc, "checkpoint=disable:%u%%"}, > {Opt_checkpoint_enable, "checkpoint=enable"}, > - {Opt_checkpoint_merge, "checkpoint=merge"}, > + {Opt_checkpoint_merge, "checkpoint_merge"}, > {Opt_compress_algorithm, "compress_algorithm=%s"}, > {Opt_compress_log_size, "compress_log_size=%u"}, > {Opt_compress_extension, "compress_extension=%s"}, > @@ -1142,12 +1142,6 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) > return -EINVAL; > } > > - if (test_opt(sbi, DISABLE_CHECKPOINT) && > - test_opt(sbi, MERGE_CHECKPOINT)) { > - f2fs_err(sbi, "checkpoint=merge cannot be used with checkpoint=disable\n"); > - return -EINVAL; > - } > - > /* Not pass down write hints if the number of active logs is lesser > * than NR_CURSEG_PERSIST_TYPE. > */ > @@ -1782,7 +1776,7 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root) > seq_printf(seq, ",checkpoint=disable:%u", > F2FS_OPTION(sbi).unusable_cap); > if (test_opt(sbi, MERGE_CHECKPOINT)) > - seq_puts(seq, ",checkpoint=merge"); > + seq_puts(seq, ",checkpoint_merge"); > if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_POSIX) > seq_printf(seq, ",fsync_mode=%s", "posix"); > else if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_STRICT) > @@ -1827,6 +1821,7 @@ static void default_options(struct f2fs_sb_info *sbi) > sbi->sb->s_flags |= SB_LAZYTIME; > set_opt(sbi, FLUSH_MERGE); > set_opt(sbi, DISCARD); > + clear_opt(sbi, MERGE_CHECKPOINT); It's not needed here? Thanks, > if (f2fs_sb_has_blkzoned(sbi)) > F2FS_OPTION(sbi).fs_mode = FS_MODE_LFS; > else > @@ -2066,9 +2061,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) > } > } > > - if (!test_opt(sbi, MERGE_CHECKPOINT)) { > - f2fs_stop_ckpt_thread(sbi); > - } else { > + if (!test_opt(sbi, DISABLE_CHECKPOINT) && > + test_opt(sbi, MERGE_CHECKPOINT)) { > err = f2fs_start_ckpt_thread(sbi); > if (err) { > f2fs_err(sbi, > @@ -2076,6 +2070,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) > err); > goto restore_gc; > } > + } else { > + f2fs_stop_ckpt_thread(sbi); > } > > /* > @@ -3831,7 +3827,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent) > > /* setup checkpoint request control and start checkpoint issue thread */ > f2fs_init_ckpt_req_control(sbi); > - if (test_opt(sbi, MERGE_CHECKPOINT)) { > + if (!test_opt(sbi, DISABLE_CHECKPOINT) && > + test_opt(sbi, MERGE_CHECKPOINT)) { > err = f2fs_start_ckpt_thread(sbi); > if (err) { > f2fs_err(sbi, > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [f2fs-dev] [PATCH v3] f2fs: rename checkpoint=merge mount option to checkpoint_merge 2021-02-02 9:28 ` Chao Yu @ 2021-02-02 9:44 ` Daeho Jeong -1 siblings, 0 replies; 12+ messages in thread From: Daeho Jeong @ 2021-02-02 9:44 UTC (permalink / raw) To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong When we remount it without the "checkpoint_merge" option, shouldn't we need to clear "MERGE_CHECKPOINT" again? This is actually what I intended, but I was wrong. Actually, I found this. When we remount the filesystem, the previous mount option is passed through the "data" argument in the below. f2fs_remount(struct super_block *sb, int *flags, char *data) If we don't provide the "nocheckpoint_merge" option, how can we turn off the "checkpoint_merge" option which is turned on in the previous mount? 2021년 2월 2일 (화) 오후 6:28, Chao Yu <yuchao0@huawei.com>님이 작성: > > On 2021/2/2 17:23, Daeho Jeong wrote: > > From: Daeho Jeong <daehojeong@google.com> > > > > As checkpoint=merge comes in, mount option setting related to checkpoint > > had been mixed up and it became hard to understand. So, I separated > > this option from "checkpoint=" and made another mount option > > "checkpoint_merge" for this. > > > > Signed-off-by: Daeho Jeong <daehojeong@google.com> > > --- > > v2: renamed "checkpoint=merge" to "checkpoint_merge" > > v3: removed "nocheckpoint_merge" option > > --- > > Documentation/filesystems/f2fs.rst | 6 +++--- > > fs/f2fs/super.c | 21 +++++++++------------ > > 2 files changed, 12 insertions(+), 15 deletions(-) > > > > diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst > > index d0ead45dc706..475994ed8b15 100644 > > --- a/Documentation/filesystems/f2fs.rst > > +++ b/Documentation/filesystems/f2fs.rst > > @@ -247,9 +247,9 @@ checkpoint=%s[:%u[%]] Set to "disable" to turn off checkpointing. Set to "enabl > > hide up to all remaining free space. The actual space that > > would be unusable can be viewed at /sys/fs/f2fs/<disk>/unusable > > This space is reclaimed once checkpoint=enable. > > - Here is another option "merge", which creates a kernel daemon > > - and makes it to merge concurrent checkpoint requests as much > > - as possible to eliminate redundant checkpoint issues. Plus, > > +checkpoint_merge When checkpoint is enabled, this can be used to create a kernel > > + daemon and make it to merge concurrent checkpoint requests as > > + much as possible to eliminate redundant checkpoint issues. Plus, > > we can eliminate the sluggish issue caused by slow checkpoint > > operation when the checkpoint is done in a process context in > > a cgroup having low i/o budget and cpu shares. To make this > > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > > index 56696f6cfa86..b60dcef7f9d0 100644 > > --- a/fs/f2fs/super.c > > +++ b/fs/f2fs/super.c > > @@ -215,7 +215,7 @@ static match_table_t f2fs_tokens = { > > {Opt_checkpoint_disable_cap, "checkpoint=disable:%u"}, > > {Opt_checkpoint_disable_cap_perc, "checkpoint=disable:%u%%"}, > > {Opt_checkpoint_enable, "checkpoint=enable"}, > > - {Opt_checkpoint_merge, "checkpoint=merge"}, > > + {Opt_checkpoint_merge, "checkpoint_merge"}, > > {Opt_compress_algorithm, "compress_algorithm=%s"}, > > {Opt_compress_log_size, "compress_log_size=%u"}, > > {Opt_compress_extension, "compress_extension=%s"}, > > @@ -1142,12 +1142,6 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) > > return -EINVAL; > > } > > > > - if (test_opt(sbi, DISABLE_CHECKPOINT) && > > - test_opt(sbi, MERGE_CHECKPOINT)) { > > - f2fs_err(sbi, "checkpoint=merge cannot be used with checkpoint=disable\n"); > > - return -EINVAL; > > - } > > - > > /* Not pass down write hints if the number of active logs is lesser > > * than NR_CURSEG_PERSIST_TYPE. > > */ > > @@ -1782,7 +1776,7 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root) > > seq_printf(seq, ",checkpoint=disable:%u", > > F2FS_OPTION(sbi).unusable_cap); > > if (test_opt(sbi, MERGE_CHECKPOINT)) > > - seq_puts(seq, ",checkpoint=merge"); > > + seq_puts(seq, ",checkpoint_merge"); > > if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_POSIX) > > seq_printf(seq, ",fsync_mode=%s", "posix"); > > else if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_STRICT) > > @@ -1827,6 +1821,7 @@ static void default_options(struct f2fs_sb_info *sbi) > > sbi->sb->s_flags |= SB_LAZYTIME; > > set_opt(sbi, FLUSH_MERGE); > > set_opt(sbi, DISCARD); > > + clear_opt(sbi, MERGE_CHECKPOINT); > > It's not needed here? > > Thanks, > > > if (f2fs_sb_has_blkzoned(sbi)) > > F2FS_OPTION(sbi).fs_mode = FS_MODE_LFS; > > else > > @@ -2066,9 +2061,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) > > } > > } > > > > - if (!test_opt(sbi, MERGE_CHECKPOINT)) { > > - f2fs_stop_ckpt_thread(sbi); > > - } else { > > + if (!test_opt(sbi, DISABLE_CHECKPOINT) && > > + test_opt(sbi, MERGE_CHECKPOINT)) { > > err = f2fs_start_ckpt_thread(sbi); > > if (err) { > > f2fs_err(sbi, > > @@ -2076,6 +2070,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) > > err); > > goto restore_gc; > > } > > + } else { > > + f2fs_stop_ckpt_thread(sbi); > > } > > > > /* > > @@ -3831,7 +3827,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent) > > > > /* setup checkpoint request control and start checkpoint issue thread */ > > f2fs_init_ckpt_req_control(sbi); > > - if (test_opt(sbi, MERGE_CHECKPOINT)) { > > + if (!test_opt(sbi, DISABLE_CHECKPOINT) && > > + test_opt(sbi, MERGE_CHECKPOINT)) { > > err = f2fs_start_ckpt_thread(sbi); > > if (err) { > > f2fs_err(sbi, > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [f2fs-dev] [PATCH v3] f2fs: rename checkpoint=merge mount option to checkpoint_merge @ 2021-02-02 9:44 ` Daeho Jeong 0 siblings, 0 replies; 12+ messages in thread From: Daeho Jeong @ 2021-02-02 9:44 UTC (permalink / raw) To: Chao Yu; +Cc: Daeho Jeong, kernel-team, linux-kernel, linux-f2fs-devel When we remount it without the "checkpoint_merge" option, shouldn't we need to clear "MERGE_CHECKPOINT" again? This is actually what I intended, but I was wrong. Actually, I found this. When we remount the filesystem, the previous mount option is passed through the "data" argument in the below. f2fs_remount(struct super_block *sb, int *flags, char *data) If we don't provide the "nocheckpoint_merge" option, how can we turn off the "checkpoint_merge" option which is turned on in the previous mount? 2021년 2월 2일 (화) 오후 6:28, Chao Yu <yuchao0@huawei.com>님이 작성: > > On 2021/2/2 17:23, Daeho Jeong wrote: > > From: Daeho Jeong <daehojeong@google.com> > > > > As checkpoint=merge comes in, mount option setting related to checkpoint > > had been mixed up and it became hard to understand. So, I separated > > this option from "checkpoint=" and made another mount option > > "checkpoint_merge" for this. > > > > Signed-off-by: Daeho Jeong <daehojeong@google.com> > > --- > > v2: renamed "checkpoint=merge" to "checkpoint_merge" > > v3: removed "nocheckpoint_merge" option > > --- > > Documentation/filesystems/f2fs.rst | 6 +++--- > > fs/f2fs/super.c | 21 +++++++++------------ > > 2 files changed, 12 insertions(+), 15 deletions(-) > > > > diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst > > index d0ead45dc706..475994ed8b15 100644 > > --- a/Documentation/filesystems/f2fs.rst > > +++ b/Documentation/filesystems/f2fs.rst > > @@ -247,9 +247,9 @@ checkpoint=%s[:%u[%]] Set to "disable" to turn off checkpointing. Set to "enabl > > hide up to all remaining free space. The actual space that > > would be unusable can be viewed at /sys/fs/f2fs/<disk>/unusable > > This space is reclaimed once checkpoint=enable. > > - Here is another option "merge", which creates a kernel daemon > > - and makes it to merge concurrent checkpoint requests as much > > - as possible to eliminate redundant checkpoint issues. Plus, > > +checkpoint_merge When checkpoint is enabled, this can be used to create a kernel > > + daemon and make it to merge concurrent checkpoint requests as > > + much as possible to eliminate redundant checkpoint issues. Plus, > > we can eliminate the sluggish issue caused by slow checkpoint > > operation when the checkpoint is done in a process context in > > a cgroup having low i/o budget and cpu shares. To make this > > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > > index 56696f6cfa86..b60dcef7f9d0 100644 > > --- a/fs/f2fs/super.c > > +++ b/fs/f2fs/super.c > > @@ -215,7 +215,7 @@ static match_table_t f2fs_tokens = { > > {Opt_checkpoint_disable_cap, "checkpoint=disable:%u"}, > > {Opt_checkpoint_disable_cap_perc, "checkpoint=disable:%u%%"}, > > {Opt_checkpoint_enable, "checkpoint=enable"}, > > - {Opt_checkpoint_merge, "checkpoint=merge"}, > > + {Opt_checkpoint_merge, "checkpoint_merge"}, > > {Opt_compress_algorithm, "compress_algorithm=%s"}, > > {Opt_compress_log_size, "compress_log_size=%u"}, > > {Opt_compress_extension, "compress_extension=%s"}, > > @@ -1142,12 +1142,6 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) > > return -EINVAL; > > } > > > > - if (test_opt(sbi, DISABLE_CHECKPOINT) && > > - test_opt(sbi, MERGE_CHECKPOINT)) { > > - f2fs_err(sbi, "checkpoint=merge cannot be used with checkpoint=disable\n"); > > - return -EINVAL; > > - } > > - > > /* Not pass down write hints if the number of active logs is lesser > > * than NR_CURSEG_PERSIST_TYPE. > > */ > > @@ -1782,7 +1776,7 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root) > > seq_printf(seq, ",checkpoint=disable:%u", > > F2FS_OPTION(sbi).unusable_cap); > > if (test_opt(sbi, MERGE_CHECKPOINT)) > > - seq_puts(seq, ",checkpoint=merge"); > > + seq_puts(seq, ",checkpoint_merge"); > > if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_POSIX) > > seq_printf(seq, ",fsync_mode=%s", "posix"); > > else if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_STRICT) > > @@ -1827,6 +1821,7 @@ static void default_options(struct f2fs_sb_info *sbi) > > sbi->sb->s_flags |= SB_LAZYTIME; > > set_opt(sbi, FLUSH_MERGE); > > set_opt(sbi, DISCARD); > > + clear_opt(sbi, MERGE_CHECKPOINT); > > It's not needed here? > > Thanks, > > > if (f2fs_sb_has_blkzoned(sbi)) > > F2FS_OPTION(sbi).fs_mode = FS_MODE_LFS; > > else > > @@ -2066,9 +2061,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) > > } > > } > > > > - if (!test_opt(sbi, MERGE_CHECKPOINT)) { > > - f2fs_stop_ckpt_thread(sbi); > > - } else { > > + if (!test_opt(sbi, DISABLE_CHECKPOINT) && > > + test_opt(sbi, MERGE_CHECKPOINT)) { > > err = f2fs_start_ckpt_thread(sbi); > > if (err) { > > f2fs_err(sbi, > > @@ -2076,6 +2070,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) > > err); > > goto restore_gc; > > } > > + } else { > > + f2fs_stop_ckpt_thread(sbi); > > } > > > > /* > > @@ -3831,7 +3827,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent) > > > > /* setup checkpoint request control and start checkpoint issue thread */ > > f2fs_init_ckpt_req_control(sbi); > > - if (test_opt(sbi, MERGE_CHECKPOINT)) { > > + if (!test_opt(sbi, DISABLE_CHECKPOINT) && > > + test_opt(sbi, MERGE_CHECKPOINT)) { > > err = f2fs_start_ckpt_thread(sbi); > > if (err) { > > f2fs_err(sbi, > > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [f2fs-dev] [PATCH v3] f2fs: rename checkpoint=merge mount option to checkpoint_merge 2021-02-02 9:44 ` Daeho Jeong @ 2021-02-02 9:54 ` Chao Yu -1 siblings, 0 replies; 12+ messages in thread From: Chao Yu @ 2021-02-02 9:54 UTC (permalink / raw) To: Daeho Jeong; +Cc: linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong On 2021/2/2 17:44, Daeho Jeong wrote: > When we remount it without the "checkpoint_merge" option, shouldn't we > need to clear "MERGE_CHECKPOINT" again? > This is actually what I intended, but I was wrong. Actually, I found this. > > When we remount the filesystem, the previous mount option is passed > through the "data" argument in the below. > f2fs_remount(struct super_block *sb, int *flags, char *data) > > If we don't provide the "nocheckpoint_merge" option, how can we turn > off the "checkpoint_merge" option which is turned on in the previous > mount? We can use "mount -o remount /dev/xxx /mnt" to disable checkpoint_merge, since that command won't pass old mount options to remount? Quoted from man mount: mount -o remount,rw /dev/foo /dir After this call all old mount options are replaced and arbitrary stuff from fstab (or mtab) is ignored, except the loop= option which is internally generated and maintained by the mount command. mount -o remount,rw /dir After this call mount reads fstab and merges these options with the options from the command line (-o). If no mountpoint found in fstab than remount with unspecified source is allowed. Thanks, > > 2021년 2월 2일 (화) 오후 6:28, Chao Yu <yuchao0@huawei.com>님이 작성: >> >> On 2021/2/2 17:23, Daeho Jeong wrote: >>> From: Daeho Jeong <daehojeong@google.com> >>> >>> As checkpoint=merge comes in, mount option setting related to checkpoint >>> had been mixed up and it became hard to understand. So, I separated >>> this option from "checkpoint=" and made another mount option >>> "checkpoint_merge" for this. >>> >>> Signed-off-by: Daeho Jeong <daehojeong@google.com> >>> --- >>> v2: renamed "checkpoint=merge" to "checkpoint_merge" >>> v3: removed "nocheckpoint_merge" option >>> --- >>> Documentation/filesystems/f2fs.rst | 6 +++--- >>> fs/f2fs/super.c | 21 +++++++++------------ >>> 2 files changed, 12 insertions(+), 15 deletions(-) >>> >>> diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst >>> index d0ead45dc706..475994ed8b15 100644 >>> --- a/Documentation/filesystems/f2fs.rst >>> +++ b/Documentation/filesystems/f2fs.rst >>> @@ -247,9 +247,9 @@ checkpoint=%s[:%u[%]] Set to "disable" to turn off checkpointing. Set to "enabl >>> hide up to all remaining free space. The actual space that >>> would be unusable can be viewed at /sys/fs/f2fs/<disk>/unusable >>> This space is reclaimed once checkpoint=enable. >>> - Here is another option "merge", which creates a kernel daemon >>> - and makes it to merge concurrent checkpoint requests as much >>> - as possible to eliminate redundant checkpoint issues. Plus, >>> +checkpoint_merge When checkpoint is enabled, this can be used to create a kernel >>> + daemon and make it to merge concurrent checkpoint requests as >>> + much as possible to eliminate redundant checkpoint issues. Plus, >>> we can eliminate the sluggish issue caused by slow checkpoint >>> operation when the checkpoint is done in a process context in >>> a cgroup having low i/o budget and cpu shares. To make this >>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c >>> index 56696f6cfa86..b60dcef7f9d0 100644 >>> --- a/fs/f2fs/super.c >>> +++ b/fs/f2fs/super.c >>> @@ -215,7 +215,7 @@ static match_table_t f2fs_tokens = { >>> {Opt_checkpoint_disable_cap, "checkpoint=disable:%u"}, >>> {Opt_checkpoint_disable_cap_perc, "checkpoint=disable:%u%%"}, >>> {Opt_checkpoint_enable, "checkpoint=enable"}, >>> - {Opt_checkpoint_merge, "checkpoint=merge"}, >>> + {Opt_checkpoint_merge, "checkpoint_merge"}, >>> {Opt_compress_algorithm, "compress_algorithm=%s"}, >>> {Opt_compress_log_size, "compress_log_size=%u"}, >>> {Opt_compress_extension, "compress_extension=%s"}, >>> @@ -1142,12 +1142,6 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) >>> return -EINVAL; >>> } >>> >>> - if (test_opt(sbi, DISABLE_CHECKPOINT) && >>> - test_opt(sbi, MERGE_CHECKPOINT)) { >>> - f2fs_err(sbi, "checkpoint=merge cannot be used with checkpoint=disable\n"); >>> - return -EINVAL; >>> - } >>> - >>> /* Not pass down write hints if the number of active logs is lesser >>> * than NR_CURSEG_PERSIST_TYPE. >>> */ >>> @@ -1782,7 +1776,7 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root) >>> seq_printf(seq, ",checkpoint=disable:%u", >>> F2FS_OPTION(sbi).unusable_cap); >>> if (test_opt(sbi, MERGE_CHECKPOINT)) >>> - seq_puts(seq, ",checkpoint=merge"); >>> + seq_puts(seq, ",checkpoint_merge"); >>> if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_POSIX) >>> seq_printf(seq, ",fsync_mode=%s", "posix"); >>> else if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_STRICT) >>> @@ -1827,6 +1821,7 @@ static void default_options(struct f2fs_sb_info *sbi) >>> sbi->sb->s_flags |= SB_LAZYTIME; >>> set_opt(sbi, FLUSH_MERGE); >>> set_opt(sbi, DISCARD); >>> + clear_opt(sbi, MERGE_CHECKPOINT); >> >> It's not needed here? >> >> Thanks, >> >>> if (f2fs_sb_has_blkzoned(sbi)) >>> F2FS_OPTION(sbi).fs_mode = FS_MODE_LFS; >>> else >>> @@ -2066,9 +2061,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) >>> } >>> } >>> >>> - if (!test_opt(sbi, MERGE_CHECKPOINT)) { >>> - f2fs_stop_ckpt_thread(sbi); >>> - } else { >>> + if (!test_opt(sbi, DISABLE_CHECKPOINT) && >>> + test_opt(sbi, MERGE_CHECKPOINT)) { >>> err = f2fs_start_ckpt_thread(sbi); >>> if (err) { >>> f2fs_err(sbi, >>> @@ -2076,6 +2070,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) >>> err); >>> goto restore_gc; >>> } >>> + } else { >>> + f2fs_stop_ckpt_thread(sbi); >>> } >>> >>> /* >>> @@ -3831,7 +3827,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent) >>> >>> /* setup checkpoint request control and start checkpoint issue thread */ >>> f2fs_init_ckpt_req_control(sbi); >>> - if (test_opt(sbi, MERGE_CHECKPOINT)) { >>> + if (!test_opt(sbi, DISABLE_CHECKPOINT) && >>> + test_opt(sbi, MERGE_CHECKPOINT)) { >>> err = f2fs_start_ckpt_thread(sbi); >>> if (err) { >>> f2fs_err(sbi, >>> > . > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [f2fs-dev] [PATCH v3] f2fs: rename checkpoint=merge mount option to checkpoint_merge @ 2021-02-02 9:54 ` Chao Yu 0 siblings, 0 replies; 12+ messages in thread From: Chao Yu @ 2021-02-02 9:54 UTC (permalink / raw) To: Daeho Jeong; +Cc: Daeho Jeong, kernel-team, linux-kernel, linux-f2fs-devel On 2021/2/2 17:44, Daeho Jeong wrote: > When we remount it without the "checkpoint_merge" option, shouldn't we > need to clear "MERGE_CHECKPOINT" again? > This is actually what I intended, but I was wrong. Actually, I found this. > > When we remount the filesystem, the previous mount option is passed > through the "data" argument in the below. > f2fs_remount(struct super_block *sb, int *flags, char *data) > > If we don't provide the "nocheckpoint_merge" option, how can we turn > off the "checkpoint_merge" option which is turned on in the previous > mount? We can use "mount -o remount /dev/xxx /mnt" to disable checkpoint_merge, since that command won't pass old mount options to remount? Quoted from man mount: mount -o remount,rw /dev/foo /dir After this call all old mount options are replaced and arbitrary stuff from fstab (or mtab) is ignored, except the loop= option which is internally generated and maintained by the mount command. mount -o remount,rw /dir After this call mount reads fstab and merges these options with the options from the command line (-o). If no mountpoint found in fstab than remount with unspecified source is allowed. Thanks, > > 2021년 2월 2일 (화) 오후 6:28, Chao Yu <yuchao0@huawei.com>님이 작성: >> >> On 2021/2/2 17:23, Daeho Jeong wrote: >>> From: Daeho Jeong <daehojeong@google.com> >>> >>> As checkpoint=merge comes in, mount option setting related to checkpoint >>> had been mixed up and it became hard to understand. So, I separated >>> this option from "checkpoint=" and made another mount option >>> "checkpoint_merge" for this. >>> >>> Signed-off-by: Daeho Jeong <daehojeong@google.com> >>> --- >>> v2: renamed "checkpoint=merge" to "checkpoint_merge" >>> v3: removed "nocheckpoint_merge" option >>> --- >>> Documentation/filesystems/f2fs.rst | 6 +++--- >>> fs/f2fs/super.c | 21 +++++++++------------ >>> 2 files changed, 12 insertions(+), 15 deletions(-) >>> >>> diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst >>> index d0ead45dc706..475994ed8b15 100644 >>> --- a/Documentation/filesystems/f2fs.rst >>> +++ b/Documentation/filesystems/f2fs.rst >>> @@ -247,9 +247,9 @@ checkpoint=%s[:%u[%]] Set to "disable" to turn off checkpointing. Set to "enabl >>> hide up to all remaining free space. The actual space that >>> would be unusable can be viewed at /sys/fs/f2fs/<disk>/unusable >>> This space is reclaimed once checkpoint=enable. >>> - Here is another option "merge", which creates a kernel daemon >>> - and makes it to merge concurrent checkpoint requests as much >>> - as possible to eliminate redundant checkpoint issues. Plus, >>> +checkpoint_merge When checkpoint is enabled, this can be used to create a kernel >>> + daemon and make it to merge concurrent checkpoint requests as >>> + much as possible to eliminate redundant checkpoint issues. Plus, >>> we can eliminate the sluggish issue caused by slow checkpoint >>> operation when the checkpoint is done in a process context in >>> a cgroup having low i/o budget and cpu shares. To make this >>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c >>> index 56696f6cfa86..b60dcef7f9d0 100644 >>> --- a/fs/f2fs/super.c >>> +++ b/fs/f2fs/super.c >>> @@ -215,7 +215,7 @@ static match_table_t f2fs_tokens = { >>> {Opt_checkpoint_disable_cap, "checkpoint=disable:%u"}, >>> {Opt_checkpoint_disable_cap_perc, "checkpoint=disable:%u%%"}, >>> {Opt_checkpoint_enable, "checkpoint=enable"}, >>> - {Opt_checkpoint_merge, "checkpoint=merge"}, >>> + {Opt_checkpoint_merge, "checkpoint_merge"}, >>> {Opt_compress_algorithm, "compress_algorithm=%s"}, >>> {Opt_compress_log_size, "compress_log_size=%u"}, >>> {Opt_compress_extension, "compress_extension=%s"}, >>> @@ -1142,12 +1142,6 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) >>> return -EINVAL; >>> } >>> >>> - if (test_opt(sbi, DISABLE_CHECKPOINT) && >>> - test_opt(sbi, MERGE_CHECKPOINT)) { >>> - f2fs_err(sbi, "checkpoint=merge cannot be used with checkpoint=disable\n"); >>> - return -EINVAL; >>> - } >>> - >>> /* Not pass down write hints if the number of active logs is lesser >>> * than NR_CURSEG_PERSIST_TYPE. >>> */ >>> @@ -1782,7 +1776,7 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root) >>> seq_printf(seq, ",checkpoint=disable:%u", >>> F2FS_OPTION(sbi).unusable_cap); >>> if (test_opt(sbi, MERGE_CHECKPOINT)) >>> - seq_puts(seq, ",checkpoint=merge"); >>> + seq_puts(seq, ",checkpoint_merge"); >>> if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_POSIX) >>> seq_printf(seq, ",fsync_mode=%s", "posix"); >>> else if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_STRICT) >>> @@ -1827,6 +1821,7 @@ static void default_options(struct f2fs_sb_info *sbi) >>> sbi->sb->s_flags |= SB_LAZYTIME; >>> set_opt(sbi, FLUSH_MERGE); >>> set_opt(sbi, DISCARD); >>> + clear_opt(sbi, MERGE_CHECKPOINT); >> >> It's not needed here? >> >> Thanks, >> >>> if (f2fs_sb_has_blkzoned(sbi)) >>> F2FS_OPTION(sbi).fs_mode = FS_MODE_LFS; >>> else >>> @@ -2066,9 +2061,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) >>> } >>> } >>> >>> - if (!test_opt(sbi, MERGE_CHECKPOINT)) { >>> - f2fs_stop_ckpt_thread(sbi); >>> - } else { >>> + if (!test_opt(sbi, DISABLE_CHECKPOINT) && >>> + test_opt(sbi, MERGE_CHECKPOINT)) { >>> err = f2fs_start_ckpt_thread(sbi); >>> if (err) { >>> f2fs_err(sbi, >>> @@ -2076,6 +2070,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) >>> err); >>> goto restore_gc; >>> } >>> + } else { >>> + f2fs_stop_ckpt_thread(sbi); >>> } >>> >>> /* >>> @@ -3831,7 +3827,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent) >>> >>> /* setup checkpoint request control and start checkpoint issue thread */ >>> f2fs_init_ckpt_req_control(sbi); >>> - if (test_opt(sbi, MERGE_CHECKPOINT)) { >>> + if (!test_opt(sbi, DISABLE_CHECKPOINT) && >>> + test_opt(sbi, MERGE_CHECKPOINT)) { >>> err = f2fs_start_ckpt_thread(sbi); >>> if (err) { >>> f2fs_err(sbi, >>> > . > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [f2fs-dev] [PATCH v3] f2fs: rename checkpoint=merge mount option to checkpoint_merge 2021-02-02 9:54 ` Chao Yu @ 2021-02-02 11:29 ` Daeho Jeong -1 siblings, 0 replies; 12+ messages in thread From: Daeho Jeong @ 2021-02-02 11:29 UTC (permalink / raw) To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong Thanks for the explanation. I am going to remove the line clearing "MERGE_CHECKPOINT". But, when we go with the below remount command, I think the "nocheckpoint_merge" option will work well to disable only just "checkpoint_merge" from the previous option. "mount -o remount,nocheckpoint_merge /dir" It would be more convenient to users. What do you think? 2021년 2월 2일 (화) 오후 6:55, Chao Yu <yuchao0@huawei.com>님이 작성: > > On 2021/2/2 17:44, Daeho Jeong wrote: > > When we remount it without the "checkpoint_merge" option, shouldn't we > > need to clear "MERGE_CHECKPOINT" again? > > This is actually what I intended, but I was wrong. Actually, I found this. > > > > When we remount the filesystem, the previous mount option is passed > > through the "data" argument in the below. > > f2fs_remount(struct super_block *sb, int *flags, char *data) > > > > If we don't provide the "nocheckpoint_merge" option, how can we turn > > off the "checkpoint_merge" option which is turned on in the previous > > mount? > > We can use "mount -o remount /dev/xxx /mnt" to disable checkpoint_merge, > since that command won't pass old mount options to remount? > > Quoted from man mount: > > mount -o remount,rw /dev/foo /dir > > After this call all old mount options are replaced and arbitrary stuff from fstab (or mtab) is ignored, except the loop= option which is internally generated and maintained by the > mount command. > > mount -o remount,rw /dir > > After this call mount reads fstab and merges these options with the options from the command line (-o). If no mountpoint found in fstab than remount with unspecified source is allowed. > > Thanks, > > > > > 2021년 2월 2일 (화) 오후 6:28, Chao Yu <yuchao0@huawei.com>님이 작성: > >> > >> On 2021/2/2 17:23, Daeho Jeong wrote: > >>> From: Daeho Jeong <daehojeong@google.com> > >>> > >>> As checkpoint=merge comes in, mount option setting related to checkpoint > >>> had been mixed up and it became hard to understand. So, I separated > >>> this option from "checkpoint=" and made another mount option > >>> "checkpoint_merge" for this. > >>> > >>> Signed-off-by: Daeho Jeong <daehojeong@google.com> > >>> --- > >>> v2: renamed "checkpoint=merge" to "checkpoint_merge" > >>> v3: removed "nocheckpoint_merge" option > >>> --- > >>> Documentation/filesystems/f2fs.rst | 6 +++--- > >>> fs/f2fs/super.c | 21 +++++++++------------ > >>> 2 files changed, 12 insertions(+), 15 deletions(-) > >>> > >>> diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst > >>> index d0ead45dc706..475994ed8b15 100644 > >>> --- a/Documentation/filesystems/f2fs.rst > >>> +++ b/Documentation/filesystems/f2fs.rst > >>> @@ -247,9 +247,9 @@ checkpoint=%s[:%u[%]] Set to "disable" to turn off checkpointing. Set to "enabl > >>> hide up to all remaining free space. The actual space that > >>> would be unusable can be viewed at /sys/fs/f2fs/<disk>/unusable > >>> This space is reclaimed once checkpoint=enable. > >>> - Here is another option "merge", which creates a kernel daemon > >>> - and makes it to merge concurrent checkpoint requests as much > >>> - as possible to eliminate redundant checkpoint issues. Plus, > >>> +checkpoint_merge When checkpoint is enabled, this can be used to create a kernel > >>> + daemon and make it to merge concurrent checkpoint requests as > >>> + much as possible to eliminate redundant checkpoint issues. Plus, > >>> we can eliminate the sluggish issue caused by slow checkpoint > >>> operation when the checkpoint is done in a process context in > >>> a cgroup having low i/o budget and cpu shares. To make this > >>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > >>> index 56696f6cfa86..b60dcef7f9d0 100644 > >>> --- a/fs/f2fs/super.c > >>> +++ b/fs/f2fs/super.c > >>> @@ -215,7 +215,7 @@ static match_table_t f2fs_tokens = { > >>> {Opt_checkpoint_disable_cap, "checkpoint=disable:%u"}, > >>> {Opt_checkpoint_disable_cap_perc, "checkpoint=disable:%u%%"}, > >>> {Opt_checkpoint_enable, "checkpoint=enable"}, > >>> - {Opt_checkpoint_merge, "checkpoint=merge"}, > >>> + {Opt_checkpoint_merge, "checkpoint_merge"}, > >>> {Opt_compress_algorithm, "compress_algorithm=%s"}, > >>> {Opt_compress_log_size, "compress_log_size=%u"}, > >>> {Opt_compress_extension, "compress_extension=%s"}, > >>> @@ -1142,12 +1142,6 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) > >>> return -EINVAL; > >>> } > >>> > >>> - if (test_opt(sbi, DISABLE_CHECKPOINT) && > >>> - test_opt(sbi, MERGE_CHECKPOINT)) { > >>> - f2fs_err(sbi, "checkpoint=merge cannot be used with checkpoint=disable\n"); > >>> - return -EINVAL; > >>> - } > >>> - > >>> /* Not pass down write hints if the number of active logs is lesser > >>> * than NR_CURSEG_PERSIST_TYPE. > >>> */ > >>> @@ -1782,7 +1776,7 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root) > >>> seq_printf(seq, ",checkpoint=disable:%u", > >>> F2FS_OPTION(sbi).unusable_cap); > >>> if (test_opt(sbi, MERGE_CHECKPOINT)) > >>> - seq_puts(seq, ",checkpoint=merge"); > >>> + seq_puts(seq, ",checkpoint_merge"); > >>> if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_POSIX) > >>> seq_printf(seq, ",fsync_mode=%s", "posix"); > >>> else if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_STRICT) > >>> @@ -1827,6 +1821,7 @@ static void default_options(struct f2fs_sb_info *sbi) > >>> sbi->sb->s_flags |= SB_LAZYTIME; > >>> set_opt(sbi, FLUSH_MERGE); > >>> set_opt(sbi, DISCARD); > >>> + clear_opt(sbi, MERGE_CHECKPOINT); > >> > >> It's not needed here? > >> > >> Thanks, > >> > >>> if (f2fs_sb_has_blkzoned(sbi)) > >>> F2FS_OPTION(sbi).fs_mode = FS_MODE_LFS; > >>> else > >>> @@ -2066,9 +2061,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) > >>> } > >>> } > >>> > >>> - if (!test_opt(sbi, MERGE_CHECKPOINT)) { > >>> - f2fs_stop_ckpt_thread(sbi); > >>> - } else { > >>> + if (!test_opt(sbi, DISABLE_CHECKPOINT) && > >>> + test_opt(sbi, MERGE_CHECKPOINT)) { > >>> err = f2fs_start_ckpt_thread(sbi); > >>> if (err) { > >>> f2fs_err(sbi, > >>> @@ -2076,6 +2070,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) > >>> err); > >>> goto restore_gc; > >>> } > >>> + } else { > >>> + f2fs_stop_ckpt_thread(sbi); > >>> } > >>> > >>> /* > >>> @@ -3831,7 +3827,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent) > >>> > >>> /* setup checkpoint request control and start checkpoint issue thread */ > >>> f2fs_init_ckpt_req_control(sbi); > >>> - if (test_opt(sbi, MERGE_CHECKPOINT)) { > >>> + if (!test_opt(sbi, DISABLE_CHECKPOINT) && > >>> + test_opt(sbi, MERGE_CHECKPOINT)) { > >>> err = f2fs_start_ckpt_thread(sbi); > >>> if (err) { > >>> f2fs_err(sbi, > >>> > > . > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [f2fs-dev] [PATCH v3] f2fs: rename checkpoint=merge mount option to checkpoint_merge @ 2021-02-02 11:29 ` Daeho Jeong 0 siblings, 0 replies; 12+ messages in thread From: Daeho Jeong @ 2021-02-02 11:29 UTC (permalink / raw) To: Chao Yu; +Cc: Daeho Jeong, kernel-team, linux-kernel, linux-f2fs-devel Thanks for the explanation. I am going to remove the line clearing "MERGE_CHECKPOINT". But, when we go with the below remount command, I think the "nocheckpoint_merge" option will work well to disable only just "checkpoint_merge" from the previous option. "mount -o remount,nocheckpoint_merge /dir" It would be more convenient to users. What do you think? 2021년 2월 2일 (화) 오후 6:55, Chao Yu <yuchao0@huawei.com>님이 작성: > > On 2021/2/2 17:44, Daeho Jeong wrote: > > When we remount it without the "checkpoint_merge" option, shouldn't we > > need to clear "MERGE_CHECKPOINT" again? > > This is actually what I intended, but I was wrong. Actually, I found this. > > > > When we remount the filesystem, the previous mount option is passed > > through the "data" argument in the below. > > f2fs_remount(struct super_block *sb, int *flags, char *data) > > > > If we don't provide the "nocheckpoint_merge" option, how can we turn > > off the "checkpoint_merge" option which is turned on in the previous > > mount? > > We can use "mount -o remount /dev/xxx /mnt" to disable checkpoint_merge, > since that command won't pass old mount options to remount? > > Quoted from man mount: > > mount -o remount,rw /dev/foo /dir > > After this call all old mount options are replaced and arbitrary stuff from fstab (or mtab) is ignored, except the loop= option which is internally generated and maintained by the > mount command. > > mount -o remount,rw /dir > > After this call mount reads fstab and merges these options with the options from the command line (-o). If no mountpoint found in fstab than remount with unspecified source is allowed. > > Thanks, > > > > > 2021년 2월 2일 (화) 오후 6:28, Chao Yu <yuchao0@huawei.com>님이 작성: > >> > >> On 2021/2/2 17:23, Daeho Jeong wrote: > >>> From: Daeho Jeong <daehojeong@google.com> > >>> > >>> As checkpoint=merge comes in, mount option setting related to checkpoint > >>> had been mixed up and it became hard to understand. So, I separated > >>> this option from "checkpoint=" and made another mount option > >>> "checkpoint_merge" for this. > >>> > >>> Signed-off-by: Daeho Jeong <daehojeong@google.com> > >>> --- > >>> v2: renamed "checkpoint=merge" to "checkpoint_merge" > >>> v3: removed "nocheckpoint_merge" option > >>> --- > >>> Documentation/filesystems/f2fs.rst | 6 +++--- > >>> fs/f2fs/super.c | 21 +++++++++------------ > >>> 2 files changed, 12 insertions(+), 15 deletions(-) > >>> > >>> diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst > >>> index d0ead45dc706..475994ed8b15 100644 > >>> --- a/Documentation/filesystems/f2fs.rst > >>> +++ b/Documentation/filesystems/f2fs.rst > >>> @@ -247,9 +247,9 @@ checkpoint=%s[:%u[%]] Set to "disable" to turn off checkpointing. Set to "enabl > >>> hide up to all remaining free space. The actual space that > >>> would be unusable can be viewed at /sys/fs/f2fs/<disk>/unusable > >>> This space is reclaimed once checkpoint=enable. > >>> - Here is another option "merge", which creates a kernel daemon > >>> - and makes it to merge concurrent checkpoint requests as much > >>> - as possible to eliminate redundant checkpoint issues. Plus, > >>> +checkpoint_merge When checkpoint is enabled, this can be used to create a kernel > >>> + daemon and make it to merge concurrent checkpoint requests as > >>> + much as possible to eliminate redundant checkpoint issues. Plus, > >>> we can eliminate the sluggish issue caused by slow checkpoint > >>> operation when the checkpoint is done in a process context in > >>> a cgroup having low i/o budget and cpu shares. To make this > >>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > >>> index 56696f6cfa86..b60dcef7f9d0 100644 > >>> --- a/fs/f2fs/super.c > >>> +++ b/fs/f2fs/super.c > >>> @@ -215,7 +215,7 @@ static match_table_t f2fs_tokens = { > >>> {Opt_checkpoint_disable_cap, "checkpoint=disable:%u"}, > >>> {Opt_checkpoint_disable_cap_perc, "checkpoint=disable:%u%%"}, > >>> {Opt_checkpoint_enable, "checkpoint=enable"}, > >>> - {Opt_checkpoint_merge, "checkpoint=merge"}, > >>> + {Opt_checkpoint_merge, "checkpoint_merge"}, > >>> {Opt_compress_algorithm, "compress_algorithm=%s"}, > >>> {Opt_compress_log_size, "compress_log_size=%u"}, > >>> {Opt_compress_extension, "compress_extension=%s"}, > >>> @@ -1142,12 +1142,6 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) > >>> return -EINVAL; > >>> } > >>> > >>> - if (test_opt(sbi, DISABLE_CHECKPOINT) && > >>> - test_opt(sbi, MERGE_CHECKPOINT)) { > >>> - f2fs_err(sbi, "checkpoint=merge cannot be used with checkpoint=disable\n"); > >>> - return -EINVAL; > >>> - } > >>> - > >>> /* Not pass down write hints if the number of active logs is lesser > >>> * than NR_CURSEG_PERSIST_TYPE. > >>> */ > >>> @@ -1782,7 +1776,7 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root) > >>> seq_printf(seq, ",checkpoint=disable:%u", > >>> F2FS_OPTION(sbi).unusable_cap); > >>> if (test_opt(sbi, MERGE_CHECKPOINT)) > >>> - seq_puts(seq, ",checkpoint=merge"); > >>> + seq_puts(seq, ",checkpoint_merge"); > >>> if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_POSIX) > >>> seq_printf(seq, ",fsync_mode=%s", "posix"); > >>> else if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_STRICT) > >>> @@ -1827,6 +1821,7 @@ static void default_options(struct f2fs_sb_info *sbi) > >>> sbi->sb->s_flags |= SB_LAZYTIME; > >>> set_opt(sbi, FLUSH_MERGE); > >>> set_opt(sbi, DISCARD); > >>> + clear_opt(sbi, MERGE_CHECKPOINT); > >> > >> It's not needed here? > >> > >> Thanks, > >> > >>> if (f2fs_sb_has_blkzoned(sbi)) > >>> F2FS_OPTION(sbi).fs_mode = FS_MODE_LFS; > >>> else > >>> @@ -2066,9 +2061,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) > >>> } > >>> } > >>> > >>> - if (!test_opt(sbi, MERGE_CHECKPOINT)) { > >>> - f2fs_stop_ckpt_thread(sbi); > >>> - } else { > >>> + if (!test_opt(sbi, DISABLE_CHECKPOINT) && > >>> + test_opt(sbi, MERGE_CHECKPOINT)) { > >>> err = f2fs_start_ckpt_thread(sbi); > >>> if (err) { > >>> f2fs_err(sbi, > >>> @@ -2076,6 +2070,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) > >>> err); > >>> goto restore_gc; > >>> } > >>> + } else { > >>> + f2fs_stop_ckpt_thread(sbi); > >>> } > >>> > >>> /* > >>> @@ -3831,7 +3827,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent) > >>> > >>> /* setup checkpoint request control and start checkpoint issue thread */ > >>> f2fs_init_ckpt_req_control(sbi); > >>> - if (test_opt(sbi, MERGE_CHECKPOINT)) { > >>> + if (!test_opt(sbi, DISABLE_CHECKPOINT) && > >>> + test_opt(sbi, MERGE_CHECKPOINT)) { > >>> err = f2fs_start_ckpt_thread(sbi); > >>> if (err) { > >>> f2fs_err(sbi, > >>> > > . > > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [f2fs-dev] [PATCH v3] f2fs: rename checkpoint=merge mount option to checkpoint_merge 2021-02-02 11:29 ` Daeho Jeong @ 2021-02-02 12:09 ` Chao Yu -1 siblings, 0 replies; 12+ messages in thread From: Chao Yu @ 2021-02-02 12:09 UTC (permalink / raw) To: Daeho Jeong, Chao Yu Cc: Daeho Jeong, kernel-team, linux-kernel, linux-f2fs-devel On 2021/2/2 19:29, Daeho Jeong wrote: > Thanks for the explanation. > > I am going to remove the line clearing "MERGE_CHECKPOINT". > But, when we go with the below remount command, I think the > "nocheckpoint_merge" option will work well to disable only just > "checkpoint_merge" from the previous option. > "mount -o remount,nocheckpoint_merge /dir" > > It would be more convenient to users. What do you think? It's fine to me, please go ahead. :) Thanks, > > 2021년 2월 2일 (화) 오후 6:55, Chao Yu <yuchao0@huawei.com>님이 작성: >> >> On 2021/2/2 17:44, Daeho Jeong wrote: >>> When we remount it without the "checkpoint_merge" option, shouldn't we >>> need to clear "MERGE_CHECKPOINT" again? >>> This is actually what I intended, but I was wrong. Actually, I found this. >>> >>> When we remount the filesystem, the previous mount option is passed >>> through the "data" argument in the below. >>> f2fs_remount(struct super_block *sb, int *flags, char *data) >>> >>> If we don't provide the "nocheckpoint_merge" option, how can we turn >>> off the "checkpoint_merge" option which is turned on in the previous >>> mount? >> >> We can use "mount -o remount /dev/xxx /mnt" to disable checkpoint_merge, >> since that command won't pass old mount options to remount? >> >> Quoted from man mount: >> >> mount -o remount,rw /dev/foo /dir >> >> After this call all old mount options are replaced and arbitrary stuff from fstab (or mtab) is ignored, except the loop= option which is internally generated and maintained by the >> mount command. >> >> mount -o remount,rw /dir >> >> After this call mount reads fstab and merges these options with the options from the command line (-o). If no mountpoint found in fstab than remount with unspecified source is allowed. >> >> Thanks, >> >>> >>> 2021년 2월 2일 (화) 오후 6:28, Chao Yu <yuchao0@huawei.com>님이 작성: >>>> >>>> On 2021/2/2 17:23, Daeho Jeong wrote: >>>>> From: Daeho Jeong <daehojeong@google.com> >>>>> >>>>> As checkpoint=merge comes in, mount option setting related to checkpoint >>>>> had been mixed up and it became hard to understand. So, I separated >>>>> this option from "checkpoint=" and made another mount option >>>>> "checkpoint_merge" for this. >>>>> >>>>> Signed-off-by: Daeho Jeong <daehojeong@google.com> >>>>> --- >>>>> v2: renamed "checkpoint=merge" to "checkpoint_merge" >>>>> v3: removed "nocheckpoint_merge" option >>>>> --- >>>>> Documentation/filesystems/f2fs.rst | 6 +++--- >>>>> fs/f2fs/super.c | 21 +++++++++------------ >>>>> 2 files changed, 12 insertions(+), 15 deletions(-) >>>>> >>>>> diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst >>>>> index d0ead45dc706..475994ed8b15 100644 >>>>> --- a/Documentation/filesystems/f2fs.rst >>>>> +++ b/Documentation/filesystems/f2fs.rst >>>>> @@ -247,9 +247,9 @@ checkpoint=%s[:%u[%]] Set to "disable" to turn off checkpointing. Set to "enabl >>>>> hide up to all remaining free space. The actual space that >>>>> would be unusable can be viewed at /sys/fs/f2fs/<disk>/unusable >>>>> This space is reclaimed once checkpoint=enable. >>>>> - Here is another option "merge", which creates a kernel daemon >>>>> - and makes it to merge concurrent checkpoint requests as much >>>>> - as possible to eliminate redundant checkpoint issues. Plus, >>>>> +checkpoint_merge When checkpoint is enabled, this can be used to create a kernel >>>>> + daemon and make it to merge concurrent checkpoint requests as >>>>> + much as possible to eliminate redundant checkpoint issues. Plus, >>>>> we can eliminate the sluggish issue caused by slow checkpoint >>>>> operation when the checkpoint is done in a process context in >>>>> a cgroup having low i/o budget and cpu shares. To make this >>>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c >>>>> index 56696f6cfa86..b60dcef7f9d0 100644 >>>>> --- a/fs/f2fs/super.c >>>>> +++ b/fs/f2fs/super.c >>>>> @@ -215,7 +215,7 @@ static match_table_t f2fs_tokens = { >>>>> {Opt_checkpoint_disable_cap, "checkpoint=disable:%u"}, >>>>> {Opt_checkpoint_disable_cap_perc, "checkpoint=disable:%u%%"}, >>>>> {Opt_checkpoint_enable, "checkpoint=enable"}, >>>>> - {Opt_checkpoint_merge, "checkpoint=merge"}, >>>>> + {Opt_checkpoint_merge, "checkpoint_merge"}, >>>>> {Opt_compress_algorithm, "compress_algorithm=%s"}, >>>>> {Opt_compress_log_size, "compress_log_size=%u"}, >>>>> {Opt_compress_extension, "compress_extension=%s"}, >>>>> @@ -1142,12 +1142,6 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) >>>>> return -EINVAL; >>>>> } >>>>> >>>>> - if (test_opt(sbi, DISABLE_CHECKPOINT) && >>>>> - test_opt(sbi, MERGE_CHECKPOINT)) { >>>>> - f2fs_err(sbi, "checkpoint=merge cannot be used with checkpoint=disable\n"); >>>>> - return -EINVAL; >>>>> - } >>>>> - >>>>> /* Not pass down write hints if the number of active logs is lesser >>>>> * than NR_CURSEG_PERSIST_TYPE. >>>>> */ >>>>> @@ -1782,7 +1776,7 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root) >>>>> seq_printf(seq, ",checkpoint=disable:%u", >>>>> F2FS_OPTION(sbi).unusable_cap); >>>>> if (test_opt(sbi, MERGE_CHECKPOINT)) >>>>> - seq_puts(seq, ",checkpoint=merge"); >>>>> + seq_puts(seq, ",checkpoint_merge"); >>>>> if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_POSIX) >>>>> seq_printf(seq, ",fsync_mode=%s", "posix"); >>>>> else if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_STRICT) >>>>> @@ -1827,6 +1821,7 @@ static void default_options(struct f2fs_sb_info *sbi) >>>>> sbi->sb->s_flags |= SB_LAZYTIME; >>>>> set_opt(sbi, FLUSH_MERGE); >>>>> set_opt(sbi, DISCARD); >>>>> + clear_opt(sbi, MERGE_CHECKPOINT); >>>> >>>> It's not needed here? >>>> >>>> Thanks, >>>> >>>>> if (f2fs_sb_has_blkzoned(sbi)) >>>>> F2FS_OPTION(sbi).fs_mode = FS_MODE_LFS; >>>>> else >>>>> @@ -2066,9 +2061,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) >>>>> } >>>>> } >>>>> >>>>> - if (!test_opt(sbi, MERGE_CHECKPOINT)) { >>>>> - f2fs_stop_ckpt_thread(sbi); >>>>> - } else { >>>>> + if (!test_opt(sbi, DISABLE_CHECKPOINT) && >>>>> + test_opt(sbi, MERGE_CHECKPOINT)) { >>>>> err = f2fs_start_ckpt_thread(sbi); >>>>> if (err) { >>>>> f2fs_err(sbi, >>>>> @@ -2076,6 +2070,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) >>>>> err); >>>>> goto restore_gc; >>>>> } >>>>> + } else { >>>>> + f2fs_stop_ckpt_thread(sbi); >>>>> } >>>>> >>>>> /* >>>>> @@ -3831,7 +3827,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent) >>>>> >>>>> /* setup checkpoint request control and start checkpoint issue thread */ >>>>> f2fs_init_ckpt_req_control(sbi); >>>>> - if (test_opt(sbi, MERGE_CHECKPOINT)) { >>>>> + if (!test_opt(sbi, DISABLE_CHECKPOINT) && >>>>> + test_opt(sbi, MERGE_CHECKPOINT)) { >>>>> err = f2fs_start_ckpt_thread(sbi); >>>>> if (err) { >>>>> f2fs_err(sbi, >>>>> >>> . >>> > > > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [f2fs-dev] [PATCH v3] f2fs: rename checkpoint=merge mount option to checkpoint_merge @ 2021-02-02 12:09 ` Chao Yu 0 siblings, 0 replies; 12+ messages in thread From: Chao Yu @ 2021-02-02 12:09 UTC (permalink / raw) To: Daeho Jeong, Chao Yu Cc: linux-f2fs-devel, kernel-team, Daeho Jeong, linux-kernel On 2021/2/2 19:29, Daeho Jeong wrote: > Thanks for the explanation. > > I am going to remove the line clearing "MERGE_CHECKPOINT". > But, when we go with the below remount command, I think the > "nocheckpoint_merge" option will work well to disable only just > "checkpoint_merge" from the previous option. > "mount -o remount,nocheckpoint_merge /dir" > > It would be more convenient to users. What do you think? It's fine to me, please go ahead. :) Thanks, > > 2021년 2월 2일 (화) 오후 6:55, Chao Yu <yuchao0@huawei.com>님이 작성: >> >> On 2021/2/2 17:44, Daeho Jeong wrote: >>> When we remount it without the "checkpoint_merge" option, shouldn't we >>> need to clear "MERGE_CHECKPOINT" again? >>> This is actually what I intended, but I was wrong. Actually, I found this. >>> >>> When we remount the filesystem, the previous mount option is passed >>> through the "data" argument in the below. >>> f2fs_remount(struct super_block *sb, int *flags, char *data) >>> >>> If we don't provide the "nocheckpoint_merge" option, how can we turn >>> off the "checkpoint_merge" option which is turned on in the previous >>> mount? >> >> We can use "mount -o remount /dev/xxx /mnt" to disable checkpoint_merge, >> since that command won't pass old mount options to remount? >> >> Quoted from man mount: >> >> mount -o remount,rw /dev/foo /dir >> >> After this call all old mount options are replaced and arbitrary stuff from fstab (or mtab) is ignored, except the loop= option which is internally generated and maintained by the >> mount command. >> >> mount -o remount,rw /dir >> >> After this call mount reads fstab and merges these options with the options from the command line (-o). If no mountpoint found in fstab than remount with unspecified source is allowed. >> >> Thanks, >> >>> >>> 2021년 2월 2일 (화) 오후 6:28, Chao Yu <yuchao0@huawei.com>님이 작성: >>>> >>>> On 2021/2/2 17:23, Daeho Jeong wrote: >>>>> From: Daeho Jeong <daehojeong@google.com> >>>>> >>>>> As checkpoint=merge comes in, mount option setting related to checkpoint >>>>> had been mixed up and it became hard to understand. So, I separated >>>>> this option from "checkpoint=" and made another mount option >>>>> "checkpoint_merge" for this. >>>>> >>>>> Signed-off-by: Daeho Jeong <daehojeong@google.com> >>>>> --- >>>>> v2: renamed "checkpoint=merge" to "checkpoint_merge" >>>>> v3: removed "nocheckpoint_merge" option >>>>> --- >>>>> Documentation/filesystems/f2fs.rst | 6 +++--- >>>>> fs/f2fs/super.c | 21 +++++++++------------ >>>>> 2 files changed, 12 insertions(+), 15 deletions(-) >>>>> >>>>> diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst >>>>> index d0ead45dc706..475994ed8b15 100644 >>>>> --- a/Documentation/filesystems/f2fs.rst >>>>> +++ b/Documentation/filesystems/f2fs.rst >>>>> @@ -247,9 +247,9 @@ checkpoint=%s[:%u[%]] Set to "disable" to turn off checkpointing. Set to "enabl >>>>> hide up to all remaining free space. The actual space that >>>>> would be unusable can be viewed at /sys/fs/f2fs/<disk>/unusable >>>>> This space is reclaimed once checkpoint=enable. >>>>> - Here is another option "merge", which creates a kernel daemon >>>>> - and makes it to merge concurrent checkpoint requests as much >>>>> - as possible to eliminate redundant checkpoint issues. Plus, >>>>> +checkpoint_merge When checkpoint is enabled, this can be used to create a kernel >>>>> + daemon and make it to merge concurrent checkpoint requests as >>>>> + much as possible to eliminate redundant checkpoint issues. Plus, >>>>> we can eliminate the sluggish issue caused by slow checkpoint >>>>> operation when the checkpoint is done in a process context in >>>>> a cgroup having low i/o budget and cpu shares. To make this >>>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c >>>>> index 56696f6cfa86..b60dcef7f9d0 100644 >>>>> --- a/fs/f2fs/super.c >>>>> +++ b/fs/f2fs/super.c >>>>> @@ -215,7 +215,7 @@ static match_table_t f2fs_tokens = { >>>>> {Opt_checkpoint_disable_cap, "checkpoint=disable:%u"}, >>>>> {Opt_checkpoint_disable_cap_perc, "checkpoint=disable:%u%%"}, >>>>> {Opt_checkpoint_enable, "checkpoint=enable"}, >>>>> - {Opt_checkpoint_merge, "checkpoint=merge"}, >>>>> + {Opt_checkpoint_merge, "checkpoint_merge"}, >>>>> {Opt_compress_algorithm, "compress_algorithm=%s"}, >>>>> {Opt_compress_log_size, "compress_log_size=%u"}, >>>>> {Opt_compress_extension, "compress_extension=%s"}, >>>>> @@ -1142,12 +1142,6 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount) >>>>> return -EINVAL; >>>>> } >>>>> >>>>> - if (test_opt(sbi, DISABLE_CHECKPOINT) && >>>>> - test_opt(sbi, MERGE_CHECKPOINT)) { >>>>> - f2fs_err(sbi, "checkpoint=merge cannot be used with checkpoint=disable\n"); >>>>> - return -EINVAL; >>>>> - } >>>>> - >>>>> /* Not pass down write hints if the number of active logs is lesser >>>>> * than NR_CURSEG_PERSIST_TYPE. >>>>> */ >>>>> @@ -1782,7 +1776,7 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root) >>>>> seq_printf(seq, ",checkpoint=disable:%u", >>>>> F2FS_OPTION(sbi).unusable_cap); >>>>> if (test_opt(sbi, MERGE_CHECKPOINT)) >>>>> - seq_puts(seq, ",checkpoint=merge"); >>>>> + seq_puts(seq, ",checkpoint_merge"); >>>>> if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_POSIX) >>>>> seq_printf(seq, ",fsync_mode=%s", "posix"); >>>>> else if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_STRICT) >>>>> @@ -1827,6 +1821,7 @@ static void default_options(struct f2fs_sb_info *sbi) >>>>> sbi->sb->s_flags |= SB_LAZYTIME; >>>>> set_opt(sbi, FLUSH_MERGE); >>>>> set_opt(sbi, DISCARD); >>>>> + clear_opt(sbi, MERGE_CHECKPOINT); >>>> >>>> It's not needed here? >>>> >>>> Thanks, >>>> >>>>> if (f2fs_sb_has_blkzoned(sbi)) >>>>> F2FS_OPTION(sbi).fs_mode = FS_MODE_LFS; >>>>> else >>>>> @@ -2066,9 +2061,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) >>>>> } >>>>> } >>>>> >>>>> - if (!test_opt(sbi, MERGE_CHECKPOINT)) { >>>>> - f2fs_stop_ckpt_thread(sbi); >>>>> - } else { >>>>> + if (!test_opt(sbi, DISABLE_CHECKPOINT) && >>>>> + test_opt(sbi, MERGE_CHECKPOINT)) { >>>>> err = f2fs_start_ckpt_thread(sbi); >>>>> if (err) { >>>>> f2fs_err(sbi, >>>>> @@ -2076,6 +2070,8 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) >>>>> err); >>>>> goto restore_gc; >>>>> } >>>>> + } else { >>>>> + f2fs_stop_ckpt_thread(sbi); >>>>> } >>>>> >>>>> /* >>>>> @@ -3831,7 +3827,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent) >>>>> >>>>> /* setup checkpoint request control and start checkpoint issue thread */ >>>>> f2fs_init_ckpt_req_control(sbi); >>>>> - if (test_opt(sbi, MERGE_CHECKPOINT)) { >>>>> + if (!test_opt(sbi, DISABLE_CHECKPOINT) && >>>>> + test_opt(sbi, MERGE_CHECKPOINT)) { >>>>> err = f2fs_start_ckpt_thread(sbi); >>>>> if (err) { >>>>> f2fs_err(sbi, >>>>> >>> . >>> > > > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-02-02 12:11 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-02-02 9:23 [PATCH v3] f2fs: rename checkpoint=merge mount option to checkpoint_merge Daeho Jeong 2021-02-02 9:23 ` [f2fs-dev] " Daeho Jeong 2021-02-02 9:28 ` Chao Yu 2021-02-02 9:28 ` Chao Yu 2021-02-02 9:44 ` Daeho Jeong 2021-02-02 9:44 ` Daeho Jeong 2021-02-02 9:54 ` Chao Yu 2021-02-02 9:54 ` Chao Yu 2021-02-02 11:29 ` Daeho Jeong 2021-02-02 11:29 ` Daeho Jeong 2021-02-02 12:09 ` Chao Yu 2021-02-02 12:09 ` Chao Yu
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.