Re: [PATCH] f2fs/025: test to do sanity check section type correctly in f2fs GC

[Zorro Lang <zlang@kernel.org>]

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 <chao@kernel.org>
> >> ---
> >> 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 <chao@kernel.org>
> >> +#
> >> +# 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:
`_require_f2fs_io gc_urgent` (there's not _require_f2fs_io:)

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

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