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.
>
>> 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?
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
