Re: [f2fs-dev] [PATCH] f2fs: fix to avoid migrating empty section

[Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>]

On 9/3/25 08:04, Zhiguo Niu wrote:
> Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>
> 于2025年9月1日周一 10:08写道：
>>
>> It reports a bug from device w/ zufs:
>>
>> F2FS-fs (dm-64): Inconsistent segment (173822) type [1, 0] in SSA and SIT
>> F2FS-fs (dm-64): Stopped filesystem due to reason: 4
>>
>> Thread A                                Thread B
>> - f2fs_expand_inode_data
>>  - f2fs_allocate_pinning_section
>>   - f2fs_gc_range
>>    - do_garbage_collect w/ segno #x
>>                                         - writepage
>>                                          - f2fs_allocate_data_block
>>                                           - new_curseg
>>                                            - allocate segno #x
>>
>> The root cause is: fallocate on pinning file may race w/ block allocation
>> as above, result in do_garbage_collect() from fallocate() may migrate
>> segment which is just allocated by a log, the log will update segment type
>> in its in-memory structure, however GC will get segment type from on-disk
>> SSA block, once segment type changes by log, we can detect such
>> inconsistency, then shutdown filesystem.
>>
>> In this case, on-disk SSA shows type of segno #173822 is 1 (SUM_TYPE_NODE),
>> however segno #173822 was just allocated as data type segment, so in-memory
>> SIT shows type of segno #173822 is 0 (SUM_TYPE_DATA).
>>
>> Change as below to fix this issue:
>> - check whether current section is empty before gc
>> - add sanity checks on do_garbage_collect() to avoid any race case, result
>> in migrating segment used by log.
>> - btw, it fixes misc issue in printed logs: "SSA and SIT" -> "SIT and SSA".
>>
>> Fixes: 9703d69d9d15 ("f2fs: support file pinning for zoned devices")
>> Cc: Daeho Jeong <daehojeong@google.com>
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
>>  fs/f2fs/gc.c | 16 +++++++++++++++-
>>  1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>> index ed3acbfc83ca..a7708cf80c04 100644
>> --- a/fs/f2fs/gc.c
>> +++ b/fs/f2fs/gc.c
>> @@ -1794,6 +1794,13 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
>>                 struct folio *sum_folio = filemap_get_folio(META_MAPPING(sbi),
>>                                         GET_SUM_BLOCK(sbi, segno));
>>
>> +               if (is_cursec(sbi, GET_SEC_FROM_SEG(sbi, segno))) {
>> +                       f2fs_err(sbi, "%s: segment %u is used by log",
>> +                                                       __func__, segno);
>> +                       f2fs_bug_on(sbi, 1);
> Hi Chao,
> Do we need f2fs_bug_on here?  or it is enough to skip current segno
> and let gc continue?

Zhiguo,

I think we should never select in-use segment as GC victim, so I add a
f2fs_bug_on() here to detect any potential bugs. Let's see what will we
find w/ this.

Thanks,

> Thanks!
>> +                       goto skip;
>> +               }
>> +
>>                 if (get_valid_blocks(sbi, segno, false) == 0)
>>                         goto freed;
>>                 if (gc_type == BG_GC && __is_large_section(sbi) &&
>> @@ -1805,7 +1812,7 @@ static int do_garbage_collect(struct f2fs_sb_info *sbi,
>>
>>                 sum = folio_address(sum_folio);
>>                 if (type != GET_SUM_TYPE((&sum->footer))) {
>> -                       f2fs_err(sbi, "Inconsistent segment (%u) type [%d, %d] in SSA and SIT",
>> +                       f2fs_err(sbi, "Inconsistent segment (%u) type [%d, %d] in SIT and SSA",
>>                                  segno, type, GET_SUM_TYPE((&sum->footer)));
>>                         f2fs_stop_checkpoint(sbi, false,
>>                                 STOP_CP_REASON_CORRUPTED_SUMMARY);
>> @@ -2068,6 +2075,13 @@ int f2fs_gc_range(struct f2fs_sb_info *sbi,
>>                         .iroot = RADIX_TREE_INIT(gc_list.iroot, GFP_NOFS),
>>                 };
>>
>> +               /*
>> +                * avoid migrating empty section, as it can be allocated by
>> +                * log in parallel.
>> +                */
>> +               if (!get_valid_blocks(sbi, segno, true))
>> +                       continue;
>> +
>>                 if (is_cursec(sbi, GET_SEC_FROM_SEG(sbi, segno)))
>>                         continue;
>>
>> --
>> 2.49.0
>>
>>
>>
>> _______________________________________________
>> Linux-f2fs-devel mailing list
>> Linux-f2fs-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel