Linux Btrfs filesystem development
 help / color / mirror / Atom feed
* [PATCH] btrfs: test defrag --nocomp converts comp extents to uncomp
@ 2026-09-19 18:22 Dhruva G
  2026-09-19 23:35 ` Qu Wenruo
  0 siblings, 1 reply; 2+ messages in thread
From: Dhruva G @ 2026-09-19 18:22 UTC (permalink / raw)
  To: linux-btrfs, kernel-team, fstests; +Cc: Boris Burkov, Anand Jain, goledhruva

"btrfs filesystem defragment --nocomp" rewrites already compressed file
data as uncompressed data. The kernel flag was added by commit
009b2056cb25 ("btrfs: defrag: add flag to force no-compression") in
v6.17, and the userspace option shipped in btrfs-progs v6.16. There is
no test for it.

Add a test that writes compressible data on a filesystem mounted with
compression enabled, runs defragment with --nocomp, and verifies that no
extent remains compressed while the file contents are unchanged across a
remount. The filesystem stays mounted with compress=zlib for the whole
test, which also covers that the defrag ioctl takes precedence over mount
options.

Regular defrag deliberately leaves maximum-sized compressed extents alone,
as asserted by btrfs/259. A --nocomp implementation that silently behaves
like regular defrag therefore requires a dedicated test.

Check support in two stages: first require btrfs-progs to advertise
--nocomp, then invoke it on a 10-byte file to verify that the kernel
accepts the flag. Kernels with the unknown-flag validation from commit
173431b274a9 return EOPNOTSUPP when NOCOMPRESS is unavailable. Kernels
predating that validation may silently ignore the flag and fail the
semantic checks instead of skipping.

Signed-off-by: Dhruva Gole <goledhruva@gmail.com>
---

Logs/ compatibility matrix:
https://gist.github.com/DhruvaG2000/62101a37f5e53e41883b4b3181250724

---

 tests/btrfs/354     | 92 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/354.out |  2 +
 2 files changed, 94 insertions(+)
 create mode 100755 tests/btrfs/354
 create mode 100644 tests/btrfs/354.out

diff --git a/tests/btrfs/354 b/tests/btrfs/354
new file mode 100755
index 00000000..fae8f47e
--- /dev/null
+++ b/tests/btrfs/354
@@ -0,0 +1,92 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2026 Meta Platforms, Inc.  All Rights Reserved.
+#
+# FS QA Test 354
+#
+# Make sure "btrfs filesystem defragment --nocomp" rewrites compressed extents
+# as uncompressed ones, even though the filesystem is mounted with compression
+# enabled, and without changing the file contents.
+#
+# Note that regular defrag deliberately skips compressed extents that are
+# already at their maximum size (see btrfs/259), so a broken --nocomp that
+# simply does nothing would go unnoticed without this test.
+#
+. ./common/preamble
+_begin_fstest auto quick defrag compress
+
+# real QA test starts here
+
+_require_scratch
+_require_btrfs_command inspect-internal dump-tree
+_require_btrfs_command filesystem defragment --nocomp
+
+_wants_kernel_commit 009b2056cb25 \
+	"btrfs: defrag: add flag to force no-compression"
+
+# Same helper as btrfs/260: btrfs specific extent attributes are only
+# available from dump-tree, not from fiemap.
+#
+# NOTE: At the moment this only works if the file is on a filesystem on top of
+#       the scratch device and the file is in the default subvolume (tree id 5).
+check_file_extent()
+{
+	local file="$1"
+	local offset="$2"
+	local expected="$3"
+	local ino=$(stat -c "%i" "$file")
+
+	echo "=== file extent at file '$file' offset $offset ===" >> $seqres.full
+	$BTRFS_UTIL_PROG inspect-internal dump-tree -t 5 $SCRATCH_DEV |\
+		grep -A4 "($ino EXTENT_DATA $offset)" > $tmp.output
+	cat $tmp.output >> $seqres.full
+	grep -q "$expected" $tmp.output ||\
+		echo "file \"$file\" offset $offset doesn't have expected string" \
+		     "\"$expected\""
+}
+
+_scratch_mkfs >> $seqres.full 2>&1 || _fail "mkfs failed"
+_scratch_mount -o compress=zlib
+
+# Probe kernel support separately from the userspace check above. Kernels
+# without the flag reject it with EOPNOTSUPP when unknown-flag validation is
+# available.
+$XFS_IO_PROG -f -c "pwrite 0 10" "$SCRATCH_MNT/probe" >> $seqres.full
+$BTRFS_UTIL_PROG filesystem defragment --nocomp "$SCRATCH_MNT/probe" \
+	>> $seqres.full 2>&1 || _notrun "defrag --nocomp not supported"
+
+# Default xfs_io pattern is highly compressible, so every extent of this file
+# ends up compressed and at the 128K maximum compressed extent size.
+$XFS_IO_PROG -f -c "pwrite 0 1m" "$SCRATCH_MNT/foo" >> $seqres.full
+$BTRFS_UTIL_PROG filesystem sync $SCRATCH_MNT
+
+# Should be zlib compressed
+check_file_extent "$SCRATCH_MNT/foo" 0 "compression 1"
+
+csum_before=$(_md5_checksum "$SCRATCH_MNT/foo")
+
+$BTRFS_UTIL_PROG filesystem defragment --nocomp "$SCRATCH_MNT/foo" \
+	>> $seqres.full 2>&1
+
+# Need to commit the transaction or dump-tree won't grab the new
+# metadata on-disk.
+$BTRFS_UTIL_PROG filesystem sync $SCRATCH_MNT
+
+# Should no longer be compressed, despite the compress mount option
+check_file_extent "$SCRATCH_MNT/foo" 0 "compression 0"
+
+# No extent of this inode may remain compressed
+ino=$(stat -c "%i" "$SCRATCH_MNT/foo")
+$BTRFS_UTIL_PROG inspect-internal dump-tree -t 5 $SCRATCH_DEV |
+	grep -A4 "($ino EXTENT_DATA " | grep -q "compression [^0]" &&
+	echo "file still has compressed extents after --nocomp defrag"
+
+# Contents must survive the rewrite, read back from disk and not page cache
+_scratch_cycle_mount "compress=zlib"
+csum_after=$(_md5_checksum "$SCRATCH_MNT/foo")
+[ "$csum_before" = "$csum_after" ] || echo "file content changed"
+
+echo "Silence is golden"
+
+# success, all done
+_exit 0
diff --git a/tests/btrfs/354.out b/tests/btrfs/354.out
new file mode 100644
index 00000000..8bc7ecf6
--- /dev/null
+++ b/tests/btrfs/354.out
@@ -0,0 +1,2 @@
+QA output created by 354
+Silence is golden
-- 
2.43.0

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] btrfs: test defrag --nocomp converts comp extents to uncomp
  2026-09-19 18:22 [PATCH] btrfs: test defrag --nocomp converts comp extents to uncomp Dhruva G
@ 2026-09-19 23:35 ` Qu Wenruo
  0 siblings, 0 replies; 2+ messages in thread
From: Qu Wenruo @ 2026-09-19 23:35 UTC (permalink / raw)
  To: Dhruva G, linux-btrfs, kernel-team, fstests; +Cc: Boris Burkov, Anand Jain



在 2026/9/20 03:52, Dhruva G 写道:
> "btrfs filesystem defragment --nocomp" rewrites already compressed file
> data as uncompressed data. The kernel flag was added by commit
> 009b2056cb25 ("btrfs: defrag: add flag to force no-compression") in
> v6.17, and the userspace option shipped in btrfs-progs v6.16. There is
> no test for it.
> 
> Add a test that writes compressible data on a filesystem mounted with
> compression enabled, runs defragment with --nocomp, and verifies that no
> extent remains compressed while the file contents are unchanged across a
> remount. The filesystem stays mounted with compress=zlib for the whole
> test, which also covers that the defrag ioctl takes precedence over mount
> options.
> 
> Regular defrag deliberately leaves maximum-sized compressed extents alone,
> as asserted by btrfs/259. A --nocomp implementation that silently behaves
> like regular defrag therefore requires a dedicated test.
> 
> Check support in two stages: first require btrfs-progs to advertise
> --nocomp, then invoke it on a 10-byte file to verify that the kernel
> accepts the flag. Kernels with the unknown-flag validation from commit
> 173431b274a9 return EOPNOTSUPP when NOCOMPRESS is unavailable. Kernels
> predating that validation may silently ignore the flag and fail the
> semantic checks instead of skipping.
> 
> Signed-off-by: Dhruva Gole <goledhruva@gmail.com>
> ---
> 
> Logs/ compatibility matrix:
> https://gist.github.com/DhruvaG2000/62101a37f5e53e41883b4b3181250724
> 
> ---
> 
>   tests/btrfs/354     | 92 +++++++++++++++++++++++++++++++++++++++++++++
>   tests/btrfs/354.out |  2 +
>   2 files changed, 94 insertions(+)
>   create mode 100755 tests/btrfs/354
>   create mode 100644 tests/btrfs/354.out
> 
> diff --git a/tests/btrfs/354 b/tests/btrfs/354
> new file mode 100755
> index 00000000..fae8f47e
> --- /dev/null
> +++ b/tests/btrfs/354
> @@ -0,0 +1,92 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2026 Meta Platforms, Inc.  All Rights Reserved.
> +#
> +# FS QA Test 354
> +#
> +# Make sure "btrfs filesystem defragment --nocomp" rewrites compressed extents
> +# as uncompressed ones, even though the filesystem is mounted with compression
> +# enabled, and without changing the file contents.
> +#
> +# Note that regular defrag deliberately skips compressed extents that are
> +# already at their maximum size (see btrfs/259), so a broken --nocomp that
> +# simply does nothing would go unnoticed without this test.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick defrag compress
> +
> +# real QA test starts here
> +
> +_require_scratch
> +_require_btrfs_command inspect-internal dump-tree
> +_require_btrfs_command filesystem defragment --nocomp
> +
> +_wants_kernel_commit 009b2056cb25 \
> +	"btrfs: defrag: add flag to force no-compression"
> +
> +# Same helper as btrfs/260: btrfs specific extent attributes are only
> +# available from dump-tree, not from fiemap.
> +#
> +# NOTE: At the moment this only works if the file is on a filesystem on top of
> +#       the scratch device and the file is in the default subvolume (tree id 5).
> +check_file_extent()
> +{
> +	local file="$1"
> +	local offset="$2"
> +	local expected="$3"
> +	local ino=$(stat -c "%i" "$file")
> +
> +	echo "=== file extent at file '$file' offset $offset ===" >> $seqres.full
> +	$BTRFS_UTIL_PROG inspect-internal dump-tree -t 5 $SCRATCH_DEV |\
> +		grep -A4 "($ino EXTENT_DATA $offset)" > $tmp.output
> +	cat $tmp.output >> $seqres.full
> +	grep -q "$expected" $tmp.output ||\
> +		echo "file \"$file\" offset $offset doesn't have expected string" \
> +		     "\"$expected\""

This is a little overkilled, if your objective is only to check if the 
file extent is compressed or not, filefrag or xfs_io "fiemap" can both 
provide a flag (0x8, encoded) showing if an extent is compressed or not.

> +}
> +
> +_scratch_mkfs >> $seqres.full 2>&1 || _fail "mkfs failed"
> +_scratch_mount -o compress=zlib
> +
> +# Probe kernel support separately from the userspace check above. Kernels
> +# without the flag reject it with EOPNOTSUPP when unknown-flag validation is
> +# available.
> +$XFS_IO_PROG -f -c "pwrite 0 10" "$SCRATCH_MNT/probe" >> $seqres.full
> +$BTRFS_UTIL_PROG filesystem defragment --nocomp "$SCRATCH_MNT/probe" \
> +	>> $seqres.full 2>&1 || _notrun "defrag --nocomp not supported"
> +
> +# Default xfs_io pattern is highly compressible, so every extent of this file
> +# ends up compressed and at the 128K maximum compressed extent size.
> +$XFS_IO_PROG -f -c "pwrite 0 1m" "$SCRATCH_MNT/foo" >> $seqres.full

Just a small nitpick, the default block size is 4K, meaning under memory 
pressure the writeback can be triggered halfway, resulting extents 
smaller than 128K, better to use 128K block size just in case.

This should be very rare though.

> +$BTRFS_UTIL_PROG filesystem sync $SCRATCH_MNT

You can merge a regular -c "sync" to above command.

> +
> +# Should be zlib compressed
> +check_file_extent "$SCRATCH_MNT/foo" 0 "compression 1"
> +
> +csum_before=$(_md5_checksum "$SCRATCH_MNT/foo")
> +
> +$BTRFS_UTIL_PROG filesystem defragment --nocomp "$SCRATCH_MNT/foo" \
> +	>> $seqres.full 2>&1
> +
> +# Need to commit the transaction or dump-tree won't grab the new
> +# metadata on-disk.
> +$BTRFS_UTIL_PROG filesystem sync $SCRATCH_MNT
> +
> +# Should no longer be compressed, despite the compress mount option
> +check_file_extent "$SCRATCH_MNT/foo" 0 "compression 0"
> +
> +# No extent of this inode may remain compressed
> +ino=$(stat -c "%i" "$SCRATCH_MNT/foo")
> +$BTRFS_UTIL_PROG inspect-internal dump-tree -t 5 $SCRATCH_DEV |
> +	grep -A4 "($ino EXTENT_DATA " | grep -q "compression [^0]" &&
> +	echo "file still has compressed extents after --nocomp defrag"

Again, regular fiemap can tell if the extent is compressed or not.

Otherwise looks good to me.

Thanks,
Qu

> +
> +# Contents must survive the rewrite, read back from disk and not page cache
> +_scratch_cycle_mount "compress=zlib"
> +csum_after=$(_md5_checksum "$SCRATCH_MNT/foo")
> +[ "$csum_before" = "$csum_after" ] || echo "file content changed"
> +
> +echo "Silence is golden"
> +
> +# success, all done
> +_exit 0
> diff --git a/tests/btrfs/354.out b/tests/btrfs/354.out
> new file mode 100644
> index 00000000..8bc7ecf6
> --- /dev/null
> +++ b/tests/btrfs/354.out
> @@ -0,0 +1,2 @@
> +QA output created by 354
> +Silence is golden


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-09-19 23:35 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-19 18:22 [PATCH] btrfs: test defrag --nocomp converts comp extents to uncomp Dhruva G
2026-09-19 23:35 ` Qu Wenruo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox