On 6/16/26 13:47, Zorro Lang wrote:
> On Tue, Jun 16, 2026 at 11:33:20AM +0800, Chao Yu wrote:
>> On 6/16/26 04:05, Zorro Lang wrote:
>>> On Mon, Jun 15, 2026 at 04:22:34PM +0800, Chao Yu wrote:
>>>> On 6/15/26 01:16, Zorro Lang wrote:
>>>>> On Fri, Jun 12, 2026 at 12:58:02AM +0000, Chao Yu wrote:
>>>>>> Without commit 520760b9f915 ("f2fs: optimize representative type
>>>>>> determination
>>>>>> in GC"), f2fs GC will report inconsistent segment type in large section
>>>>>> issue,
>>>>>> and then it will force to shutdown filesystem.
>>>>>>
>>>>>> [ 768.190903] F2FS-fs (loop51): Inconsistent segment (3) type [1, 0] in
>>>>>> SIT and SSA
>>>>>>
>>>>>> The reason is f2fs kernel will assume all segment type inside large
>>>>>> section is
>>>>>> the same, during GC it loads type from one segment and compare it to
>>>>>> other
>>>>>> segments' type, however due to recovery flow, the chosen segment may has
>>>>>> zero
>>>>>> valid blocks w/ different segment type, since the segment is
>>>>>> invalid(free) one,
>>>>>> it will never be migrated, so that we should not treat such state as
>>>>>> abnormal
>>>>>> condition.
>>>>>>
>>>>>> This testcase is created to simulate above condition to see whether f2fs
>>>>>> kernel
>>>>>> module can handle it correctly
>>>>>>
>>>>>> Signed-off-by: Chao Yu <[email protected]>
>>>>>> ---
>>>>>> v2:
>>>>>> - clear MKFS_OPTIONS and MOUNT_OPTIONS to guarantee block allocation is
>>>>>> as expected.
>>>>>
>>>>> Hi Chao,
>>>>>
>>>>> Sorry, I just noticed your reply to my review on the previous patch
>>>>> version.
>>>>> Due to some unexpected shake-ups recently, I’ve been bogged down with
>>>>> setting
>>>>> up and modifying various new system environments, and I accidentally
>>>>> marked
>>>>> some unread emails as read.
>>>>
>>>> No worries. :)
>>>>
>>>>>
>>>>> The patch looks good to me, with just a few picky review points below:
>>>>
>>>> Thanks Zorro for taking a look.
>>>>
>>>>>
>>>>>> tests/f2fs/025 | 92 ++++++++++++++++++++++++++++++++++++++++++++++
>>>>>> tests/f2fs/025.out | 2 +
>>>>>> 2 files changed, 94 insertions(+)
>>>>>> create mode 100644 tests/f2fs/025
>>>>>> create mode 100644 tests/f2fs/025.out
>>>>>>
>>>>>> diff --git a/tests/f2fs/025 b/tests/f2fs/025
>>>>>> new file mode 100644
>>>>>> index 000000000..397e5439a
>>>>>> --- /dev/null
>>>>>> +++ b/tests/f2fs/025
>>>>>> @@ -0,0 +1,92 @@
>>>>>> +#! /bin/bash
>>>>>> +# SPDX-License-Identifier: GPL-2.0
>>>>>> +# Copyright (c) 2026 Chao Yu <[email protected]>
>>>>>> +#
>>>>>> +# FS QA Test No. f2fs/025
>>>>>> +#
>>>>>> +# Check whether f2fs will encounter cp_error (Inconsistent segment type)
>>>>>> +# when doing sanity check on type of segments inside large section
>>>>>> during
>>>>>> +# garbage collection.
>>>>>> +#
>>>>>> +. ./common/preamble
>>>>>> +_begin_fstest auto quick
>>>>>> +
>>>>>> +_fixed_by_kernel_commit 520760b9f915 \
>>>>>> + "f2fs: optimize representative type determination in GC"
>>>>>> +
>>>>>> +. ./common/filter
>>>>>> +
>>>>>> +_cleanup()
>>>>>> +{
>>>>>> + cd /
>>>>>> + rm -r -f $tmp.*
>>>>>> +}
>>>>>
>>>>> This _cleanup() function is same as default. It can be removed.
>>>>
>>>> Will remove.
>>>>
>>>>>
>>>>>> +
>>>>>> +_require_scratch
>>>>>> +_require_xfs_io_command "pwrite"
>>>>>> +_require_xfs_io_command "truncate"
>>>>>> +_require_command "$F2FS_IO_PROG" f2fs_io
>>>>>> +_require_check_dmesg
>>>>>> +
>>>>>> +# Clear options to avoid interference from external configurations
>>>>>> +export MKFS_OPTIONS=""
>>>>>> +export MOUNT_OPTIONS=""
>>>>>> +
>>>>>> +# Format with 96MB size and 2 segments per section
>>>>>> +_scratch_mkfs_sized $((96 * 1024 * 1024)) "" "-s 2" >> $seqres.full 2>&1
>>>>>> +
>>>>>> +# Mount with mode=lfs
>>>>>> +_scratch_mount -o mode=lfs >> $seqres.full 2>&1
>>>>> ^^^^^^^^^^^^^^^^^^^^
>>>>> It's helpless, due to if _scratch_mount fails, it exit() directly.
>>>>
>>>> Right, will fix.
>>>>
>>>>>
>>>>>> +
>>>>>> +# Create files to fill whole filesystem, then segment type will be
>>>>>> changed to node type
>>>>>> +for ((i=0;i<5120;i++)) do
>>>>>> + touch $SCRATCH_MNT/$i >> $seqres.full 2>&1
>>>>>> +done
>>>>>> +sync
>>>>>> +
>>>>>> +# Remove all files to create free(empty) node segments
>>>>>> +rm -f $SCRATCH_MNT/*
>>>>>> +sync
>>>>>> +
>>>>>> +# Allocate free space so that we have chance to reuse free(empty) node
>>>>>> segments
>>>>>> +$XFS_IO_PROG -f -c "pwrite -b 4k 0 1928k" $SCRATCH_MNT/file >>
>>>>>> $seqres.full 2>&1
>>>>>> +sync
>>>>>> +
>>>>>> +$XFS_IO_PROG -c "truncate 0" $SCRATCH_MNT/file >> $seqres.full 2>&1
>>>>>> +$XFS_IO_PROG -d -c "pwrite -b 4k 0 16M" $SCRATCH_MNT/file >>
>>>>>> $seqres.full 2>&1
>>>>>> +$XFS_IO_PROG -c "truncate 0" $SCRATCH_MNT/file >> $seqres.full 2>&1
>>>>>> +$XFS_IO_PROG -d -c "pwrite -b 4k 0 16M" $SCRATCH_MNT/file >>
>>>>>> $seqres.full 2>&1
>>>>>> +$XFS_IO_PROG -c "truncate 0" $SCRATCH_MNT/file >> $seqres.full 2>&1
>>>>>> +sync
>>>>>> +
>>>>>> +$XFS_IO_PROG -d -c "pwrite -b 4k 0 8M" $SCRATCH_MNT/file >>
>>>>>> $seqres.full 2>&1
>>>>>> +$XFS_IO_PROG -c "truncate 0" $SCRATCH_MNT/file >> $seqres.full 2>&1
>>>>>> +$XFS_IO_PROG -d -c "pwrite -b 4k 0 32K" $SCRATCH_MNT/file >>
>>>>>> $seqres.full 2>&1
>>>>>> +$XFS_IO_PROG -c "truncate 0" $SCRATCH_MNT/file >> $seqres.full 2>&1
>>>>>> +$XFS_IO_PROG -d -c "pwrite -b 4k 0 2M" -c "fsync" $SCRATCH_MNT/file >>
>>>>>> $seqres.full 2>&1
>>>>>> +
>>>>>> +# Shutdown the filesystem without checkpoint
>>>>>> +$F2FS_IO_PROG shutdown 2 $SCRATCH_MNT >> $seqres.full 2>&1
>>>>>
>>>>> I'm wondering if we can have f2fs supporting in common _scratch_shutdown
>>>>> helper :)
>>>>
>>>> I think we can change f2fs testcase to use _scratch_shutdown because the
>>>> definition of
>>>> nologflush shutdown interface is the same as xfs':
>>>>
>>>> /*
>>>> * should be same as XFS_IOC_GOINGDOWN.
>>>> * Flags for going down operation used by FS_IOC_GOINGDOWN
>>>> */
>>>> #define F2FS_IOC_SHUTDOWN _IOR('X', 125, __u32) /* Shutdown */
>>>> #define F2FS_GOING_DOWN_NOSYNC 0x2 /* going down */
>>>>
>>>> #define XFS_IOC_GOINGDOWN _IOR ('X', 125, uint32_t)
>>>> #define XFS_FSOP_GOING_FLAGS_NOLOGFLUSH 0x2 /* don't flush
>>>> log nor data */
>>>>
>>>>>
>>>>>> +
>>>>>> +_scratch_unmount >> $seqres.full 2>&1
>>>>> ^^^^^^^^^^^^^^^^^^^^
>>>>>
>>>>> If unmount fails, how about let it output the errors, to break the golden
>>>>> image?
>>>>
>>>> Yes, it's better.
>>>>
>>>>>
>>>>>> +
>>>>>> +_scratch_mount -o mode=lfs >> $seqres.full 2>&1
>>>>> ^^^^^^^^^^^^^^^^^^^^
>>>>> helpless
>>>>
>>>> Will fix.
>>>>
>>>>>
>>>>>> +
>>>>>> +# Run urgent_gc mode to trigger garbage collection
>>>>>> +dev_name=$(_short_dev $SCRATCH_DEV)
>>>>>> +if [ -f /sys/fs/f2fs/$dev_name/gc_urgent ]; then
>>>>>> + echo 1 > /sys/fs/f2fs/$dev_name/gc_urgent
>>>>>> +fi
>>>>>
>>>>> Hmm... what if there's not /sys/fs/f2fs/$dev_name/gc_urgent? Does it
>>>>> affect the test result?
>>>>>
>>>>> If it does, this's a necessary requirement for this test, we shouldn't
>>>>> ignore it and keep running. Does $F2FS_IO_PROG provide a command to
>>>>> make a force GC? Or we need to check this file and _notrun if it's
>>>>> not existed.
>>>>
>>>> Ah, right, that's good point!
>>>>
>>>> We can use "$F2FS_IO_PROG gc_urgent <dev_name> run 5" instead, it will do
>>>> below commands:
>>>
>>> Great, I just hope the *gc_urgent* isn't a new feature which needs
>>> something likes:
>>
>> I guess it's not a new subcommand for f2fs_io,
>>
>> commit 22d758e2e6af210dc9e6cdf99438f063383ba72f
>> Author: Jaegeuk Kim <[email protected]>
>> Date: Tue Feb 19 19:07:21 2019 -0800
>>
>> f2fs_io: add gc_urgent
>>
>> e.g.,
>> f2fs_io gc_urgent dm-4 [start/end/run] [time in sec]
>>
>> This controls sysfs/gc_urgent to run f2fs_gc urgently.
>>
>> Signed-off-by: Jaegeuk Kim <[email protected]>
>>
>>> `_require_f2fs_io gc_urgent` (there's not _require_f2fs_io:)
>>
>> Agreed, we need to introduce _require_f2fs_io(), let me work on this.
>
> Oh, 2019 was 7 years ago. No one should be complaining about this unless
> they're on a super old downstream f2fs-tools. But anyway, having
> _require_f2fs_io is definitely good for future f2fs testing :)
Yeah, agreed.
>
> Therefore, _require_f2fs_io is not strictly urgent for this patch. It's up to
> you whether to include it now or handle it in a later update.
Let me update a bit later.
>
>>
>>>
>>>> 1. echo 1 > /sys/fs/f2fs/$dev_name/gc_urgent
>>>> 2. sleep 5 seconds
>>>> 3. echo 0 > /sys/fs/f2fs/$dev_name/gc_urgent
>>>
>>> It also depends on the /sys/fs/f2fs/$dev_name/gc_urgent too. So we have to
>>> face
>>> the same question:
>>> If this file doesn't exist, should this test case _notrun?
>>
>> Oh, right, maybe we can introduce _require_f2fs_sysfs() to check whether
>> f2fs kernel
>> module has supported target sysfs node?
>
> There's a _require_fs_sysfs_attr, so...
>
> if you don't care about $SCRATCH_DEV is mounted or not, you can:
> _require_fs_sysfs_attr $TEST_DEV gc_urgent
gc_urgent is not a per-image feature, so above version looks fine.
>
> or after _scratch_mount:
> _require_fs_sysfs_attr $SCRATCH_DEV gc_urgent
>
> Then I think you can either use `$F2FS_IO_PROG gc_urgent`, or if you're
> worried
> about its compatibility, you can:
I'm not worried about the compatibility.
> _set_fs_sysfs_attr $SCRATCH_DEV gc_urgent 1
> sleep 5
Thanks for the suggestion. :)
Let me update w/:
_require_fs_sysfs_attr $TEST_DEV gc_urgent
...
$F2FS_IO_PROG gc_urgent
Thanks,
>
> Thanks,
> Zorro
>
>>
>> Thanks,
>>
>>>
>>> Thanks,
>>> Zorro
>>>
>>>>
>>>>>
>>>>>> +
>>>>>> +# Wait background GC thread to wake up to run and potentially encounter
>>>>>> the inconsistency
>>>>>> +sleep 5
>>>>>
>>>>> Does this sleep try to wait above "echo 1 >
>>>>> /sys/fs/f2fs/$dev_name/gc_urgent"?
>>>>> If so, it makes more sense to move it into the "if-then" logic.
>>>>>
>>>>>> +
>>>>>> +_scratch_unmount >> $seqres.full 2>&1
>>>>> ^^^^^^^^^^^^^^^^^^^^
>>>>> Same as above.
>>>>
>>>> Will fix.
>>>>
>>>>>
>>>>>> +
>>>>>> +# Check whether the dmesg has the warning indicating the bug
>>>>>> +_check_dmesg_for "F2FS-fs \($dev_name\): Inconsistent segment" && \
>>>>>> + _fail "F2FS-fs ($dev_name): Inconsistent segment type detected
>>>>>> in dmesg!"
>>>>>> +
>>>>>> +echo "Silence is golden"
>>>>>> +status=0
>>>>>> +exit
>>>>>
>>>>> We've replaced "status=0;exit;" with "_exit 0".
>>>>
>>>> Will fix.
>>>>
>>>> Thanks,
>>>>
>>>>>
>>>>> Thanks,
>>>>> Zorro
>>>>>
>>>>>> diff --git a/tests/f2fs/025.out b/tests/f2fs/025.out
>>>>>> new file mode 100644
>>>>>> index 000000000..3d70951ef
>>>>>> --- /dev/null
>>>>>> +++ b/tests/f2fs/025.out
>>>>>> @@ -0,0 +1,2 @@
>>>>>> +QA output created by 025
>>>>>> +Silence is golden
>>>>>> --
>>>>>> 2.49.0
>>>>>>
>>>>
>>>>
>>
>>
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
