| Message ID | 20230414035146.1381029-1-chao@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [f2fs-dev] f2fs_io: support checkpoint command | expand |
>Fixed a xfstests failure. > >From 400c722c2117660b83190c88e5442d63fbbffe6e Mon Sep 17 00:00:00 2001 >From: Jaegeuk Kim <jaegeuk@kernel.org> >Date: Mon, 10 Apr 2023 14:48:50 -0700 >Subject: [PATCH] f2fs: refactor f2fs_gc to call checkpoint in urgent condition > >The major change is to call checkpoint, if there's not enough space while having >some prefree segments in FG_GC case. > >Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> >--- > fs/f2fs/gc.c | 27 +++++++++++++-------------- > 1 file changed, 13 insertions(+), 14 deletions(-) > >diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c >index c748cdfb0501..ba5775dcade6 100644 >--- a/fs/f2fs/gc.c >+++ b/fs/f2fs/gc.c >@@ -1829,7 +1829,10 @@ int f2fs_gc(struct f2fs_sb_info *sbi, struct f2fs_gc_control *gc_control) > goto stop; > } > >- if (gc_type == BG_GC && has_not_enough_free_secs(sbi, 0, 0)) { >+ /* Let's run FG_GC, if we don't have enough space. */ >+ if (has_not_enough_free_secs(sbi, 0, 0)) { >+ gc_type = FG_GC; >+ Hi, Jaegeuk & Chao Would it be possible to clarify if this patch is intended to perform checkpoint every gc round? Thanks. > /* > * For example, if there are many prefree_segments below given > * threshold, we can make them free by checkpoint. Then, we >@@ -1840,8 +1843,6 @@ int f2fs_gc(struct f2fs_sb_info *sbi, struct f2fs_gc_control *gc_control) > if (ret) > goto stop; > } >- if (has_not_enough_free_secs(sbi, 0, 0)) >- gc_type = FG_GC; > } > > /* f2fs_balance_fs doesn't need to do BG_GC in critical path. */ >@@ -1868,19 +1869,15 @@ int f2fs_gc(struct f2fs_sb_info *sbi, struct f2fs_gc_control *gc_control) > if (seg_freed == f2fs_usable_segs_in_sec(sbi, segno)) > sec_freed++; > >- if (gc_type == FG_GC) >+ if (gc_type == FG_GC) { > sbi->cur_victim_sec = NULL_SEGNO; > >- if (gc_control->init_gc_type == FG_GC || >- !has_not_enough_free_secs(sbi, >- (gc_type == FG_GC) ? sec_freed : 0, 0)) { >- if (gc_type == FG_GC && sec_freed < gc_control->nr_free_secs) >- goto go_gc_more; >- goto stop; >- } >- >- /* FG_GC stops GC by skip_count */ >- if (gc_type == FG_GC) { >+ if (!has_not_enough_free_secs(sbi, sec_freed, 0)) { >+ if (!gc_control->no_bg_gc && >+ sec_freed < gc_control->nr_free_secs) >+ goto go_gc_more; >+ goto stop; >+ } > if (sbi->skipped_gc_rwsem) > skipped_round++; > round++; >@@ -1889,6 +1886,8 @@ int f2fs_gc(struct f2fs_sb_info *sbi, struct f2fs_gc_control *gc_control) > ret = f2fs_write_checkpoint(sbi, &cpc); > goto stop; > } >+ } else if (!has_not_enough_free_secs(sbi, 0, 0)) { >+ goto stop; > } > > __get_secs_required(sbi, NULL, &upper_secs, NULL); >-- >2.40.0.634.g4ca3ef3211-goog > > > >_______________________________________________ >Linux-f2fs-devel mailing list >Linux-f2fs-devel@lists.sourceforge.net >https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
On 04/17, Yonggil Song wrote: > >Fixed a xfstests failure. > > > >From 400c722c2117660b83190c88e5442d63fbbffe6e Mon Sep 17 00:00:00 2001 > >From: Jaegeuk Kim <jaegeuk@kernel.org> > >Date: Mon, 10 Apr 2023 14:48:50 -0700 > >Subject: [PATCH] f2fs: refactor f2fs_gc to call checkpoint in urgent condition > > > >The major change is to call checkpoint, if there's not enough space while having > >some prefree segments in FG_GC case. > > > >Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> > >--- > > fs/f2fs/gc.c | 27 +++++++++++++-------------- > > 1 file changed, 13 insertions(+), 14 deletions(-) > > > >diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c > >index c748cdfb0501..ba5775dcade6 100644 > >--- a/fs/f2fs/gc.c > >+++ b/fs/f2fs/gc.c > >@@ -1829,7 +1829,10 @@ int f2fs_gc(struct f2fs_sb_info *sbi, struct f2fs_gc_control *gc_control) > > goto stop; > > } > > > >- if (gc_type == BG_GC && has_not_enough_free_secs(sbi, 0, 0)) { > >+ /* Let's run FG_GC, if we don't have enough space. */ > >+ if (has_not_enough_free_secs(sbi, 0, 0)) { > >+ gc_type = FG_GC; > >+ > > Hi, Jaegeuk & Chao > > Would it be possible to clarify if this patch is intended to perform checkpoint every gc round? Intention is to trigger checkpoint when there's not enough free space. So, it's not for every gc round. > > Thanks. > > > /* > > * For example, if there are many prefree_segments below given > > * threshold, we can make them free by checkpoint. Then, we > >@@ -1840,8 +1843,6 @@ int f2fs_gc(struct f2fs_sb_info *sbi, struct f2fs_gc_control *gc_control) > > if (ret) > > goto stop; > > } > >- if (has_not_enough_free_secs(sbi, 0, 0)) > >- gc_type = FG_GC; > > } > > > > /* f2fs_balance_fs doesn't need to do BG_GC in critical path. */ > >@@ -1868,19 +1869,15 @@ int f2fs_gc(struct f2fs_sb_info *sbi, struct f2fs_gc_control *gc_control) > > if (seg_freed == f2fs_usable_segs_in_sec(sbi, segno)) > > sec_freed++; > > > >- if (gc_type == FG_GC) > >+ if (gc_type == FG_GC) { > > sbi->cur_victim_sec = NULL_SEGNO; > > > >- if (gc_control->init_gc_type == FG_GC || > >- !has_not_enough_free_secs(sbi, > >- (gc_type == FG_GC) ? sec_freed : 0, 0)) { > >- if (gc_type == FG_GC && sec_freed < gc_control->nr_free_secs) > >- goto go_gc_more; > >- goto stop; > >- } > >- > >- /* FG_GC stops GC by skip_count */ > >- if (gc_type == FG_GC) { > >+ if (!has_not_enough_free_secs(sbi, sec_freed, 0)) { > >+ if (!gc_control->no_bg_gc && > >+ sec_freed < gc_control->nr_free_secs) > >+ goto go_gc_more; > >+ goto stop; > >+ } > > if (sbi->skipped_gc_rwsem) > > skipped_round++; > > round++; > >@@ -1889,6 +1886,8 @@ int f2fs_gc(struct f2fs_sb_info *sbi, struct f2fs_gc_control *gc_control) > > ret = f2fs_write_checkpoint(sbi, &cpc); > > goto stop; > > } > >+ } else if (!has_not_enough_free_secs(sbi, 0, 0)) { > >+ goto stop; > > } > > > > __get_secs_required(sbi, NULL, &upper_secs, NULL); > >-- > >2.40.0.634.g4ca3ef3211-goog > > > > > > > >_______________________________________________ > >Linux-f2fs-devel mailing list > >Linux-f2fs-devel@lists.sourceforge.net > >https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
On 04/17, Yonggil Song wrote: >> >Fixed a xfstests failure. >> > >> >From 400c722c2117660b83190c88e5442d63fbbffe6e Mon Sep 17 00:00:00 2001 >> >From: Jaegeuk Kim <jaegeuk@kernel.org> >> >Date: Mon, 10 Apr 2023 14:48:50 -0700 >> >Subject: [PATCH] f2fs: refactor f2fs_gc to call checkpoint in urgent condition >> > >> >The major change is to call checkpoint, if there's not enough space while having >> >some prefree segments in FG_GC case. >> > >> >Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> >> >--- >> > fs/f2fs/gc.c | 27 +++++++++++++-------------- >> > 1 file changed, 13 insertions(+), 14 deletions(-) >> > >> >diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c >> >index c748cdfb0501..ba5775dcade6 100644 >> >--- a/fs/f2fs/gc.c >> >+++ b/fs/f2fs/gc.c >> >@@ -1829,7 +1829,10 @@ int f2fs_gc(struct f2fs_sb_info *sbi, struct f2fs_gc_control *gc_control) >> > goto stop; >> > } >> > >> >- if (gc_type == BG_GC && has_not_enough_free_secs(sbi, 0, 0)) { >> >+ /* Let's run FG_GC, if we don't have enough space. */ >> >+ if (has_not_enough_free_secs(sbi, 0, 0)) { >> >+ gc_type = FG_GC; >> >+ >> >> Hi, Jaegeuk & Chao >> >> Would it be possible to clarify if this patch is intended to perform checkpoint every gc round? > >Intention is to trigger checkpoint when there's not enough free space. So, it's >not for every gc round. > Thanks for your reply. When the file system is almost full, the victim’s valid blocks ratio is high. Therefore, most gc rounds consume free sections to relocate victims. So, free sections shrink and prefree remains after jumping to gc_more. Wouldn’t this trigger a checkpoint every gc round? Thanks. >> >> Thanks. >> >> > /* >> > * For example, if there are many prefree_segments below given >> > * threshold, we can make them free by checkpoint. Then, we >> >@@ -1840,8 +1843,6 @@ int f2fs_gc(struct f2fs_sb_info *sbi, struct f2fs_gc_control *gc_control) >> > if (ret) >> > goto stop; >> > } >> >- if (has_not_enough_free_secs(sbi, 0, 0)) >> >- gc_type = FG_GC; >> > } >> > >> > /* f2fs_balance_fs doesn't need to do BG_GC in critical path. */ >> >@@ -1868,19 +1869,15 @@ int f2fs_gc(struct f2fs_sb_info *sbi, struct f2fs_gc_control *gc_control) >> > if (seg_freed == f2fs_usable_segs_in_sec(sbi, segno)) >> > sec_freed++; >> > >> >- if (gc_type == FG_GC) >> >+ if (gc_type == FG_GC) { >> > sbi->cur_victim_sec = NULL_SEGNO; >> > >> >- if (gc_control->init_gc_type == FG_GC || >> >- !has_not_enough_free_secs(sbi, >> >- (gc_type == FG_GC) ? sec_freed : 0, 0)) { >> >- if (gc_type == FG_GC && sec_freed < gc_control->nr_free_secs) >> >- goto go_gc_more; >> >- goto stop; >> >- } >> >- >> >- /* FG_GC stops GC by skip_count */ >> >- if (gc_type == FG_GC) { >> >+ if (!has_not_enough_free_secs(sbi, sec_freed, 0)) { >> >+ if (!gc_control->no_bg_gc && >> >+ sec_freed < gc_control->nr_free_secs) >> >+ goto go_gc_more; >> >+ goto stop; >> >+ } >> > if (sbi->skipped_gc_rwsem) >> > skipped_round++; >> > round++; >> >@@ -1889,6 +1886,8 @@ int f2fs_gc(struct f2fs_sb_info *sbi, struct f2fs_gc_control *gc_control) >> > ret = f2fs_write_checkpoint(sbi, &cpc); >> > goto stop; >> > } >> >+ } else if (!has_not_enough_free_secs(sbi, 0, 0)) { >> >+ goto stop; >> > } >> > >> > __get_secs_required(sbi, NULL, &upper_secs, NULL); >> >-- >> >2.40.0.634.g4ca3ef3211-goog >> > >> > >> > >> >_______________________________________________ >> >Linux-f2fs- >> mailing list >> >Linux-f2fs- >>@lists.sourceforge.net >> >https://lists.sourceforge.net/lists/listinfo/linux-f2fs- >>
diff --git a/man/f2fs_io.8 b/man/f2fs_io.8 index 33789c2..f1a48ca 100644 --- a/man/f2fs_io.8 +++ b/man/f2fs_io.8 @@ -135,6 +135,9 @@ Reserve free blocks to prepare decompressing blocks in the file. .TP \fBgc\fR \fI[sync_mode] [file]\fR Trigger filesystem GC +.TP +\fBcheckpoint\fR \fI[file]\fR +Trigger filesystem checkpoint .SH AUTHOR This version of .B f2fs_io diff --git a/tools/f2fs_io/f2fs_io.c b/tools/f2fs_io/f2fs_io.c index 6dcd840..9cfca71 100644 --- a/tools/f2fs_io/f2fs_io.c +++ b/tools/f2fs_io/f2fs_io.c @@ -1310,6 +1310,30 @@ static void do_gc(int argc, char **argv, const struct cmd_desc *cmd) exit(0); } +#define checkpoint_desc "trigger filesystem checkpoint" +#define checkpoint_help "f2fs_io checkpoint [file_path]\n\n" + +static void do_checkpoint(int argc, char **argv, const struct cmd_desc *cmd) +{ + int ret, fd; + + if (argc != 2) { + fputs("Excess arguments\n\n", stderr); + fputs(cmd->cmd_help, stderr); + exit(1); + } + + fd = xopen(argv[1], O_WRONLY, 0); + + ret = ioctl(fd, F2FS_IOC_WRITE_CHECKPOINT); + if (ret < 0) + die_errno("F2FS_IOC_WRITE_CHECKPOINT failed"); + + printf("trigger filesystem checkpoint ret=%d\n", ret); + exit(0); +} + + #define CMD_HIDDEN 0x0001 #define CMD(name) { #name, do_##name, name##_desc, name##_help, 0 } #define _CMD(name) { #name, do_##name, NULL, NULL, CMD_HIDDEN } @@ -1342,6 +1366,7 @@ const struct cmd_desc cmd_list[] = { CMD(get_filename_encrypt_mode), CMD(rename), CMD(gc), + CMD(checkpoint), { NULL, NULL, NULL, NULL, 0 } };
This patch supports a new sub-command 'checkpoint' in f2fs_io to trigger filesystem checkpoint via F2FS_IOC_WRITE_CHECKPOINT ioctl. Signed-off-by: Chao Yu <chao@kernel.org> --- man/f2fs_io.8 | 3 +++ tools/f2fs_io/f2fs_io.c | 25 +++++++++++++++++++++++++ 2 files changed, 28 insertions(+)