diff mbox series

[3/3] btrfs: test defrag with compressed extents

Message ID 20220128002701.11971-3-wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series [v3,1/3] btrfs: test defrag with regular and preallocated extents | expand

Commit Message

Qu Wenruo Jan. 28, 2022, 12:27 a.m. UTC
There is a long existing bug in btrfs defrag code that it will always
try to defrag compressed extents, even they are already at max capacity.

This will not reduce the number of extents, but only waste IO/CPU.

The kernel fix is titled:

  btrfs: defrag: don't defrag extents which is already at its max capacity

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
Changelog:
v2:
- Use fiemap output to compare the difference
  Now no need to use _get_file_extent_sector() helper at all.

- Remove unnecessary mount options

- Enlarge the write size to 16M
  To be future proof

- Shorten the subject
---
 tests/btrfs/257     | 57 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/257.out |  2 ++
 2 files changed, 59 insertions(+)
 create mode 100755 tests/btrfs/257
 create mode 100644 tests/btrfs/257.out

Comments

Filipe Manana Jan. 28, 2022, 12:20 p.m. UTC | #1
On Fri, Jan 28, 2022 at 08:27:01AM +0800, Qu Wenruo wrote:
> There is a long existing bug in btrfs defrag code that it will always
> try to defrag compressed extents, even they are already at max capacity.
> 
> This will not reduce the number of extents, but only waste IO/CPU.
> 
> The kernel fix is titled:
> 
>   btrfs: defrag: don't defrag extents which is already at its max capacity
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> Changelog:
> v2:
> - Use fiemap output to compare the difference
>   Now no need to use _get_file_extent_sector() helper at all.
> 
> - Remove unnecessary mount options
> 
> - Enlarge the write size to 16M
>   To be future proof
> 
> - Shorten the subject
> ---
>  tests/btrfs/257     | 57 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/257.out |  2 ++
>  2 files changed, 59 insertions(+)
>  create mode 100755 tests/btrfs/257
>  create mode 100644 tests/btrfs/257.out
> 
> diff --git a/tests/btrfs/257 b/tests/btrfs/257
> new file mode 100755
> index 00000000..bacd0c23
> --- /dev/null
> +++ b/tests/btrfs/257
> @@ -0,0 +1,57 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2022 SUSE Linux Products GmbH. All Rights Reserved.
> +#
> +# FS QA Test 257
> +#
> +# Make sure btrfs defrag ioctl won't defrag compressed extents which are already
> +# at their max capacity.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick defrag

Still missing the 'compress' group.

> +
> +# Import common functions.
> +. ./common/filter
> +. ./common/btrfs
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs btrfs
> +_require_scratch
> +
> +_scratch_mkfs >> $seqres.full
> +
> +_scratch_mount -o compress
> +
> +# Btrfs uses 128K as max extent size for compressed extents, this would result
> +# several compressed extents all at their max size
> +$XFS_IO_PROG -f -c "pwrite -S 0xee 0 16m" -c sync \
> +	$SCRATCH_MNT/foobar >> $seqres.full
> +
> +old_csum=$(_md5_checksum $SCRATCH_MNT/foobar)
> +
> +echo "=== File extent layout before defrag ===" >> $seqres.full
> +$XFS_IO_PROG -c "fiemap -v" "$SCRATCH_MNT/foobar" >> $seqres.full
> +$XFS_IO_PROG -c "fiemap -v" "$SCRATCH_MNT/foobar" > $tmp.before
> +
> +$BTRFS_UTIL_PROG filesystem defrag "$SCRATCH_MNT/foobar" >> $seqres.full
> +sync
> +
> +new_csum=$(_md5_checksum $SCRATCH_MNT/foobar)
> +
> +echo "=== File extent layout before defrag ===" >> $seqres.full
> +$XFS_IO_PROG -c "fiemap -v" "$SCRATCH_MNT/foobar" >> $seqres.full
> +$XFS_IO_PROG -c "fiemap -v" "$SCRATCH_MNT/foobar" > $tmp.after
> +
> +if [ $new_csum != $old_csum ]; then
> +	echo "file content changed"
> +fi
> +
> +diff -q $tmp.before $tmp.after || echo "compressed extents get defragged"

get -> got

Like patch 1/3, it can probably be fixed by Eryu when he picks it.

Minor things apart, it looks good, and it fails without the kernel patch
and passes with it, as expected.

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Thanks.

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

Patch

diff --git a/tests/btrfs/257 b/tests/btrfs/257
new file mode 100755
index 00000000..bacd0c23
--- /dev/null
+++ b/tests/btrfs/257
@@ -0,0 +1,57 @@ 
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2022 SUSE Linux Products GmbH. All Rights Reserved.
+#
+# FS QA Test 257
+#
+# Make sure btrfs defrag ioctl won't defrag compressed extents which are already
+# at their max capacity.
+#
+. ./common/preamble
+_begin_fstest auto quick defrag
+
+# Import common functions.
+. ./common/filter
+. ./common/btrfs
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs btrfs
+_require_scratch
+
+_scratch_mkfs >> $seqres.full
+
+_scratch_mount -o compress
+
+# Btrfs uses 128K as max extent size for compressed extents, this would result
+# several compressed extents all at their max size
+$XFS_IO_PROG -f -c "pwrite -S 0xee 0 16m" -c sync \
+	$SCRATCH_MNT/foobar >> $seqres.full
+
+old_csum=$(_md5_checksum $SCRATCH_MNT/foobar)
+
+echo "=== File extent layout before defrag ===" >> $seqres.full
+$XFS_IO_PROG -c "fiemap -v" "$SCRATCH_MNT/foobar" >> $seqres.full
+$XFS_IO_PROG -c "fiemap -v" "$SCRATCH_MNT/foobar" > $tmp.before
+
+$BTRFS_UTIL_PROG filesystem defrag "$SCRATCH_MNT/foobar" >> $seqres.full
+sync
+
+new_csum=$(_md5_checksum $SCRATCH_MNT/foobar)
+
+echo "=== File extent layout before defrag ===" >> $seqres.full
+$XFS_IO_PROG -c "fiemap -v" "$SCRATCH_MNT/foobar" >> $seqres.full
+$XFS_IO_PROG -c "fiemap -v" "$SCRATCH_MNT/foobar" > $tmp.after
+
+if [ $new_csum != $old_csum ]; then
+	echo "file content changed"
+fi
+
+diff -q $tmp.before $tmp.after || echo "compressed extents get defragged"
+
+echo "Silence is golden"
+
+# success, all done
+status=0
+exit
diff --git a/tests/btrfs/257.out b/tests/btrfs/257.out
new file mode 100644
index 00000000..cc3693f3
--- /dev/null
+++ b/tests/btrfs/257.out
@@ -0,0 +1,2 @@ 
+QA output created by 257
+Silence is golden