| Message ID | 20250402080428.2811-1-yohan.joung@sk.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [f2fs-dev,v3] f2fs: prevent the current section from being selected as a victim during GC | expand |
On 2025/4/2 16:04, yohan.joung wrote: > When selecting a victim using next_victim_seg in a large section, the > selected section might already have been cleared and designated as the > new current section, making it actively in use. > This behavior causes inconsistency between the SIT and SSA. > > F2FS-fs (dm-54): Inconsistent segment (70961) type [0, 1] in SSA and SIT > Call trace: > dump_backtrace+0xe8/0x10c > show_stack+0x18/0x28 > dump_stack_lvl+0x50/0x6c > dump_stack+0x18/0x28 > f2fs_stop_checkpoint+0x1c/0x3c > do_garbage_collect+0x41c/0x271c > f2fs_gc+0x27c/0x828 > gc_thread_func+0x290/0x88c > kthread+0x11c/0x164 > ret_from_fork+0x10/0x20 > > issue scenario > segs_per_sec=2 > - seg#0 and seg#1 are all dirty > - all valid blocks are removed in seg#1 > - gc select this sec and next_victim_seg=seg#0 > - migrate seg#0, next_victim_seg=seg#1 > - checkpoint -> sec(seg#0, seg#1) becomes free > - allocator assigns sec(seg#0, seg#1) to curseg > - gc tries to migrate seg#1 > > Signed-off-by: yohan.joung <yohan.joung@sk.com> > Signed-off-by: Chao Yu <chao@kernel.org> > --- > fs/f2fs/segment.h | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h > index 0465dc00b349..129df633d656 100644 > --- a/fs/f2fs/segment.h > +++ b/fs/f2fs/segment.h > @@ -476,6 +476,12 @@ static inline void __set_test_and_free(struct f2fs_sb_info *sbi, > if (next >= start_segno + usable_segs) { > if (test_and_clear_bit(secno, free_i->free_secmap)) > free_i->free_sections++; > + > + if (GET_SEC_FROM_SEG(sbi->next_victim_seg[BG_GC]) == secno) > + sbi->next_victim_seg[BG_GC] = NULL_SEGNO; > + > + if (GET_SEC_FROM_SEG(sbi->next_victim_seg[FG_GC]) == secno) > + sbi->next_victim_seg[FG_GC] = NULL_SEGNO; Only need to reset next_victim_seg[] when setting section free? if (!test_and_clear_bit(secno, free_i->free_secmap)) goto skip_free; free_i->free_sections++; if (GET_SEC_FROM_SEG(sbi->next_victim_seg[BG_GC]) == secno) sbi->next_victim_seg[BG_GC] = NULL_SEGNO; if (GET_SEC_FROM_SEG(sbi->next_victim_seg[FG_GC]) == secno) sbi->next_victim_seg[FG_GC] = NULL_SEGNO; Thanks, > } > } > skip_free:
On Thu, 3 Apr 2025 14:26:59 +0800 Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net> wrote: > On 2025/4/2 16:04, yohan.joung wrote: > > When selecting a victim using next_victim_seg in a large section, the > > selected section might already have been cleared and designated as the > > new current section, making it actively in use. > > This behavior causes inconsistency between the SIT and SSA. > > > > F2FS-fs (dm-54): Inconsistent segment (70961) type [0, 1] in SSA and SIT > > Call trace: > > dump_backtrace+0xe8/0x10c > > show_stack+0x18/0x28 > > dump_stack_lvl+0x50/0x6c > > dump_stack+0x18/0x28 > > f2fs_stop_checkpoint+0x1c/0x3c > > do_garbage_collect+0x41c/0x271c > > f2fs_gc+0x27c/0x828 > > gc_thread_func+0x290/0x88c > > kthread+0x11c/0x164 > > ret_from_fork+0x10/0x20 > > > > issue scenario > > segs_per_sec=2 > > - seg#0 and seg#1 are all dirty > > - all valid blocks are removed in seg#1 > > - gc select this sec and next_victim_seg=seg#0 > > - migrate seg#0, next_victim_seg=seg#1 > > - checkpoint -> sec(seg#0, seg#1) becomes free > > - allocator assigns sec(seg#0, seg#1) to curseg > > - gc tries to migrate seg#1 > > > > Signed-off-by: yohan.joung <yohan.joung@sk.com> > > Signed-off-by: Chao Yu <chao@kernel.org> > > --- > > fs/f2fs/segment.h | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h > > index 0465dc00b349..129df633d656 100644 > > --- a/fs/f2fs/segment.h > > +++ b/fs/f2fs/segment.h > > @@ -476,6 +476,12 @@ static inline void __set_test_and_free(struct f2fs_sb_info *sbi, > > if (next >= start_segno + usable_segs) { > > if (test_and_clear_bit(secno, free_i->free_secmap)) > > free_i->free_sections++; > > + > > + if (GET_SEC_FROM_SEG(sbi->next_victim_seg[BG_GC]) == secno) > > + sbi->next_victim_seg[BG_GC] = NULL_SEGNO; > > + > > + if (GET_SEC_FROM_SEG(sbi->next_victim_seg[FG_GC]) == secno) > > + sbi->next_victim_seg[FG_GC] = NULL_SEGNO; > > Only need to reset next_victim_seg[] when setting section free? Ah, I missed that. I'll put up the patch again right away. Thanks. > > if (!test_and_clear_bit(secno, free_i->free_secmap)) > goto skip_free; > > free_i->free_sections++; > > if (GET_SEC_FROM_SEG(sbi->next_victim_seg[BG_GC]) == secno) > sbi->next_victim_seg[BG_GC] = NULL_SEGNO; > if (GET_SEC_FROM_SEG(sbi->next_victim_seg[FG_GC]) == secno) > sbi->next_victim_seg[FG_GC] = NULL_SEGNO; > > Thanks, > > > } > > } > > skip_free: > > > > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h index 0465dc00b349..129df633d656 100644 --- a/fs/f2fs/segment.h +++ b/fs/f2fs/segment.h @@ -476,6 +476,12 @@ static inline void __set_test_and_free(struct f2fs_sb_info *sbi, if (next >= start_segno + usable_segs) { if (test_and_clear_bit(secno, free_i->free_secmap)) free_i->free_sections++; + + if (GET_SEC_FROM_SEG(sbi->next_victim_seg[BG_GC]) == secno) + sbi->next_victim_seg[BG_GC] = NULL_SEGNO; + + if (GET_SEC_FROM_SEG(sbi->next_victim_seg[FG_GC]) == secno) + sbi->next_victim_seg[FG_GC] = NULL_SEGNO; } } skip_free: