From: David Disseldorp <ddiss@suse.de>
To: fdmanana@kernel.org
Cc: fstests@vger.kernel.org, linux-btrfs@vger.kernel.org,
Filipe Manana <fdmanana@suse.com>
Subject: Re: [PATCH] btrfs: test that we don't create pointless compressed inline extents
Date: Tue, 15 Sep 2026 21:44:31 +1000 [thread overview]
Message-ID: <20260915214431.72728a7a.ddiss@suse.de> (raw)
In-Reply-To: <83e3a8678c5eafb1cfcf10741a53b6e586cd72d8.1789408399.git.fdmanana@suse.com>
On Mon, 14 Sep 2026 18:54:23 +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> Verify that we don't create compressed inline extents when the compressed
> data size is not smaller than the uncompressed data size.
>
> This exercises a regression introduced in kernel 7.1 and that is fixed by
> the following patch:
>
> "btrfs: fix creation of compressed inline extents that don't save space"
>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
Looks good and works for me:
Reviewed-by: David Disseldorp <ddiss@suse.de>
A couple of minor nits below...
> ---
> tests/btrfs/354 | 44 ++++++++++++++++++++++++++++++++++++++++++++
> tests/btrfs/354.out | 2 ++
> 2 files changed, 46 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..801dc587
> --- /dev/null
> +++ b/tests/btrfs/354
> @@ -0,0 +1,44 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2026 SUSE S.A. All Rights Reserved.
> +#
> +# FS QA Test 354
> +#
> +# Verify that we don't create compressed inline extents when the compressed
> +# data size is not smaller than the uncompressed data size.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick compress
> +
> +. ./common/filter
> +
> +_fixed_by_kernel_commit xxxxxxxxxxxx \
> + "btrfs: fix creation of compressed inline extents that don't save space"
> +
> +_require_scratch
> +_require_btrfs_command inspect-internal dump-tree
> +
> +algo_list=($(_btrfs_compression_algos))
> +for algo in ${algo_list[@]}; do
> + _scratch_mkfs >>$seqres.full 2>&1 || _fail "mkfs failed"
> + _scratch_mount "-o compress=$algo"
> +
> + echo 'a file with test content' > $SCRATCH_MNT/foobar
> + _scratch_unmount
> + _check_scratch_fs
> +
> + extent_size=$($BTRFS_UTIL_PROG inspect-internal dump-tree -t 5 $SCRATCH_DEV | \
> + grep 'inline extent' | \
> + $AWK_PROG '{ for (i = 1; i < NF; i++) if ($i ~ /^size$/) print $(i + 1) }')
> + ram_size=$($BTRFS_UTIL_PROG inspect-internal dump-tree -t 5 $SCRATCH_DEV | \
> + grep 'inline extent' | \
> + $AWK_PROG '{ for (i = 1; i < NF; i++) if ($i ~ /^ram_bytes$/) print $(i + 1) }')
> +
> + if [ "$extent_size" -gt "$ram_size" ]; then
> + echo "inline extent size ($extent_size) greater than ram_size ($ram_size) for algo $algo"
> + fi
It might be a little nicer to avoid multiple dumps and do the check in
awk. The following works for me, feel free to squash in...
diff --git a/tests/btrfs/354 b/tests/btrfs/354
index 801dc587..e744b945 100755
--- a/tests/btrfs/354
+++ b/tests/btrfs/354
@@ -27,18 +27,20 @@ for algo in ${algo_list[@]}; do
_scratch_unmount
_check_scratch_fs
- extent_size=$($BTRFS_UTIL_PROG inspect-internal dump-tree -t 5 $SCRATCH_DEV | \
- grep 'inline extent' | \
- $AWK_PROG '{ for (i = 1; i < NF; i++) if ($i ~ /^size$/) print $(i + 1) }')
- ram_size=$($BTRFS_UTIL_PROG inspect-internal dump-tree -t 5 $SCRATCH_DEV | \
- grep 'inline extent' | \
- $AWK_PROG '{ for (i = 1; i < NF; i++) if ($i ~ /^ram_bytes$/) print $(i + 1) }')
-
- if [ "$extent_size" -gt "$ram_size" ]; then
- echo "inline extent size ($extent_size) greater than ram_size ($ram_size) for algo $algo"
- fi
+ $BTRFS_UTIL_PROG inspect-internal dump-tree -t 5 $SCRATCH_DEV | \
+ tee -a $seqres.full | \
+ $AWK_PROG -v algo="$algo" '
+ /inline extent/ {
+ for (i = 1; i < NF; i++) {
+ if ($i ~ /^size$/)
+ extent_size = $(i + 1)
+ if ($i ~ /^ram_bytes$/)
+ ram_size = $(i + 1)
+ }
+ if (extent_size > ram_size)
+ printf "inline extent size (%d) greater than ram_size (%d) for algo %s\n", extent_size, ram_size, algo
+ }'
> +done
> +
> +# success, all done
Delete the line above, as we get here on failure too.
> +echo "Silence is golden"
> +_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
next prev parent reply other threads:[~2026-09-15 11:45 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-14 17:54 [PATCH] btrfs: test that we don't create pointless compressed inline extents fdmanana
2026-09-15 11:44 ` David Disseldorp [this message]
2026-09-15 15:05 ` Filipe Manana
2026-09-15 15:05 ` [PATCH v2] " fdmanana
2026-09-15 22:16 ` Qu Wenruo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260915214431.72728a7a.ddiss@suse.de \
--to=ddiss@suse.de \
--cc=fdmanana@kernel.org \
--cc=fdmanana@suse.com \
--cc=fstests@vger.kernel.org \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox