On Tue, Feb 8, 2022 at 12:00 PM Qu Wenruo <wqu@xxxxxxxx> wrote:
>
> There is a long existing problem that extent_map::generation is not
> populated (thus always 0) if its read from disk.
>
> This can prevent btrfs autodefrag from working as it relies on
> extent_map::generation.
> If it's always 0, then autodefrag will not consider the range as a
> defrag target.
>
> The test case itself will verify the behavior by:
>
> - Create a fragmented file
> By writing backwards with OSYNC
> This will also queue the file for autodefrag.
>
> - Drop all cache
> Including the extent map cache, meaning later read will
> all get extent map by reading from on-disk file extent items.
>
> - Trigger autodefrag and verify the file layout
> If defrag works, the new file layout should differ from the original
> one.
>
> The kernel fix is titled:
>
> btrfs: populate extent_map::generation when reading from disk
>
> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
> ---
> tests/btrfs/259 | 64 +++++++++++++++++++++++++++++++++++++++++++++
> tests/btrfs/259.out | 2 ++
> 2 files changed, 66 insertions(+)
> create mode 100755 tests/btrfs/259
> create mode 100644 tests/btrfs/259.out
>
> diff --git a/tests/btrfs/259 b/tests/btrfs/259
> new file mode 100755
> index 00000000..577e4ce4
> --- /dev/null
> +++ b/tests/btrfs/259
> @@ -0,0 +1,64 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2022 SUSE Linux Products GmbH. All Rights Reserved.
> +#
> +# FS QA Test 259
> +#
> +# Make sure autodefrag can still defrag the file even their extent maps are
> +# read from disk
> +#
> +. ./common/preamble
> +_begin_fstest auto quick defrag
> +
> +# Override the default cleanup function.
> +# _cleanup()
> +# {
> +# cd /
> +# rm -r -f $tmp.*
> +# }
> +
> +# Import common functions.
> +# . ./common/filter
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs btrfs
> +_require_scratch
> +
> +# Need 4K sectorsize, as the autodefrag threshold is only 64K,
> +# thus 64K sectorsize will not work.
> +_require_btrfs_support_sectorsize 4096
Missing a:
_require_xfs_io_command fiemap
> +_scratch_mkfs -s 4k >> $seqres.full
> +_scratch_mount -o datacow,autodefrag
> +
> +# Create fragmented write
> +$XFS_IO_PROG -f -s -c "pwrite 24k 8k" -c "pwrite 16k 8k" \
> + -c "pwrite 8k 8k" -c "pwrite 0 8k" \
> + "$SCRATCH_MNT/foobar" >> $seqres.full
> +sync
A comment on why this sync is needed would be good to have.
It may be confusing to the reader since we were doing synchronous writes before.
> +
> +echo "=== Before autodefrag ===" >> $seqres.full
> +$XFS_IO_PROG -c "fiemap -v" "$SCRATCH_MNT/foobar" >> $tmp.before
> +cat $tmp.before >> $seqres.full
> +
> +# Drop the cache (including extent map cache per-inode)
> +echo 3 > /proc/sys/vm/drop_caches
> +
> +# Now trigger autodefrag
A bit more explanation would be useful.
Set the commit interval to 1 second, so that 1 second after the
remount the transaction kthread runs
and wakes up the cleaner kthread, which in turn will run autodefrag.
> +_scratch_remount commit=1
> +sleep 3
> +sync
This sync is useless, so it should go away.
Otherwise, it looks good and the test works as expected.
Thanks for doing it.
> +
> +echo "=== After autodefrag ===" >> $seqres.full
> +$XFS_IO_PROG -c "fiemap -v" "$SCRATCH_MNT/foobar" >> $tmp.after
> +cat $tmp.after >> $seqres.full
> +
> +# The layout should differ if autodefrag is working
> +diff $tmp.before $tmp.after > /dev/null && echo "autodefrag didn't defrag the file"
> +
> +echo "Silence is golden"
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/259.out b/tests/btrfs/259.out
> new file mode 100644
> index 00000000..bfbd2dea
> --- /dev/null
> +++ b/tests/btrfs/259.out
> @@ -0,0 +1,2 @@
> +QA output created by 259
> +Silence is golden
> --
> 2.34.1
>
