Re: [PATCH v2 1/2] btrfs/076: support smaller extent size limit

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



On Fri, Sep 15, 2023 at 8:28 AM Naohiro Aota <naohiro.aota@xxxxxxx> wrote:
>
> Running btrfs/076 on a zoned null_blk device will fail with the following error.
>
>   - output mismatch (see /host/results/btrfs/076.out.bad)
>       --- tests/btrfs/076.out     2021-02-05 01:44:20.000000000 +0000
>       +++ /host/results/btrfs/076.out.bad 2023-09-15 01:49:36.000000000 +0000
>       @@ -1,3 +1,3 @@
>        QA output created by 076
>       -80
>       -80
>       +83
>       +83
>       ...
>
> This is because the default value of zone_append_max_bytes is 127.5 KB
> which is smaller than BTRFS_MAX_UNCOMPRESSED (128K). So, the extent size is
> limited to 126976 (= ROUND_DOWN(127.5K, 4096)), which makes the number of
> extents larger, and fails the test.
>
> Instead of hard-coding the number of extents, we can calculate it using the
> max extent size of an extent. It is limited by either
> BTRFS_MAX_UNCOMPRESSED or zone_append_max_bytes.
>
> Signed-off-by: Naohiro Aota <naohiro.aota@xxxxxxx>

Looks good,

Reviewed-by: Filipe Manana <fdmanana@xxxxxxxx>

Just two minor comments below.

> ---
>  tests/btrfs/076     | 23 +++++++++++++++++++++--
>  tests/btrfs/076.out |  3 +--
>  2 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/tests/btrfs/076 b/tests/btrfs/076
> index 89e9672d09e2..a5cc3eb96b2f 100755
> --- a/tests/btrfs/076
> +++ b/tests/btrfs/076
> @@ -28,13 +28,28 @@ _supported_fs btrfs
>  _require_test
>  _require_scratch
>
> +# An extent size can be up to BTRFS_MAX_UNCOMPRESSED
> +max_extent_size=$(( 128 << 10 ))

For consistency with every other test and common files, using 128 *
1024 would be perhaps better. I certainly find it easier to read, but
that's a personal preference only.

> +if _scratch_btrfs_is_zoned; then
> +       zone_append_max=$(cat "/sys/block/$(_short_dev $SCRATCH_DEV)/queue/zone_append_max_bytes")
> +       if [[ $zone_append_max -gt 0 && $zone_append_max -lt $max_extent_size ]]; then
> +               # Round down to PAGE_SIZE
> +               max_extent_size=$(( $zone_append_max / 4096 * 4096 ))
> +       fi
> +fi
> +file_size=$(( 10 << 20 ))

And this one it's even less immediate to understand, having 1 * 1024 *
1024 would make it much more easier to read.

Thanks.

> +expect=$(( (file_size + max_extent_size - 1) / max_extent_size ))
> +
>  _scratch_mkfs >> $seqres.full 2>&1
>  _scratch_mount "-o compress=lzo"
>
>  $XFS_IO_PROG -f -c "pwrite 0 10M" -c "fsync" \
>         $SCRATCH_MNT/data >> $seqres.full 2>&1
>
> -_extent_count $SCRATCH_MNT/data
> +res=$(_extent_count $SCRATCH_MNT/data)
> +if [[ $res -ne $expect ]]; then
> +       _fail "Expected $expect extents, got $res"
> +fi
>
>  $XFS_IO_PROG -f -c "pwrite 0 $((4096*33))" -c "fsync" \
>         $SCRATCH_MNT/data >> $seqres.full 2>&1
> @@ -42,7 +57,11 @@ $XFS_IO_PROG -f -c "pwrite 0 $((4096*33))" -c "fsync" \
>  $XFS_IO_PROG -f -c "pwrite 0 10M" -c "fsync" \
>         $SCRATCH_MNT/data >> $seqres.full 2>&1
>
> -_extent_count $SCRATCH_MNT/data
> +res=$(_extent_count $SCRATCH_MNT/data)
> +if [[ $res -ne $expect ]]; then
> +       _fail "Expected $expect extents, got $res"
> +fi
>
> +echo "Silence is golden"
>  status=0
>  exit
> diff --git a/tests/btrfs/076.out b/tests/btrfs/076.out
> index b99f7eb10a16..248e095d91af 100644
> --- a/tests/btrfs/076.out
> +++ b/tests/btrfs/076.out
> @@ -1,3 +1,2 @@
>  QA output created by 076
> -80
> -80
> +Silence is golden
> --
> 2.42.0
>




[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux