Linux Btrfs filesystem development
 help / color / mirror / Atom feed
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


  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