Re: [PATCH 1/3] btrfs: exercise NOCOMPRESS range and EOF handling

[Dhruva G <goledhruva@gmail.com>]

On 12-10-2026 03:30, Qu Wenruo wrote:
> 
> 
> 在 2026/10/12 06:10, Dhruva G 写道:
>> Check that a bounded --nocomp request rewrites compressed extents in the
>> selected range while distant untouched regions still point to their
>> original compressed storage.
>> Cover a partial final sector, zero length, a length extending past EOF,
>> start at EOF, and the empty-file no-op. Preserve contents and sizes across
>> the conversion and a remount.
>>
>> Add shared FIEMAP and integrity helpers for the NOCOMPRESS tests. Require
>> full mapping coverage and inspect every overlapping extent so holes and
>> partly converted ranges cannot pass. Include a small ioctl helper to
>> probe kernel support and check errno without btrfs-progs translation.
>>
>> Signed-off-by: Dhruva Gole <goledhruva@gmail.com>
>> ---
>>   .gitignore                |   1 +
>>   common/btrfs_nocomp       | 134 ++++++++++++++++++++++++++++++++++++++
>>   src/Makefile              |   2 +-
>>   src/btrfs_defrag_nocomp.c |  63 ++++++++++++++++++
>>   tests/btrfs/355           |  80 +++++++++++++++++++++++
>>   tests/btrfs/355.out       |   2 +
>>   6 files changed, 281 insertions(+), 1 deletion(-)
>>   create mode 100644 common/btrfs_nocomp
>>   create mode 100644 src/btrfs_defrag_nocomp.c
>>   create mode 100755 tests/btrfs/355
>>   create mode 100644 tests/btrfs/355.out
>>
>> diff --git a/.gitignore b/.gitignore
>> index 0b6b9452..1bfa12ef 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -216,6 +216,7 @@ tags
>>   /src/truncate
>>   /src/t_btrfs_received_uuid_ioctl
>>   /src/btrfs_ioctl
>> +/src/btrfs_defrag_nocomp
>>   
>>   # Symlinked files
>>   /tests/generic/035.out
>> diff --git a/common/btrfs_nocomp b/common/btrfs_nocomp
>> new file mode 100644
>> index 00000000..7e2111e1
>> --- /dev/null
>> +++ b/common/btrfs_nocomp
>> @@ -0,0 +1,134 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) Meta Platforms, Inc. and affiliates.
>> +# Helpers for BTRFS_DEFRAG_RANGE_NOCOMPRESS tests.
> 
> I don't like a dedicated file just do all those.
> Why not put them into common/btrfs?
> 
>> +
>> +_nc_require()
> 
> And "nc" is too generic, _require_btrfs_defrag_nocompress() would make 
> more sense.
> 
> All the namings here look just some temporary scaffolding, not for 
> generic purposes.

Agreed. I'll move the shared helpers into common/btrfs and use more descriptive names in v2.

> 
>> +{
>> +	_require_scratch
>> +	_require_test_program btrfs_defrag_nocomp
>> +	_require_xfs_io_command fiemap ranged
>> +	_require_btrfs_command filesystem defragment --nocomp
>> +	_require_btrfs_no_nodatacow
>> +	_require_btrfs_no_nodatasum
>> +	_require_btrfs_no_autodefrag
>> +	_require_btrfs_support_sectorsize 4096
>> +	_wants_kernel_commit 009b2056cb25 \
>> +		"btrfs: defrag: add flag to force no-compression"
>> +}
>> +
>> +_nc_write()
>> +{
>> +	$XFS_IO_PROG -f -c "pwrite -S $4 -b 128K $2 $3" -c fsync "$1" \
>> +		>> "$seqres.full" 2>&1 || _fail "write failed: $1"
>> +}
>> +
>> +_nc_probe()
>> +{
>> +	local result
>> +
>> +	_nc_write "$SCRATCH_MNT/probe" 0 131072 0x5a
>> +	result=$($here/src/btrfs_defrag_nocomp "$SCRATCH_MNT/probe" 0 131072) ||
>> +		_fail "NOCOMPRESS probe failed"
> 
> Why we need such dedicated probe program while the kernel is already 
> rejecting unknown flags with ENOTTY?
> 
> And I think this probing should also be done inside the 
> _require_btrfs_defrag_nocompress().

Agreed. I'll do this in v2.

> 
>> +	case "$result" in
>> +	OK) ;;
>> +	EOPNOTSUPP|ENOTTY) _notrun "kernel does not support NOCOMPRESS" ;;
>> +	*) _fail "unexpected NOCOMPRESS probe result: $result" ;;
>> +	esac
>> +	rm -f "$SCRATCH_MNT/probe"
>> +}
>> +
>> +_nc_defrag()
>> +{
>> +	local file="${@: -1}"
>> +
>> +	$BTRFS_UTIL_PROG filesystem defragment --nocomp "$@" \
>> +		>> "$seqres.full" 2>&1 || _fail "NOCOMPRESS defrag failed"
>> +	$XFS_IO_PROG -c fsync "$file" >> "$seqres.full" 2>&1 ||
>> +		_fail "fsync after defrag failed"
>> +}
>> +
>> +_nc_map()
>> +{
>> +	$XFS_IO_PROG -c "fiemap -v $2 $3" "$1" > "$tmp.nc-map" \
>> +		2>> "$seqres.full" || _fail "fiemap failed: $1"
>> +	cat "$tmp.nc-map" >> "$seqres.full"
>> +}
>> +
>> +# FIEMAP prints logical offsets in 512-byte units. Check every overlapping
>> +# extent and complete coverage, so a hole or a partly converted range fails.
>> +# The caller supplies byte offsets and lengths, including a partial EOF sector.
>> +_nc_check_range()
> 
> This looks a little overkilled to me.
> > For regular hole/compression flags, what about filtering out the 
> non-hole logical bytenr (e.g. replace them with 'XXX' or 'regular'), 
> then output to the stdout, and use golden output to verify?


For patch 3, the test also needs to distinguish compressed inline extents from compressed non-inline extents.
_filter_fiemap_flags 1 retains the encoded flag, but currently filters out the inline flag.

Would you prefer a small opt-in extension to retain the inline flag, or a test-local filter?
With the extension, existing callers would retain their current output, and the inline test could use:
_filter_fiemap_flags 1 1.

Thanks for reviewing and suggestions!

Regards,
Dhruva Gole