On Fri, Jan 28, 2022 at 12:13 PM Qu Wenruo <quwenruo.btrfs@xxxxxxx> wrote:
>
>
>
> On 2022/1/28 19:57, Filipe Manana wrote:
> > On Fri, Jan 28, 2022 at 08:27:00AM +0800, Qu Wenruo wrote:
> >> In v5.11~v5.15 kernels, there is a regression in autodefrag that if a
> >> cluster (up to 256K in size) has even a single hole, the whole cluster
> >> will be rejected.
> >>
> >> This will greatly reduce the efficiency of autodefrag.
> >>
> >> The behavior is fixed in v5.16 by a full rework, although the rework
> >> itself has other problems, it at least solves the problem.
> >>
> >> Here we add a test case to reproduce the case, where we have a 128K
> >> cluster, the first half is fragmented extents which can be defragged.
> >> The second half is hole.
> >>
> >> Make sure autodefrag can defrag the 64K part.
> >>
> >> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
> >
> > Tried the test, and it succeeds here without the kernel fix applied.
> > I've ran it 10 times, and it always passed without the kernel fix.
>
> Have you tried v5.15 kernels? As v5.16 has the reworked code and will
> always pass.
>
> Or you can try to revert those 3 commits:
>
> c22a3572cbaf ("btrfs: defrag: enable defrag for subpage case")
> c635757365c3 ("btrfs: defrag: remove the old infrastructure")
> 7b508037d4ca ("btrfs: defrag: use defrag_one_cluster() to implement
> btrfs_defrag_file()")
With all those reverted, plus all the recent fixes that went to Linus'
tree, it fails as expected.
I got lost somewhere while navigating the sea of so many recent defrag
patches and versions.
So:
Reviewed-by: Filipe Manana <fdmanana@xxxxxxxx>
Thanks.
>
> That's how I tested the case with the same kernel base without going
> back to v5.15.
>
> And with those 3 commits reverted (aka, my baseline to emulate v5.15),
> it fails like this:
>
> btrfs/256 4s ... - output mismatch (see
> /home/adam/xfstests-dev/results//btrfs/256.out.bad)
> --- tests/btrfs/256.out 2022-01-28 08:19:11.123333302 +0800
> +++ /home/adam/xfstests-dev/results//btrfs/256.out.bad 2022-01-28
> 20:10:18.416666746 +0800
> @@ -1,2 +1,3 @@
> QA output created by 256
> +regular extents didn't get defragged
> Silence is golden
> ...
> (Run 'diff -u /home/adam/xfstests-dev/tests/btrfs/256.out
> /home/adam/xfstests-dev/results//btrfs/256.out.bad' to see the entire diff)
> Ran: btrfs/256
>
> With the fiemap result in the full log:
> === File extent layout before autodefrag ===
> /mnt/scratch/foobar:
> EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS
> 0: [0..31]: 26720..26751 32 0x0
> 1: [32..63]: 26688..26719 32 0x0
> 2: [64..95]: 26656..26687 32 0x0
> 3: [96..127]: 26624..26655 32 0x1
> 4: [128..255]: hole 128
> old md5=9ef8ace32f3b9890cff4fd43699bbd81
> === File extent layout after autodefrag ===
> /mnt/scratch/foobar:
> EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS
> 0: [0..31]: 26720..26751 32 0x0
> 1: [32..63]: 26688..26719 32 0x0
> 2: [64..95]: 26656..26687 32 0x0
> 3: [96..127]: 26624..26655 32 0x1
> 4: [128..255]: hole 128
> new md5=9ef8ace32f3b9890cff4fd43699bbd81
>
> With my POC patch for v5.15 or v5.16 kernel it passes as expected.
>
> Thus that's why there is no fix mentioned in the commit message.
>
> Thanks,
> Qu
>
>
>
> >
> > Thanks.
> >
> >> ---
> >> Changelog:
> >> v2:
> >> - Use the previously define _get_file_extent_sector() helper
> >> This also removed some out-of-sync error messages
> >>
> >> - Trigger autodefrag using commit=1 mount option
> >> No need for special purpose patch any more.
> >>
> >> - Use xfs_io -s to skip several sync calls
> >>
> >> - Shorten the subject of the commit
> >> ---
> >> tests/btrfs/256 | 80 +++++++++++++++++++++++++++++++++++++++++++++
> >> tests/btrfs/256.out | 2 ++
> >> 2 files changed, 82 insertions(+)
> >> create mode 100755 tests/btrfs/256
> >> create mode 100644 tests/btrfs/256.out
> >>
> >> diff --git a/tests/btrfs/256 b/tests/btrfs/256
> >> new file mode 100755
> >> index 00000000..def83a15
> >> --- /dev/null
> >> +++ b/tests/btrfs/256
> >> @@ -0,0 +1,80 @@
> >> +#! /bin/bash
> >> +# SPDX-License-Identifier: GPL-2.0
> >> +# Copyright (C) 2022 SUSE Linux Products GmbH. All Rights Reserved.
> >> +#
> >> +# FS QA Test 256
> >> +#
> >> +# Make sure btrfs auto defrag can properly defrag clusters which has hole
> >> +# in the middle
> >> +#
> >> +. ./common/preamble
> >> +_begin_fstest auto defrag quick
> >> +
> >> +. ./common/btrfs
> >> +. ./common/filter
> >> +
> >> +# real QA test starts here
> >> +
> >> +# Modify as appropriate.
> >> +_supported_fs generic
> >> +_require_scratch
> >> +
> >> +# Needs 4K sectorsize, as larger sectorsize can change the file layout.
> >> +_require_btrfs_support_sectorsize 4096
> >> +
> >> +_scratch_mkfs >> $seqres.full
> >> +
> >> +# Need datacow to show which range is defragged, and we're testing
> >> +# autodefrag
> >> +_scratch_mount -o datacow,autodefrag
> >> +
> >> +# Create a layout where we have fragmented extents at [0, 64k) (sync write in
> >> +# reserve order), then a hole at [64k, 128k)
> >> +$XFS_IO_PROG -f -s -c "pwrite 48k 16k" -c "pwrite 32k 16k" \
> >> + -c "pwrite 16k 16k" -c "pwrite 0 16k" \
> >> + $SCRATCH_MNT/foobar >> $seqres.full
> >> +truncate -s 128k $SCRATCH_MNT/foobar
> >> +
> >> +old_csum=$(_md5_checksum $SCRATCH_MNT/foobar)
> >> +echo "=== File extent layout before autodefrag ===" >> $seqres.full
> >> +$XFS_IO_PROG -c "fiemap -v" "$SCRATCH_MNT/foobar" >> $seqres.full
> >> +echo "old md5=$old_csum" >> $seqres.full
> >> +
> >> +old_regular=$(_get_file_extent_sector "$SCRATCH_MNT/foobar" 0)
> >> +old_hole=$(_get_file_extent_sector "$SCRATCH_MNT/foobar" 64k)
> >> +
> >> +# Now trigger autodefrag, autodefrag is triggered in the cleaner thread,
> >> +# which will be woken up by commit thread
> >> +_scratch_remount commit=1
> >> +sleep 3
> >> +sync
> >> +
> >> +new_csum=$(_md5_checksum $SCRATCH_MNT/foobar)
> >> +new_regular=$(_get_file_extent_sector "$SCRATCH_MNT/foobar" 0)
> >> +new_hole=$(_get_file_extent_sector "$SCRATCH_MNT/foobar" 64k)
> >> +
> >> +echo "=== File extent layout after autodefrag ===" >> $seqres.full
> >> +$XFS_IO_PROG -c "fiemap -v" "$SCRATCH_MNT/foobar" >> $seqres.full
> >> +echo "new md5=$new_csum" >> $seqres.full
> >> +
> >> +# In v5.11~v5.15 kernels, regular extents won't get defragged, and would trigger
> >> +# the following output
> >> +if [ $new_regular == $old_regular ]; then
> >> + echo "regular extents didn't get defragged"
> >> +fi
> >> +
> >> +# In v5.10 and earlier kernel, autodefrag may choose to defrag holes,
> >> +# which should be avoided.
> >> +if [ "$new_hole" != "$old_hole" ]; then
> >> + echo "hole extents got defragged"
> >> +fi
> >> +
> >> +# Defrag should not change file content
> >> +if [ "$new_csum" != "$old_csum" ]; then
> >> + echo "file content changed"
> >> +fi
> >> +
> >> +echo "Silence is golden"
> >> +# success, all done
> >> +status=0
> >> +exit
> >> diff --git a/tests/btrfs/256.out b/tests/btrfs/256.out
> >> new file mode 100644
> >> index 00000000..7ee8e2e5
> >> --- /dev/null
> >> +++ b/tests/btrfs/256.out
> >> @@ -0,0 +1,2 @@
> >> +QA output created by 256
> >> +Silence is golden
> >> --
> >> 2.34.1
> >>
