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:
1. echo 1 > /sys/fs/f2fs/$dev_name/gc_urgent
2. sleep 5 seconds
3. echo 0 > /sys/fs/f2fs/$dev_name/gc_urgent
>
>> +
>> +# 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
