Re: [PATCH] btrfs: test defrag --nocomp converts comp extents to uncomp

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 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