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 Thu, Sep 21, 2023 at 07:41:19AM +0000, Naohiro Aota wrote:
> On Fri, Sep 15, 2023 at 10:16:32AM +0100, Filipe Manana wrote:
> > 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.
> 
> Agreed. I'll use 1024 and repost. Thanks.

I've changed that part when I merged this patch (haven't pushed), so you
don't need to send this patch again, save that time :)

Thanks,
Zorro




[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