[PATCH] btrfs: add test to verify the defrag behavior around preallocated extents

[PATCH] btrfs: add test to verify the defrag behavior around preallocated extents

[Qu Wenruo]

Recent v5.16 has some regression around btrfs autodefrag mount option,
and the extra scrutiny around defrag code exposes some questionable
behavior from the old code.

One behavior is to defrag extents along with the next preallocated
extent.

This behavior will cause extra IO and convert all the preallocated
extent to regular zero filled extents, rendering the preallocated extent
useless.

The kernel fix is titled:

  btrfs: defrag: don't try to merge regular extents with preallocated extents

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tests/btrfs/255     | 81 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/255.out |  2 ++
 2 files changed, 83 insertions(+)
 create mode 100755 tests/btrfs/255
 create mode 100644 tests/btrfs/255.out

diff --git a/tests/btrfs/255 b/tests/btrfs/255
new file mode 100755
index 00000000..3d952c1f
--- /dev/null
+++ b/tests/btrfs/255
@@ -0,0 +1,81 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2022 SUSE Linux Products GmbH. All Rights Reserved.
+#
+# FS QA Test 255
+#
+# Make sure btrfs doesn't defrag preallocated extents, nor lone extents
+# before preallocated extents.
+#
+
+. ./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
+
+get_extent_disk_sector()
+{
+	local file=$1
+	local offset=$2
+
+	$XFS_IO_PROG -c "fiemap $offset" "$file" | _filter_xfs_io_fiemap |\
+	       	head -n1 | awk '{print $3}'
+}
+# Needs 4K sectorsize
+_scratch_mkfs -s 4k >> $seqres.full 2>&1
+
+# Need datacow to make the defragged extents to have different bytenr
+_scratch_mount -o datacow
+
+# Create a file with the following layout:
+# 0       4K        8K        16K
+# |<- R ->|<-- Preallocated ->|
+# R is regular extents.
+#
+# In this case it makes no sense to defrag any extent.
+$XFS_IO_PROG -f -c "pwrite 0 4k" -c sync -c "falloc 4k 12k" \
+	"$SCRATCH_MNT/foobar" >> $seqres.full
+
+echo "=== Initial file extent layout ===" >> $seqres.full
+$XFS_IO_PROG -c "fiemap -v" "$SCRATCH_MNT/foobar" >> $seqres.full
+
+# Save the bytenr of both extents
+old_regular=$(get_extent_disk_sector "$SCRATCH_MNT/foobar" 0)
+old_prealloc=$(get_extent_disk_sector "$SCRATCH_MNT/foobar" 4096)
+
+# Now defrag and write the defragged range back to disk
+$BTRFS_UTIL_PROG filesystem defrag "$SCRATCH_MNT/foobar" >> $seqres.full
+sync
+
+echo "=== File extent layout after defrag ===" >> $seqres.full
+$XFS_IO_PROG -c "fiemap -v" "$SCRATCH_MNT/foobar" >> $seqres.full
+
+new_regular=$(get_extent_disk_sector "$SCRATCH_MNT/foobar" 0)
+new_prealloc=$(get_extent_disk_sector "$SCRATCH_MNT/foobar" 4096)
+
+if [ "$old_regular" -ne "$new_regular" ]; then
+	echo "the single lone sector get defragged"
+fi
+if [ "$old_prealloc" -ne "$new_prealloc" ]; then
+	echo "the preallocated extent get defragged"
+fi
+
+echo "Silence is golden"
+
+# success, all done
+status=0
+exit
diff --git a/tests/btrfs/255.out b/tests/btrfs/255.out
new file mode 100644
index 00000000..7eefb828
--- /dev/null
+++ b/tests/btrfs/255.out
@@ -0,0 +1,2 @@
+QA output created by 255
+Silence is golden
-- 
2.34.1

Re: [PATCH] btrfs: add test to verify the defrag behavior around preallocated extents

[Filipe Manana]

On Sun, Jan 23, 2022 at 01:31:33PM +0800, Qu Wenruo wrote:
> Recent v5.16 has some regression around btrfs autodefrag mount option,
> and the extra scrutiny around defrag code exposes some questionable
> behavior from the old code.
> 
> One behavior is to defrag extents along with the next preallocated
> extent.
> 
> This behavior will cause extra IO and convert all the preallocated
> extent to regular zero filled extents, rendering the preallocated extent
> useless.
> 
> The kernel fix is titled:
> 
>   btrfs: defrag: don't try to merge regular extents with preallocated extents
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  tests/btrfs/255     | 81 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/255.out |  2 ++
>  2 files changed, 83 insertions(+)
>  create mode 100755 tests/btrfs/255
>  create mode 100644 tests/btrfs/255.out
> 
> diff --git a/tests/btrfs/255 b/tests/btrfs/255
> new file mode 100755
> index 00000000..3d952c1f
> --- /dev/null
> +++ b/tests/btrfs/255
> @@ -0,0 +1,81 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2022 SUSE Linux Products GmbH. All Rights Reserved.
> +#
> +# FS QA Test 255
> +#
> +# Make sure btrfs doesn't defrag preallocated extents, nor lone extents
> +# before preallocated extents.
> +#
> +
> +. ./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
> +
> +get_extent_disk_sector()
> +{
> +	local file=$1
> +	local offset=$2
> +
> +	$XFS_IO_PROG -c "fiemap $offset" "$file" | _filter_xfs_io_fiemap |\
> +	       	head -n1 | awk '{print $3}'

awk -> $AWK_PROG

> +}
> +# Needs 4K sectorsize
> +_scratch_mkfs -s 4k >> $seqres.full 2>&1

Shouldn't it be a require rule so that it doesn't fail in some weird way
in case it's running a machine with a page size > 4K and on a kernel without
subpage size support?

> +
> +# Need datacow to make the defragged extents to have different bytenr
> +_scratch_mount -o datacow
> +
> +# Create a file with the following layout:
> +# 0       4K        8K        16K
> +# |<- R ->|<-- Preallocated ->|
> +# R is regular extents.
> +#
> +# In this case it makes no sense to defrag any extent.
> +$XFS_IO_PROG -f -c "pwrite 0 4k" -c sync -c "falloc 4k 12k" \
> +	"$SCRATCH_MNT/foobar" >> $seqres.full
> +
> +echo "=== Initial file extent layout ===" >> $seqres.full
> +$XFS_IO_PROG -c "fiemap -v" "$SCRATCH_MNT/foobar" >> $seqres.full
> +
> +# Save the bytenr of both extents
> +old_regular=$(get_extent_disk_sector "$SCRATCH_MNT/foobar" 0)
> +old_prealloc=$(get_extent_disk_sector "$SCRATCH_MNT/foobar" 4096)
> +
> +# Now defrag and write the defragged range back to disk
> +$BTRFS_UTIL_PROG filesystem defrag "$SCRATCH_MNT/foobar" >> $seqres.full
> +sync
> +
> +echo "=== File extent layout after defrag ===" >> $seqres.full
> +$XFS_IO_PROG -c "fiemap -v" "$SCRATCH_MNT/foobar" >> $seqres.full
> +
> +new_regular=$(get_extent_disk_sector "$SCRATCH_MNT/foobar" 0)
> +new_prealloc=$(get_extent_disk_sector "$SCRATCH_MNT/foobar" 4096)
> +
> +if [ "$old_regular" -ne "$new_regular" ]; then
> +	echo "the single lone sector get defragged"

get -> got

> +fi
> +if [ "$old_prealloc" -ne "$new_prealloc" ]; then
> +	echo "the preallocated extent get defragged"

get -> got

Otherwise it looks good, thanks.

> +fi
> +
> +echo "Silence is golden"
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/255.out b/tests/btrfs/255.out
> new file mode 100644
> index 00000000..7eefb828
> --- /dev/null
> +++ b/tests/btrfs/255.out
> @@ -0,0 +1,2 @@
> +QA output created by 255
> +Silence is golden
> -- 
> 2.34.1
>