* [PATCH 1/2] f2fs: don't start checkpoint thread in readonly mountpoint @ 2021-03-17 9:56 ` Chao Yu 0 siblings, 0 replies; 6+ messages in thread From: Chao Yu @ 2021-03-17 9:56 UTC (permalink / raw) To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu In readonly mountpoint, there should be no write IOs include checkpoint IO, so that it's not needed to create kernel checkpoint thread. Signed-off-by: Chao Yu <yuchao0@huawei.com> --- fs/f2fs/super.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index ce88db0170fa..6716af216dca 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -2091,8 +2091,10 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) } } - if (!test_opt(sbi, DISABLE_CHECKPOINT) && - test_opt(sbi, MERGE_CHECKPOINT)) { + if ((*flags & SB_RDONLY) || test_opt(sbi, DISABLE_CHECKPOINT) || + !test_opt(sbi, MERGE_CHECKPOINT)) { + f2fs_stop_ckpt_thread(sbi); + } else { err = f2fs_start_ckpt_thread(sbi); if (err) { f2fs_err(sbi, @@ -2100,8 +2102,6 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) err); goto restore_gc; } - } else { - f2fs_stop_ckpt_thread(sbi); } /* @@ -3857,7 +3857,7 @@ 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, DISABLE_CHECKPOINT) && + if (!f2fs_readonly(sb) && !test_opt(sbi, DISABLE_CHECKPOINT) && test_opt(sbi, MERGE_CHECKPOINT)) { err = f2fs_start_ckpt_thread(sbi); if (err) { -- 2.29.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [f2fs-dev] [PATCH 1/2] f2fs: don't start checkpoint thread in readonly mountpoint @ 2021-03-17 9:56 ` Chao Yu 0 siblings, 0 replies; 6+ messages in thread From: Chao Yu @ 2021-03-17 9:56 UTC (permalink / raw) To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel In readonly mountpoint, there should be no write IOs include checkpoint IO, so that it's not needed to create kernel checkpoint thread. Signed-off-by: Chao Yu <yuchao0@huawei.com> --- fs/f2fs/super.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index ce88db0170fa..6716af216dca 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -2091,8 +2091,10 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) } } - if (!test_opt(sbi, DISABLE_CHECKPOINT) && - test_opt(sbi, MERGE_CHECKPOINT)) { + if ((*flags & SB_RDONLY) || test_opt(sbi, DISABLE_CHECKPOINT) || + !test_opt(sbi, MERGE_CHECKPOINT)) { + f2fs_stop_ckpt_thread(sbi); + } else { err = f2fs_start_ckpt_thread(sbi); if (err) { f2fs_err(sbi, @@ -2100,8 +2102,6 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) err); goto restore_gc; } - } else { - f2fs_stop_ckpt_thread(sbi); } /* @@ -3857,7 +3857,7 @@ 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, DISABLE_CHECKPOINT) && + if (!f2fs_readonly(sb) && !test_opt(sbi, DISABLE_CHECKPOINT) && test_opt(sbi, MERGE_CHECKPOINT)) { err = f2fs_start_ckpt_thread(sbi); if (err) { -- 2.29.2 _______________________________________________ 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] 6+ messages in thread
* [PATCH 2/2] f2fs: fix error path of f2fs_remount() 2021-03-17 9:56 ` [f2fs-dev] " Chao Yu @ 2021-03-17 9:56 ` Chao Yu -1 siblings, 0 replies; 6+ messages in thread From: Chao Yu @ 2021-03-17 9:56 UTC (permalink / raw) To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu In error path of f2fs_remount(), it missed to restart/stop kernel thread or enable/disable checkpoint, then mount option status may not be consistent with real condition of filesystem, so let's reorder remount flow a bit as below and do recovery correctly in error path: 1) handle gc thread 2) handle ckpt thread 3) handle flush thread 4) handle checkpoint disabling Signed-off-by: Chao Yu <yuchao0@huawei.com> --- fs/f2fs/super.c | 47 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 6716af216dca..fa60f08c30bb 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -1942,8 +1942,9 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) struct f2fs_mount_info org_mount_opt; unsigned long old_sb_flags; int err; - bool need_restart_gc = false; - bool need_stop_gc = false; + 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 no_extent_cache = !test_opt(sbi, EXTENT_CACHE); bool disable_checkpoint = test_opt(sbi, DISABLE_CHECKPOINT); bool no_io_align = !F2FS_IO_ALIGNED(sbi); @@ -2081,19 +2082,10 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) clear_sbi_flag(sbi, SBI_IS_CLOSE); } - if (checkpoint_changed) { - if (test_opt(sbi, DISABLE_CHECKPOINT)) { - err = f2fs_disable_checkpoint(sbi); - if (err) - goto restore_gc; - } else { - f2fs_enable_checkpoint(sbi); - } - } - if ((*flags & SB_RDONLY) || test_opt(sbi, DISABLE_CHECKPOINT) || !test_opt(sbi, MERGE_CHECKPOINT)) { f2fs_stop_ckpt_thread(sbi); + need_restart_ckpt = true; } else { err = f2fs_start_ckpt_thread(sbi); if (err) { @@ -2102,6 +2094,7 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) err); goto restore_gc; } + need_stop_ckpt = true; } /* @@ -2111,11 +2104,24 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) if ((*flags & SB_RDONLY) || !test_opt(sbi, FLUSH_MERGE)) { clear_opt(sbi, FLUSH_MERGE); f2fs_destroy_flush_cmd_control(sbi, false); + need_restart_flush = true; } else { err = f2fs_create_flush_cmd_control(sbi); if (err) - goto restore_gc; + goto restore_ckpt; + need_stop_flush = true; } + + if (checkpoint_changed) { + if (test_opt(sbi, DISABLE_CHECKPOINT)) { + err = f2fs_disable_checkpoint(sbi); + if (err) + goto restore_flush; + } else { + f2fs_enable_checkpoint(sbi); + } + } + skip: #ifdef CONFIG_QUOTA /* Release old quota file names */ @@ -2130,6 +2136,21 @@ 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_flush: + if (need_restart_flush) { + if (f2fs_create_flush_cmd_control(sbi)) + f2fs_warn(sbi, "background flush thread has stopped"); + } else if (need_stop_flush) { + clear_opt(sbi, FLUSH_MERGE); + f2fs_destroy_flush_cmd_control(sbi, false); + } +restore_ckpt: + if (need_restart_ckpt) { + if (f2fs_start_ckpt_thread(sbi)) + f2fs_warn(sbi, "background ckpt thread has stopped"); + } else if (need_stop_ckpt) { + f2fs_stop_ckpt_thread(sbi); + } restore_gc: if (need_restart_gc) { if (f2fs_start_gc_thread(sbi)) -- 2.29.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [f2fs-dev] [PATCH 2/2] f2fs: fix error path of f2fs_remount() @ 2021-03-17 9:56 ` Chao Yu 0 siblings, 0 replies; 6+ messages in thread From: Chao Yu @ 2021-03-17 9:56 UTC (permalink / raw) To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel In error path of f2fs_remount(), it missed to restart/stop kernel thread or enable/disable checkpoint, then mount option status may not be consistent with real condition of filesystem, so let's reorder remount flow a bit as below and do recovery correctly in error path: 1) handle gc thread 2) handle ckpt thread 3) handle flush thread 4) handle checkpoint disabling Signed-off-by: Chao Yu <yuchao0@huawei.com> --- fs/f2fs/super.c | 47 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 6716af216dca..fa60f08c30bb 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -1942,8 +1942,9 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) struct f2fs_mount_info org_mount_opt; unsigned long old_sb_flags; int err; - bool need_restart_gc = false; - bool need_stop_gc = false; + 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 no_extent_cache = !test_opt(sbi, EXTENT_CACHE); bool disable_checkpoint = test_opt(sbi, DISABLE_CHECKPOINT); bool no_io_align = !F2FS_IO_ALIGNED(sbi); @@ -2081,19 +2082,10 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) clear_sbi_flag(sbi, SBI_IS_CLOSE); } - if (checkpoint_changed) { - if (test_opt(sbi, DISABLE_CHECKPOINT)) { - err = f2fs_disable_checkpoint(sbi); - if (err) - goto restore_gc; - } else { - f2fs_enable_checkpoint(sbi); - } - } - if ((*flags & SB_RDONLY) || test_opt(sbi, DISABLE_CHECKPOINT) || !test_opt(sbi, MERGE_CHECKPOINT)) { f2fs_stop_ckpt_thread(sbi); + need_restart_ckpt = true; } else { err = f2fs_start_ckpt_thread(sbi); if (err) { @@ -2102,6 +2094,7 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) err); goto restore_gc; } + need_stop_ckpt = true; } /* @@ -2111,11 +2104,24 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) if ((*flags & SB_RDONLY) || !test_opt(sbi, FLUSH_MERGE)) { clear_opt(sbi, FLUSH_MERGE); f2fs_destroy_flush_cmd_control(sbi, false); + need_restart_flush = true; } else { err = f2fs_create_flush_cmd_control(sbi); if (err) - goto restore_gc; + goto restore_ckpt; + need_stop_flush = true; } + + if (checkpoint_changed) { + if (test_opt(sbi, DISABLE_CHECKPOINT)) { + err = f2fs_disable_checkpoint(sbi); + if (err) + goto restore_flush; + } else { + f2fs_enable_checkpoint(sbi); + } + } + skip: #ifdef CONFIG_QUOTA /* Release old quota file names */ @@ -2130,6 +2136,21 @@ 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_flush: + if (need_restart_flush) { + if (f2fs_create_flush_cmd_control(sbi)) + f2fs_warn(sbi, "background flush thread has stopped"); + } else if (need_stop_flush) { + clear_opt(sbi, FLUSH_MERGE); + f2fs_destroy_flush_cmd_control(sbi, false); + } +restore_ckpt: + if (need_restart_ckpt) { + if (f2fs_start_ckpt_thread(sbi)) + f2fs_warn(sbi, "background ckpt thread has stopped"); + } else if (need_stop_ckpt) { + f2fs_stop_ckpt_thread(sbi); + } restore_gc: if (need_restart_gc) { if (f2fs_start_gc_thread(sbi)) -- 2.29.2 _______________________________________________ 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] 6+ messages in thread
* Re: [PATCH 2/2] f2fs: fix error path of f2fs_remount() 2021-03-17 9:56 ` [f2fs-dev] " Chao Yu @ 2021-03-24 2:19 ` Chao Yu -1 siblings, 0 replies; 6+ messages in thread From: Chao Yu @ 2021-03-24 2:19 UTC (permalink / raw) To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao Ping, On 2021/3/17 17:56, Chao Yu wrote: > In error path of f2fs_remount(), it missed to restart/stop kernel thread > or enable/disable checkpoint, then mount option status may not be > consistent with real condition of filesystem, so let's reorder remount > flow a bit as below and do recovery correctly in error path: > > 1) handle gc thread > 2) handle ckpt thread > 3) handle flush thread > 4) handle checkpoint disabling > > Signed-off-by: Chao Yu <yuchao0@huawei.com> > --- > fs/f2fs/super.c | 47 ++++++++++++++++++++++++++++++++++------------- > 1 file changed, 34 insertions(+), 13 deletions(-) > > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > index 6716af216dca..fa60f08c30bb 100644 > --- a/fs/f2fs/super.c > +++ b/fs/f2fs/super.c > @@ -1942,8 +1942,9 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) > struct f2fs_mount_info org_mount_opt; > unsigned long old_sb_flags; > int err; > - bool need_restart_gc = false; > - bool need_stop_gc = false; > + 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 no_extent_cache = !test_opt(sbi, EXTENT_CACHE); > bool disable_checkpoint = test_opt(sbi, DISABLE_CHECKPOINT); > bool no_io_align = !F2FS_IO_ALIGNED(sbi); > @@ -2081,19 +2082,10 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) > clear_sbi_flag(sbi, SBI_IS_CLOSE); > } > > - if (checkpoint_changed) { > - if (test_opt(sbi, DISABLE_CHECKPOINT)) { > - err = f2fs_disable_checkpoint(sbi); > - if (err) > - goto restore_gc; > - } else { > - f2fs_enable_checkpoint(sbi); > - } > - } > - > if ((*flags & SB_RDONLY) || test_opt(sbi, DISABLE_CHECKPOINT) || > !test_opt(sbi, MERGE_CHECKPOINT)) { > f2fs_stop_ckpt_thread(sbi); > + need_restart_ckpt = true; > } else { > err = f2fs_start_ckpt_thread(sbi); > if (err) { > @@ -2102,6 +2094,7 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) > err); > goto restore_gc; > } > + need_stop_ckpt = true; > } > > /* > @@ -2111,11 +2104,24 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) > if ((*flags & SB_RDONLY) || !test_opt(sbi, FLUSH_MERGE)) { > clear_opt(sbi, FLUSH_MERGE); > f2fs_destroy_flush_cmd_control(sbi, false); > + need_restart_flush = true; > } else { > err = f2fs_create_flush_cmd_control(sbi); > if (err) > - goto restore_gc; > + goto restore_ckpt; > + need_stop_flush = true; > } > + > + if (checkpoint_changed) { > + if (test_opt(sbi, DISABLE_CHECKPOINT)) { > + err = f2fs_disable_checkpoint(sbi); > + if (err) > + goto restore_flush; > + } else { > + f2fs_enable_checkpoint(sbi); > + } > + } > + > skip: > #ifdef CONFIG_QUOTA > /* Release old quota file names */ > @@ -2130,6 +2136,21 @@ 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_flush: > + if (need_restart_flush) { > + if (f2fs_create_flush_cmd_control(sbi)) > + f2fs_warn(sbi, "background flush thread has stopped"); > + } else if (need_stop_flush) { > + clear_opt(sbi, FLUSH_MERGE); > + f2fs_destroy_flush_cmd_control(sbi, false); > + } > +restore_ckpt: > + if (need_restart_ckpt) { > + if (f2fs_start_ckpt_thread(sbi)) > + f2fs_warn(sbi, "background ckpt thread has stopped"); > + } else if (need_stop_ckpt) { > + f2fs_stop_ckpt_thread(sbi); > + } > restore_gc: > if (need_restart_gc) { > if (f2fs_start_gc_thread(sbi)) > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [f2fs-dev] [PATCH 2/2] f2fs: fix error path of f2fs_remount() @ 2021-03-24 2:19 ` Chao Yu 0 siblings, 0 replies; 6+ messages in thread From: Chao Yu @ 2021-03-24 2:19 UTC (permalink / raw) To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel Ping, On 2021/3/17 17:56, Chao Yu wrote: > In error path of f2fs_remount(), it missed to restart/stop kernel thread > or enable/disable checkpoint, then mount option status may not be > consistent with real condition of filesystem, so let's reorder remount > flow a bit as below and do recovery correctly in error path: > > 1) handle gc thread > 2) handle ckpt thread > 3) handle flush thread > 4) handle checkpoint disabling > > Signed-off-by: Chao Yu <yuchao0@huawei.com> > --- > fs/f2fs/super.c | 47 ++++++++++++++++++++++++++++++++++------------- > 1 file changed, 34 insertions(+), 13 deletions(-) > > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > index 6716af216dca..fa60f08c30bb 100644 > --- a/fs/f2fs/super.c > +++ b/fs/f2fs/super.c > @@ -1942,8 +1942,9 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) > struct f2fs_mount_info org_mount_opt; > unsigned long old_sb_flags; > int err; > - bool need_restart_gc = false; > - bool need_stop_gc = false; > + 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 no_extent_cache = !test_opt(sbi, EXTENT_CACHE); > bool disable_checkpoint = test_opt(sbi, DISABLE_CHECKPOINT); > bool no_io_align = !F2FS_IO_ALIGNED(sbi); > @@ -2081,19 +2082,10 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) > clear_sbi_flag(sbi, SBI_IS_CLOSE); > } > > - if (checkpoint_changed) { > - if (test_opt(sbi, DISABLE_CHECKPOINT)) { > - err = f2fs_disable_checkpoint(sbi); > - if (err) > - goto restore_gc; > - } else { > - f2fs_enable_checkpoint(sbi); > - } > - } > - > if ((*flags & SB_RDONLY) || test_opt(sbi, DISABLE_CHECKPOINT) || > !test_opt(sbi, MERGE_CHECKPOINT)) { > f2fs_stop_ckpt_thread(sbi); > + need_restart_ckpt = true; > } else { > err = f2fs_start_ckpt_thread(sbi); > if (err) { > @@ -2102,6 +2094,7 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) > err); > goto restore_gc; > } > + need_stop_ckpt = true; > } > > /* > @@ -2111,11 +2104,24 @@ static int f2fs_remount(struct super_block *sb, int *flags, char *data) > if ((*flags & SB_RDONLY) || !test_opt(sbi, FLUSH_MERGE)) { > clear_opt(sbi, FLUSH_MERGE); > f2fs_destroy_flush_cmd_control(sbi, false); > + need_restart_flush = true; > } else { > err = f2fs_create_flush_cmd_control(sbi); > if (err) > - goto restore_gc; > + goto restore_ckpt; > + need_stop_flush = true; > } > + > + if (checkpoint_changed) { > + if (test_opt(sbi, DISABLE_CHECKPOINT)) { > + err = f2fs_disable_checkpoint(sbi); > + if (err) > + goto restore_flush; > + } else { > + f2fs_enable_checkpoint(sbi); > + } > + } > + > skip: > #ifdef CONFIG_QUOTA > /* Release old quota file names */ > @@ -2130,6 +2136,21 @@ 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_flush: > + if (need_restart_flush) { > + if (f2fs_create_flush_cmd_control(sbi)) > + f2fs_warn(sbi, "background flush thread has stopped"); > + } else if (need_stop_flush) { > + clear_opt(sbi, FLUSH_MERGE); > + f2fs_destroy_flush_cmd_control(sbi, false); > + } > +restore_ckpt: > + if (need_restart_ckpt) { > + if (f2fs_start_ckpt_thread(sbi)) > + f2fs_warn(sbi, "background ckpt thread has stopped"); > + } else if (need_stop_ckpt) { > + f2fs_stop_ckpt_thread(sbi); > + } > restore_gc: > if (need_restart_gc) { > if (f2fs_start_gc_thread(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] 6+ messages in thread
end of thread, other threads:[~2021-03-24 2:20 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-17 9:56 [PATCH 1/2] f2fs: don't start checkpoint thread in readonly mountpoint Chao Yu 2021-03-17 9:56 ` [f2fs-dev] " Chao Yu 2021-03-17 9:56 ` [PATCH 2/2] f2fs: fix error path of f2fs_remount() Chao Yu 2021-03-17 9:56 ` [f2fs-dev] " Chao Yu 2021-03-24 2:19 ` Chao Yu 2021-03-24 2:19 ` [f2fs-dev] " 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.