* [f2fs-dev] [PATCH v5] f2fs: prevent the current section from being selected as a victim during GC
@ 2025-04-03 23:21 yohan.joung
2025-04-04 19:55 ` Jaegeuk Kim via Linux-f2fs-devel
0 siblings, 1 reply; 8+ messages in thread
From: yohan.joung @ 2025-04-03 23:21 UTC (permalink / raw)
To: jaegeuk, chao; +Cc: pilhyun.kim, linux-kernel, linux-f2fs-devel
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 | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index 0465dc00b349..0773283babfa 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -474,8 +474,15 @@ static inline void __set_test_and_free(struct f2fs_sb_info *sbi,
next = find_next_bit(free_i->free_segmap,
start_segno + SEGS_PER_SEC(sbi), start_segno);
if (next >= start_segno + usable_segs) {
- if (test_and_clear_bit(secno, free_i->free_secmap))
+ if (test_and_clear_bit(secno, free_i->free_secmap)) {
free_i->free_sections++;
+
+ if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[BG_GC]) == secno)
+ sbi->next_victim_seg[BG_GC] = NULL_SEGNO;
+
+ if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[FG_GC]) == secno)
+ sbi->next_victim_seg[FG_GC] = NULL_SEGNO;
+ }
}
}
skip_free:
--
2.33.0
_______________________________________________
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] 8+ messages in thread
* Re: [f2fs-dev] [PATCH v5] f2fs: prevent the current section from being selected as a victim during GC
2025-04-03 23:21 [f2fs-dev] [PATCH v5] f2fs: prevent the current section from being selected as a victim during GC yohan.joung
@ 2025-04-04 19:55 ` Jaegeuk Kim via Linux-f2fs-devel
2025-04-07 2:08 ` Chao Yu via Linux-f2fs-devel
0 siblings, 1 reply; 8+ messages in thread
From: Jaegeuk Kim via Linux-f2fs-devel @ 2025-04-04 19:55 UTC (permalink / raw)
To: yohan.joung; +Cc: pilhyun.kim, linux-kernel, linux-f2fs-devel
Hi Yohan,
I modified this patch after applying the clean up by
https://lore.kernel.org/linux-f2fs-devel/20250404195442.413945-1-jaegeuk@kernel.org/T/#u
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -486,6 +486,11 @@ static inline void __set_test_and_free(struct f2fs_sb_info *sbi,
free_i->free_sections++;
+ if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[BG_GC]) == secno)
+ sbi->next_victim_seg[BG_GC] = NULL_SEGNO;
+ if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[FG_GC]) == secno)
+ sbi->next_victim_seg[FG_GC] = NULL_SEGNO;
+
unlock_out:
spin_unlock(&free_i->segmap_lock);
}
On 04/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 | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> index 0465dc00b349..0773283babfa 100644
> --- a/fs/f2fs/segment.h
> +++ b/fs/f2fs/segment.h
> @@ -474,8 +474,15 @@ static inline void __set_test_and_free(struct f2fs_sb_info *sbi,
> next = find_next_bit(free_i->free_segmap,
> start_segno + SEGS_PER_SEC(sbi), start_segno);
> if (next >= start_segno + usable_segs) {
> - if (test_and_clear_bit(secno, free_i->free_secmap))
> + if (test_and_clear_bit(secno, free_i->free_secmap)) {
> free_i->free_sections++;
> +
> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[BG_GC]) == secno)
> + sbi->next_victim_seg[BG_GC] = NULL_SEGNO;
> +
> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[FG_GC]) == secno)
> + sbi->next_victim_seg[FG_GC] = NULL_SEGNO;
> + }
> }
> }
> skip_free:
> --
> 2.33.0
_______________________________________________
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] 8+ messages in thread
* Re: [f2fs-dev] [PATCH v5] f2fs: prevent the current section from being selected as a victim during GC
2025-04-04 19:55 ` Jaegeuk Kim via Linux-f2fs-devel
@ 2025-04-07 2:08 ` Chao Yu via Linux-f2fs-devel
2025-04-07 2:11 ` Chao Yu via Linux-f2fs-devel
0 siblings, 1 reply; 8+ messages in thread
From: Chao Yu via Linux-f2fs-devel @ 2025-04-07 2:08 UTC (permalink / raw)
To: Jaegeuk Kim, yohan.joung; +Cc: pilhyun.kim, linux-kernel, linux-f2fs-devel
On 4/5/25 03:55, Jaegeuk Kim wrote:
> Hi Yohan,
>
> I modified this patch after applying the clean up by
>
> https://lore.kernel.org/linux-f2fs-devel/20250404195442.413945-1-jaegeuk@kernel.org/T/#u
>
> --- a/fs/f2fs/segment.h
> +++ b/fs/f2fs/segment.h
> @@ -486,6 +486,11 @@ static inline void __set_test_and_free(struct f2fs_sb_info *sbi,
>
> free_i->free_sections++;
>
> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[BG_GC]) == secno)
> + sbi->next_victim_seg[BG_GC] = NULL_SEGNO;
> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[FG_GC]) == secno)
> + sbi->next_victim_seg[FG_GC] = NULL_SEGNO;
Reviewed-by: Chao Yu <chao@kernel.org>
Thanks,
> +
> unlock_out:
> spin_unlock(&free_i->segmap_lock);
> }
>
> On 04/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 | 9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
>> index 0465dc00b349..0773283babfa 100644
>> --- a/fs/f2fs/segment.h
>> +++ b/fs/f2fs/segment.h
>> @@ -474,8 +474,15 @@ static inline void __set_test_and_free(struct f2fs_sb_info *sbi,
>> next = find_next_bit(free_i->free_segmap,
>> start_segno + SEGS_PER_SEC(sbi), start_segno);
>> if (next >= start_segno + usable_segs) {
>> - if (test_and_clear_bit(secno, free_i->free_secmap))
>> + if (test_and_clear_bit(secno, free_i->free_secmap)) {
>> free_i->free_sections++;
>> +
>> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[BG_GC]) == secno)
>> + sbi->next_victim_seg[BG_GC] = NULL_SEGNO;
>> +
>> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[FG_GC]) == secno)
>> + sbi->next_victim_seg[FG_GC] = NULL_SEGNO;
>> + }
>> }
>> }
>> skip_free:
>> --
>> 2.33.0
_______________________________________________
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] 8+ messages in thread
* Re: [f2fs-dev] [PATCH v5] f2fs: prevent the current section from being selected as a victim during GC
2025-04-07 2:08 ` Chao Yu via Linux-f2fs-devel
@ 2025-04-07 2:11 ` Chao Yu via Linux-f2fs-devel
2025-04-10 3:53 ` Jaegeuk Kim via Linux-f2fs-devel
0 siblings, 1 reply; 8+ messages in thread
From: Chao Yu via Linux-f2fs-devel @ 2025-04-07 2:11 UTC (permalink / raw)
To: yohan.joung, Jaegeuk Kim; +Cc: pilhyun.kim, linux-kernel, linux-f2fs-devel
On 4/7/25 10:08, Chao Yu wrote:
> On 4/5/25 03:55, Jaegeuk Kim wrote:
>> Hi Yohan,
>>
>> I modified this patch after applying the clean up by
>>
>> https://lore.kernel.org/linux-f2fs-devel/20250404195442.413945-1-jaegeuk@kernel.org/T/#u
>>
>> --- a/fs/f2fs/segment.h
>> +++ b/fs/f2fs/segment.h
>> @@ -486,6 +486,11 @@ static inline void __set_test_and_free(struct f2fs_sb_info *sbi,
>>
>> free_i->free_sections++;
>>
>> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[BG_GC]) == secno)
>> + sbi->next_victim_seg[BG_GC] = NULL_SEGNO;
>> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[FG_GC]) == secno)
>> + sbi->next_victim_seg[FG_GC] = NULL_SEGNO;
>
> Reviewed-by: Chao Yu <chao@kernel.org>
Oh, can we add Fixes line to make it to be merged into stable kernel?
Thanks,
>
> Thanks,
>
>> +
>> unlock_out:
>> spin_unlock(&free_i->segmap_lock);
>> }
>>
>> On 04/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 | 9 ++++++++-
>>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
>>> index 0465dc00b349..0773283babfa 100644
>>> --- a/fs/f2fs/segment.h
>>> +++ b/fs/f2fs/segment.h
>>> @@ -474,8 +474,15 @@ static inline void __set_test_and_free(struct f2fs_sb_info *sbi,
>>> next = find_next_bit(free_i->free_segmap,
>>> start_segno + SEGS_PER_SEC(sbi), start_segno);
>>> if (next >= start_segno + usable_segs) {
>>> - if (test_and_clear_bit(secno, free_i->free_secmap))
>>> + if (test_and_clear_bit(secno, free_i->free_secmap)) {
>>> free_i->free_sections++;
>>> +
>>> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[BG_GC]) == secno)
>>> + sbi->next_victim_seg[BG_GC] = NULL_SEGNO;
>>> +
>>> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[FG_GC]) == secno)
>>> + sbi->next_victim_seg[FG_GC] = NULL_SEGNO;
>>> + }
>>> }
>>> }
>>> skip_free:
>>> --
>>> 2.33.0
>
_______________________________________________
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] 8+ messages in thread
* Re: [f2fs-dev] [PATCH v5] f2fs: prevent the current section from being selected as a victim during GC
2025-04-07 2:11 ` Chao Yu via Linux-f2fs-devel
@ 2025-04-10 3:53 ` Jaegeuk Kim via Linux-f2fs-devel
2025-04-10 5:12 ` yohan.joung
2025-04-10 5:14 ` Chao Yu via Linux-f2fs-devel
0 siblings, 2 replies; 8+ messages in thread
From: Jaegeuk Kim via Linux-f2fs-devel @ 2025-04-10 3:53 UTC (permalink / raw)
To: Chao Yu; +Cc: pilhyun.kim, linux-kernel, linux-f2fs-devel
On 04/07, Chao Yu wrote:
> On 4/7/25 10:08, Chao Yu wrote:
> > On 4/5/25 03:55, Jaegeuk Kim wrote:
> >> Hi Yohan,
> >>
> >> I modified this patch after applying the clean up by
> >>
> >> https://lore.kernel.org/linux-f2fs-devel/20250404195442.413945-1-jaegeuk@kernel.org/T/#u
> >>
> >> --- a/fs/f2fs/segment.h
> >> +++ b/fs/f2fs/segment.h
> >> @@ -486,6 +486,11 @@ static inline void __set_test_and_free(struct f2fs_sb_info *sbi,
> >>
> >> free_i->free_sections++;
> >>
> >> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[BG_GC]) == secno)
> >> + sbi->next_victim_seg[BG_GC] = NULL_SEGNO;
> >> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[FG_GC]) == secno)
> >> + sbi->next_victim_seg[FG_GC] = NULL_SEGNO;
> >
> > Reviewed-by: Chao Yu <chao@kernel.org>
>
> Oh, can we add Fixes line to make it to be merged into stable kernel?
Which one would be good to add?
>
> Thanks,
>
> >
> > Thanks,
> >
> >> +
> >> unlock_out:
> >> spin_unlock(&free_i->segmap_lock);
> >> }
> >>
> >> On 04/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 | 9 ++++++++-
> >>> 1 file changed, 8 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> >>> index 0465dc00b349..0773283babfa 100644
> >>> --- a/fs/f2fs/segment.h
> >>> +++ b/fs/f2fs/segment.h
> >>> @@ -474,8 +474,15 @@ static inline void __set_test_and_free(struct f2fs_sb_info *sbi,
> >>> next = find_next_bit(free_i->free_segmap,
> >>> start_segno + SEGS_PER_SEC(sbi), start_segno);
> >>> if (next >= start_segno + usable_segs) {
> >>> - if (test_and_clear_bit(secno, free_i->free_secmap))
> >>> + if (test_and_clear_bit(secno, free_i->free_secmap)) {
> >>> free_i->free_sections++;
> >>> +
> >>> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[BG_GC]) == secno)
> >>> + sbi->next_victim_seg[BG_GC] = NULL_SEGNO;
> >>> +
> >>> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[FG_GC]) == secno)
> >>> + sbi->next_victim_seg[FG_GC] = NULL_SEGNO;
> >>> + }
> >>> }
> >>> }
> >>> skip_free:
> >>> --
> >>> 2.33.0
> >
_______________________________________________
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] 8+ messages in thread
* Re: [f2fs-dev] [PATCH v5] f2fs: prevent the current section from being selected as a victim during GC
2025-04-10 3:53 ` Jaegeuk Kim via Linux-f2fs-devel
@ 2025-04-10 5:12 ` yohan.joung
2025-04-10 5:23 ` yohan.joung
2025-04-10 5:14 ` Chao Yu via Linux-f2fs-devel
1 sibling, 1 reply; 8+ messages in thread
From: yohan.joung @ 2025-04-10 5:12 UTC (permalink / raw)
To: Jaegeuk Kim via Linux-f2fs-devel; +Cc: pilhyun.kim, linux-kernel
On Thu, 10 Apr 2025 03:53:07 +0000 [thread overview] Jaegeuk Kim via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net> wrote:
> On 04/07, Chao Yu wrote:
> > On 4/7/25 10:08, Chao Yu wrote:
> > > On 4/5/25 03:55, Jaegeuk Kim wrote:
> > >> Hi Yohan,
> > >>
> > >> I modified this patch after applying the clean up by
> > >>
> > >> https://lore.kernel.org/linux-f2fs-devel/20250404195442.413945-1-jaegeuk@kernel.org/T/#u
> > >>
> > >> --- a/fs/f2fs/segment.h
> > >> +++ b/fs/f2fs/segment.h
> > >> @@ -486,6 +486,11 @@ static inline void __set_test_and_free(struct f2fs_sb_info *sbi,
> > >>
> > >> free_i->free_sections++;
> > >>
> > >> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[BG_GC]) == secno)
> > >> + sbi->next_victim_seg[BG_GC] = NULL_SEGNO;
> > >> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[FG_GC]) == secno)
> > >> + sbi->next_victim_seg[FG_GC] = NULL_SEGNO;
> > >
> > > Reviewed-by: Chao Yu <chao@kernel.org>
> >
> > Oh, can we add Fixes line to make it to be merged into stable kernel?
>
> Which one would be good to add?
jaegeuk, please apply the patch above
Thanks
> >
> > Thanks,
> >
> > >
> > > Thanks,
> > >
> > >> +
> > >> unlock_out:
> > >> spin_unlock(&free_i->segmap_lock);
> > >> }
> > >>
> > >> On 04/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 | 9 ++++++++-
> > >>> 1 file changed, 8 insertions(+), 1 deletion(-)
> > >>>
> > >>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> > >>> index 0465dc00b349..0773283babfa 100644
> > >>> --- a/fs/f2fs/segment.h
> > >>> +++ b/fs/f2fs/segment.h
> > >>> @@ -474,8 +474,15 @@ static inline void __set_test_and_free(struct f2fs_sb_info *sbi,
> > >>> next = find_next_bit(free_i->free_segmap,
> > >>> start_segno + SEGS_PER_SEC(sbi), start_segno);
> > >>> if (next >= start_segno + usable_segs) {
> > >>> - if (test_and_clear_bit(secno, free_i->free_secmap))
> > >>> + if (test_and_clear_bit(secno, free_i->free_secmap)) {
> > >>> free_i->free_sections++;
> > >>> +
> > >>> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[BG_GC]) == secno)
> > >>> + sbi->next_victim_seg[BG_GC] = NULL_SEGNO;
> > >>> +
> > >>> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[FG_GC]) == secno)
> > >>> + sbi->next_victim_seg[FG_GC] = NULL_SEGNO;
> > >>> + }
> > >>> }
> > >>> }
> > >>> skip_free:
> > >>> --
> > >>> 2.33.0
> > >
_______________________________________________
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] 8+ messages in thread
* Re: [f2fs-dev] [PATCH v5] f2fs: prevent the current section from being selected as a victim during GC
2025-04-10 3:53 ` Jaegeuk Kim via Linux-f2fs-devel
2025-04-10 5:12 ` yohan.joung
@ 2025-04-10 5:14 ` Chao Yu via Linux-f2fs-devel
1 sibling, 0 replies; 8+ messages in thread
From: Chao Yu via Linux-f2fs-devel @ 2025-04-10 5:14 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: pilhyun.kim, linux-kernel, linux-f2fs-devel
On 4/10/25 11:53, Jaegeuk Kim wrote:
> On 04/07, Chao Yu wrote:
>> On 4/7/25 10:08, Chao Yu wrote:
>>> On 4/5/25 03:55, Jaegeuk Kim wrote:
>>>> Hi Yohan,
>>>>
>>>> I modified this patch after applying the clean up by
>>>>
>>>> https://lore.kernel.org/linux-f2fs-devel/20250404195442.413945-1-jaegeuk@kernel.org/T/#u
>>>>
>>>> --- a/fs/f2fs/segment.h
>>>> +++ b/fs/f2fs/segment.h
>>>> @@ -486,6 +486,11 @@ static inline void __set_test_and_free(struct f2fs_sb_info *sbi,
>>>>
>>>> free_i->free_sections++;
>>>>
>>>> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[BG_GC]) == secno)
>>>> + sbi->next_victim_seg[BG_GC] = NULL_SEGNO;
>>>> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[FG_GC]) == secno)
>>>> + sbi->next_victim_seg[FG_GC] = NULL_SEGNO;
>>>
>>> Reviewed-by: Chao Yu <chao@kernel.org>
>>
>> Oh, can we add Fixes line to make it to be merged into stable kernel?
>
> Which one would be good to add?
I guess this one:
Fixes: e3080b0120a1 ("f2fs: support subsectional garbage collection")
Thanks,
>
>>
>> Thanks,
>>
>>>
>>> Thanks,
>>>
>>>> +
>>>> unlock_out:
>>>> spin_unlock(&free_i->segmap_lock);
>>>> }
>>>>
>>>> On 04/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 | 9 ++++++++-
>>>>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
>>>>> index 0465dc00b349..0773283babfa 100644
>>>>> --- a/fs/f2fs/segment.h
>>>>> +++ b/fs/f2fs/segment.h
>>>>> @@ -474,8 +474,15 @@ static inline void __set_test_and_free(struct f2fs_sb_info *sbi,
>>>>> next = find_next_bit(free_i->free_segmap,
>>>>> start_segno + SEGS_PER_SEC(sbi), start_segno);
>>>>> if (next >= start_segno + usable_segs) {
>>>>> - if (test_and_clear_bit(secno, free_i->free_secmap))
>>>>> + if (test_and_clear_bit(secno, free_i->free_secmap)) {
>>>>> free_i->free_sections++;
>>>>> +
>>>>> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[BG_GC]) == secno)
>>>>> + sbi->next_victim_seg[BG_GC] = NULL_SEGNO;
>>>>> +
>>>>> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[FG_GC]) == secno)
>>>>> + sbi->next_victim_seg[FG_GC] = NULL_SEGNO;
>>>>> + }
>>>>> }
>>>>> }
>>>>> skip_free:
>>>>> --
>>>>> 2.33.0
>>>
_______________________________________________
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] 8+ messages in thread
* Re: [f2fs-dev] [PATCH v5] f2fs: prevent the current section from being selected as a victim during GC
2025-04-10 5:12 ` yohan.joung
@ 2025-04-10 5:23 ` yohan.joung
0 siblings, 0 replies; 8+ messages in thread
From: yohan.joung @ 2025-04-10 5:23 UTC (permalink / raw)
To: yohan.joung; +Cc: pilhyun.kim, linux-kernel, Jaegeuk Kim via Linux-f2fs-devel
On Thu, 10 Apr 2025 14:12:39 +0900 [thread overview] "yohan.joung" <yohan.joung@sk.com> wrote:
> On Thu, 10 Apr 2025 03:53:07 +0000 [thread overview] Jaegeuk Kim via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net> wrote:
> > On 04/07, Chao Yu wrote:
> > > On 4/7/25 10:08, Chao Yu wrote:
> > > > On 4/5/25 03:55, Jaegeuk Kim wrote:
> > > >> Hi Yohan,
> > > >>
> > > >> I modified this patch after applying the clean up by
> > > >>
> > > >> https://lore.kernel.org/linux-f2fs-devel/20250404195442.413945-1-jaegeuk@kernel.org/T/#u
> > > >>
> > > >> --- a/fs/f2fs/segment.h
> > > >> +++ b/fs/f2fs/segment.h
> > > >> @@ -486,6 +486,11 @@ static inline void __set_test_and_free(struct f2fs_sb_info *sbi,
> > > >>
> > > >> free_i->free_sections++;
> > > >>
> > > >> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[BG_GC]) == secno)
> > > >> + sbi->next_victim_seg[BG_GC] = NULL_SEGNO;
> > > >> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[FG_GC]) == secno)
> > > >> + sbi->next_victim_seg[FG_GC] = NULL_SEGNO;
> > > >
> > > > Reviewed-by: Chao Yu <chao@kernel.org>
> > >
> > > Oh, can we add Fixes line to make it to be merged into stable kernel?
> >
> > Which one would be good to add?
> jaegeuk, please apply the patch above
> Thanks
sorry it is correct to apply the one below.
> > >
> > > Thanks,
> > >
> > > >
> > > > Thanks,
> > > >
> > > >> +
> > > >> unlock_out:
> > > >> spin_unlock(&free_i->segmap_lock);
> > > >> }
> > > >>
> > > >> On 04/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 | 9 ++++++++-
> > > >>> 1 file changed, 8 insertions(+), 1 deletion(-)
> > > >>>
> > > >>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> > > >>> index 0465dc00b349..0773283babfa 100644
> > > >>> --- a/fs/f2fs/segment.h
> > > >>> +++ b/fs/f2fs/segment.h
> > > >>> @@ -474,8 +474,15 @@ static inline void __set_test_and_free(struct f2fs_sb_info *sbi,
> > > >>> next = find_next_bit(free_i->free_segmap,
> > > >>> start_segno + SEGS_PER_SEC(sbi), start_segno);
> > > >>> if (next >= start_segno + usable_segs) {
> > > >>> - if (test_and_clear_bit(secno, free_i->free_secmap))
> > > >>> + if (test_and_clear_bit(secno, free_i->free_secmap)) {
> > > >>> free_i->free_sections++;
> > > >>> +
> > > >>> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[BG_GC]) == secno)
> > > >>> + sbi->next_victim_seg[BG_GC] = NULL_SEGNO;
> > > >>> +
> > > >>> + if (GET_SEC_FROM_SEG(sbi, sbi->next_victim_seg[FG_GC]) == secno)
> > > >>> + sbi->next_victim_seg[FG_GC] = NULL_SEGNO;
> > > >>> + }
> > > >>> }
> > > >>> }
> > > >>> skip_free:
> > > >>> --
> > > >>> 2.33.0
_______________________________________________
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] 8+ messages in thread
end of thread, other threads:[~2025-04-10 5:24 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-03 23:21 [f2fs-dev] [PATCH v5] f2fs: prevent the current section from being selected as a victim during GC yohan.joung
2025-04-04 19:55 ` Jaegeuk Kim via Linux-f2fs-devel
2025-04-07 2:08 ` Chao Yu via Linux-f2fs-devel
2025-04-07 2:11 ` Chao Yu via Linux-f2fs-devel
2025-04-10 3:53 ` Jaegeuk Kim via Linux-f2fs-devel
2025-04-10 5:12 ` yohan.joung
2025-04-10 5:23 ` yohan.joung
2025-04-10 5:14 ` Chao Yu via Linux-f2fs-devel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).