[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7059eada-a51d-4f68-b62a-0f2c89c9b01c@kernel.org>
Date: Mon, 7 Apr 2025 10:08:30 +0800
From: Chao Yu <chao@...nel.org>
To: Jaegeuk Kim <jaegeuk@...nel.org>, "yohan.joung" <yohan.joung@...com>
Cc: chao@...nel.org, linux-f2fs-devel@...ts.sourceforge.net,
linux-kernel@...r.kernel.org, pilhyun.kim@...com
Subject: Re: [PATCH v5] f2fs: prevent the current section from being selected
as a victim during GC
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@...nel.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@...com>
>> Signed-off-by: Chao Yu <chao@...nel.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
Powered by blists - more mailing lists