linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] generic: test cloning large exents to a file with many small extents
@ 2019-06-27 17:00 fdmanana
  2019-06-27 20:28 ` Darrick J. Wong
  2019-06-28 22:08 ` [PATCH v2] " fdmanana
  0 siblings, 2 replies; 8+ messages in thread
From: fdmanana @ 2019-06-27 17:00 UTC (permalink / raw)
  To: fstests; +Cc: linux-btrfs, Filipe Manana

From: Filipe Manana <fdmanana@suse.com>

Test that if we clone a file with some large extents into a file that has
many small extents, when the fs is nearly full, the clone operation does
not fail and produces the correct result.

This is motivated by a bug found in btrfs wich is fixed by the following
patches for the linux kernel:

 [PATCH 1/2] Btrfs: factor out extent dropping code from hole punch handler
 [PATCH 2/2] Btrfs: fix ENOSPC errors, leading to transaction aborts, when
             cloning extents

The test currently passes on xfs.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 tests/generic/558     | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/558.out |  5 ++++
 tests/generic/group   |  1 +
 3 files changed, 81 insertions(+)
 create mode 100755 tests/generic/558
 create mode 100644 tests/generic/558.out

diff --git a/tests/generic/558 b/tests/generic/558
new file mode 100755
index 00000000..ee16cdf7
--- /dev/null
+++ b/tests/generic/558
@@ -0,0 +1,75 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2019 SUSE Linux Products GmbH. All Rights Reserved.
+#
+# FSQA Test No. 558
+#
+# Test that if we clone a file with some large extents into a file that has
+# many small extents, when the fs is nearly full, the clone operation does
+# not fail and produces the correct result.
+#
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/reflink
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_scratch_reflink
+
+rm -f $seqres.full
+
+_scratch_mkfs_sized $((512 * 1024 * 1024)) >>$seqres.full 2>&1
+_scratch_mount
+
+file_size=$(( 128 * 1024 * 1024 )) # 128Mb
+extent_size=4096
+num_extents=$(( $file_size / $extent_size ))
+
+# Create a file with many small extents.
+for ((i = 0; i < $num_extents; i++)); do
+	offset=$(( $i * $extent_size ))
+	$XFS_IO_PROG -f -s -c "pwrite -S 0xe5 $offset $extent_size" \
+		$SCRATCH_MNT/foo >>/dev/null
+done
+
+# Create file bar with the same size that file foo has but with large extents.
+$XFS_IO_PROG -f -c "pwrite -S 0xc7 -b $file_size 0 $file_size" \
+	$SCRATCH_MNT/bar >>/dev/null
+
+# Fill the fs (for btrfs we are interested in filling all unallocated space
+# and most of the existing metadata block group(s), so that after this there
+# will be no unallocated space and metadata space will be mostly full but with
+# more than enough free space for the clone operation below to succeed).
+i=1
+while true; do
+	$XFS_IO_PROG -f -c "pwrite 0 2K" $SCRATCH_MNT/filler_$i &> /dev/null
+	[ $? -ne 0 ] && break
+	i=$(( i + 1 ))
+done
+
+# Now clone file bar into file foo. This is supposed to succeed and not fail
+# with ENOSPC for example.
+$XFS_IO_PROG -c "reflink $SCRATCH_MNT/bar" $SCRATCH_MNT/foo >>/dev/null
+
+_scratch_remount
+
+echo "File foo data after cloning and remount:"
+od -A d -t x1 $SCRATCH_MNT/foo
+
+status=0
+exit
diff --git a/tests/generic/558.out b/tests/generic/558.out
new file mode 100644
index 00000000..d1e8e70f
--- /dev/null
+++ b/tests/generic/558.out
@@ -0,0 +1,5 @@
+QA output created by 558
+File foo data after cloning and remount:
+0000000 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7
+*
+134217728
diff --git a/tests/generic/group b/tests/generic/group
index 543c0627..c06c1cd1 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -560,3 +560,4 @@
 555 auto quick cap
 556 auto quick casefold
 557 auto quick log
+558 auto clone
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] generic: test cloning large exents to a file with many small extents
  2019-06-27 17:00 [PATCH] generic: test cloning large exents to a file with many small extents fdmanana
@ 2019-06-27 20:28 ` Darrick J. Wong
  2019-06-27 21:47   ` Filipe Manana
  2019-06-28 22:08 ` [PATCH v2] " fdmanana
  1 sibling, 1 reply; 8+ messages in thread
From: Darrick J. Wong @ 2019-06-27 20:28 UTC (permalink / raw)
  To: fdmanana; +Cc: fstests, linux-btrfs, Filipe Manana

On Thu, Jun 27, 2019 at 06:00:30PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Test that if we clone a file with some large extents into a file that has
> many small extents, when the fs is nearly full, the clone operation does
> not fail and produces the correct result.
> 
> This is motivated by a bug found in btrfs wich is fixed by the following
> patches for the linux kernel:
> 
>  [PATCH 1/2] Btrfs: factor out extent dropping code from hole punch handler
>  [PATCH 2/2] Btrfs: fix ENOSPC errors, leading to transaction aborts, when
>              cloning extents
> 
> The test currently passes on xfs.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
>  tests/generic/558     | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/558.out |  5 ++++
>  tests/generic/group   |  1 +
>  3 files changed, 81 insertions(+)
>  create mode 100755 tests/generic/558
>  create mode 100644 tests/generic/558.out
> 
> diff --git a/tests/generic/558 b/tests/generic/558
> new file mode 100755
> index 00000000..ee16cdf7
> --- /dev/null
> +++ b/tests/generic/558
> @@ -0,0 +1,75 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2019 SUSE Linux Products GmbH. All Rights Reserved.
> +#
> +# FSQA Test No. 558
> +#
> +# Test that if we clone a file with some large extents into a file that has
> +# many small extents, when the fs is nearly full, the clone operation does
> +# not fail and produces the correct result.
> +#
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/reflink
> +
> +# real QA test starts here
> +_supported_fs generic
> +_supported_os Linux
> +_require_scratch_reflink
> +
> +rm -f $seqres.full
> +
> +_scratch_mkfs_sized $((512 * 1024 * 1024)) >>$seqres.full 2>&1
> +_scratch_mount
> +
> +file_size=$(( 128 * 1024 * 1024 )) # 128Mb
> +extent_size=4096

What if the fs block size is 64k?

> +num_extents=$(( $file_size / $extent_size ))
> +
> +# Create a file with many small extents.
> +for ((i = 0; i < $num_extents; i++)); do
> +	offset=$(( $i * $extent_size ))
> +	$XFS_IO_PROG -f -s -c "pwrite -S 0xe5 $offset $extent_size" \
> +		$SCRATCH_MNT/foo >>/dev/null
> +done

I wouldn't have thought that this would actually succeed on xfs because
you could lay extents down one after the other, but then started seeing
this in the filefrag output:

File size of /opt/foo is 528384 (129 blocks of 4096 bytes)
 ext:     logical_offset:        physical_offset: length:   expected: flags:
   3:       17..      17:         52..        52:      1:         37:
   4:       18..      18:         67..        67:      1:         53:
   5:       19..      19:         81..        81:      1:         68:
   6:       20..      20:         94..        94:      1:         82:
   7:       21..      21:        106..       106:      1:         95:
   8:       22..      22:        117..       117:      1:        107:
   9:       23..      23:        127..       127:      1:        118:
  10:       24..      24:        136..       136:      1:        128:
  11:       25..      25:        144..       144:      1:        137:
  12:       26..      26:        151..       151:      1:        145:
  13:       27..      27:        157..       157:      1:        152:
  14:       28..      28:        162..       162:      1:        158:
  15:       29..      29:        166..       166:      1:        163:
  16:       30..      30:        169..       169:      1:        167:
  17:       31..      32:        171..       172:      2:        170:
  18:       33..      33:        188..       188:      1:        173:

52, 67, 81, 94, 106, 117, 127, 136, 44, 151, 157, 162, 166, 169, 171...

 +15 +14 +13 +12  +11  +10   +9   +8  +7   +6   +5   +4   +3   +2...

Hm, I wonder what quirk of the xfs allocator this might be?

> +
> +# Create file bar with the same size that file foo has but with large extents.
> +$XFS_IO_PROG -f -c "pwrite -S 0xc7 -b $file_size 0 $file_size" \
> +	$SCRATCH_MNT/bar >>/dev/null
> +
> +# Fill the fs (for btrfs we are interested in filling all unallocated space
> +# and most of the existing metadata block group(s), so that after this there
> +# will be no unallocated space and metadata space will be mostly full but with
> +# more than enough free space for the clone operation below to succeed).
> +i=1
> +while true; do
> +	$XFS_IO_PROG -f -c "pwrite 0 2K" $SCRATCH_MNT/filler_$i &> /dev/null
> +	[ $? -ne 0 ] && break
> +	i=$(( i + 1 ))
> +done

_fill_fs?

> +
> +# Now clone file bar into file foo. This is supposed to succeed and not fail
> +# with ENOSPC for example.
> +$XFS_IO_PROG -c "reflink $SCRATCH_MNT/bar" $SCRATCH_MNT/foo >>/dev/null

_reflink $SCRATCH_MNT/bar $SCRATCH_MNT/foo ?

--D

> +
> +_scratch_remount
> +
> +echo "File foo data after cloning and remount:"
> +od -A d -t x1 $SCRATCH_MNT/foo
> +
> +status=0
> +exit
> diff --git a/tests/generic/558.out b/tests/generic/558.out
> new file mode 100644
> index 00000000..d1e8e70f
> --- /dev/null
> +++ b/tests/generic/558.out
> @@ -0,0 +1,5 @@
> +QA output created by 558
> +File foo data after cloning and remount:
> +0000000 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7
> +*
> +134217728
> diff --git a/tests/generic/group b/tests/generic/group
> index 543c0627..c06c1cd1 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -560,3 +560,4 @@
>  555 auto quick cap
>  556 auto quick casefold
>  557 auto quick log
> +558 auto clone
> -- 
> 2.11.0
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] generic: test cloning large exents to a file with many small extents
  2019-06-27 20:28 ` Darrick J. Wong
@ 2019-06-27 21:47   ` Filipe Manana
  2019-06-28  3:32     ` Darrick J. Wong
  0 siblings, 1 reply; 8+ messages in thread
From: Filipe Manana @ 2019-06-27 21:47 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: fstests, linux-btrfs, Filipe Manana

On Thu, Jun 27, 2019 at 9:28 PM Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> On Thu, Jun 27, 2019 at 06:00:30PM +0100, fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> >
> > Test that if we clone a file with some large extents into a file that has
> > many small extents, when the fs is nearly full, the clone operation does
> > not fail and produces the correct result.
> >
> > This is motivated by a bug found in btrfs wich is fixed by the following
> > patches for the linux kernel:
> >
> >  [PATCH 1/2] Btrfs: factor out extent dropping code from hole punch handler
> >  [PATCH 2/2] Btrfs: fix ENOSPC errors, leading to transaction aborts, when
> >              cloning extents
> >
> > The test currently passes on xfs.
> >
> > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > ---
> >  tests/generic/558     | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/558.out |  5 ++++
> >  tests/generic/group   |  1 +
> >  3 files changed, 81 insertions(+)
> >  create mode 100755 tests/generic/558
> >  create mode 100644 tests/generic/558.out
> >
> > diff --git a/tests/generic/558 b/tests/generic/558
> > new file mode 100755
> > index 00000000..ee16cdf7
> > --- /dev/null
> > +++ b/tests/generic/558
> > @@ -0,0 +1,75 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (C) 2019 SUSE Linux Products GmbH. All Rights Reserved.
> > +#
> > +# FSQA Test No. 558
> > +#
> > +# Test that if we clone a file with some large extents into a file that has
> > +# many small extents, when the fs is nearly full, the clone operation does
> > +# not fail and produces the correct result.
> > +#
> > +seq=`basename $0`
> > +seqres=$RESULT_DIR/$seq
> > +echo "QA output created by $seq"
> > +tmp=/tmp/$$
> > +status=1     # failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > +     cd /
> > +     rm -f $tmp.*
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +. ./common/reflink
> > +
> > +# real QA test starts here
> > +_supported_fs generic
> > +_supported_os Linux
> > +_require_scratch_reflink
> > +
> > +rm -f $seqres.full
> > +
> > +_scratch_mkfs_sized $((512 * 1024 * 1024)) >>$seqres.full 2>&1
> > +_scratch_mount
> > +
> > +file_size=$(( 128 * 1024 * 1024 )) # 128Mb
> > +extent_size=4096
>
> What if the fs block size is 64k?

Then we get extents of 64Kb instead of 4Kb. Works on btrfs. Is it a
problem for xfs (or any other fs)?

>
> > +num_extents=$(( $file_size / $extent_size ))
> > +
> > +# Create a file with many small extents.
> > +for ((i = 0; i < $num_extents; i++)); do
> > +     offset=$(( $i * $extent_size ))
> > +     $XFS_IO_PROG -f -s -c "pwrite -S 0xe5 $offset $extent_size" \
> > +             $SCRATCH_MNT/foo >>/dev/null
> > +done
>
> I wouldn't have thought that this would actually succeed on xfs because
> you could lay extents down one after the other, but then started seeing
> this in the filefrag output:
>
> File size of /opt/foo is 528384 (129 blocks of 4096 bytes)
>  ext:     logical_offset:        physical_offset: length:   expected: flags:
>    3:       17..      17:         52..        52:      1:         37:
>    4:       18..      18:         67..        67:      1:         53:
>    5:       19..      19:         81..        81:      1:         68:
>    6:       20..      20:         94..        94:      1:         82:
>    7:       21..      21:        106..       106:      1:         95:
>    8:       22..      22:        117..       117:      1:        107:
>    9:       23..      23:        127..       127:      1:        118:
>   10:       24..      24:        136..       136:      1:        128:
>   11:       25..      25:        144..       144:      1:        137:
>   12:       26..      26:        151..       151:      1:        145:
>   13:       27..      27:        157..       157:      1:        152:
>   14:       28..      28:        162..       162:      1:        158:
>   15:       29..      29:        166..       166:      1:        163:
>   16:       30..      30:        169..       169:      1:        167:
>   17:       31..      32:        171..       172:      2:        170:
>   18:       33..      33:        188..       188:      1:        173:
>
> 52, 67, 81, 94, 106, 117, 127, 136, 44, 151, 157, 162, 166, 169, 171...
>
>  +15 +14 +13 +12  +11  +10   +9   +8  +7   +6   +5   +4   +3   +2...
>
> Hm, I wonder what quirk of the xfs allocator this might be?

Don't ask me, I have no idea how it works :)

>
> > +
> > +# Create file bar with the same size that file foo has but with large extents.
> > +$XFS_IO_PROG -f -c "pwrite -S 0xc7 -b $file_size 0 $file_size" \
> > +     $SCRATCH_MNT/bar >>/dev/null
> > +
> > +# Fill the fs (for btrfs we are interested in filling all unallocated space
> > +# and most of the existing metadata block group(s), so that after this there
> > +# will be no unallocated space and metadata space will be mostly full but with
> > +# more than enough free space for the clone operation below to succeed).
> > +i=1
> > +while true; do
> > +     $XFS_IO_PROG -f -c "pwrite 0 2K" $SCRATCH_MNT/filler_$i &> /dev/null
> > +     [ $? -ne 0 ] && break
> > +     i=$(( i + 1 ))
> > +done
>
> _fill_fs?

No, because that will do fallocate. Writing 2Kb files here is on
purpose because that will fill the metadata
block group(s) (on btrfs we inline extents in the btree (metadata) as
long as they are not bigger than 2Kb, by default).
Using fallocate, we end up using data block groups.

>
> > +
> > +# Now clone file bar into file foo. This is supposed to succeed and not fail
> > +# with ENOSPC for example.
> > +$XFS_IO_PROG -c "reflink $SCRATCH_MNT/bar" $SCRATCH_MNT/foo >>/dev/null
>
> _reflink $SCRATCH_MNT/bar $SCRATCH_MNT/foo ?

Does exactly the same. I don't mind changing it however.

Thanks.

>
> --D
>
> > +
> > +_scratch_remount
> > +
> > +echo "File foo data after cloning and remount:"
> > +od -A d -t x1 $SCRATCH_MNT/foo
> > +
> > +status=0
> > +exit
> > diff --git a/tests/generic/558.out b/tests/generic/558.out
> > new file mode 100644
> > index 00000000..d1e8e70f
> > --- /dev/null
> > +++ b/tests/generic/558.out
> > @@ -0,0 +1,5 @@
> > +QA output created by 558
> > +File foo data after cloning and remount:
> > +0000000 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7
> > +*
> > +134217728
> > diff --git a/tests/generic/group b/tests/generic/group
> > index 543c0627..c06c1cd1 100644
> > --- a/tests/generic/group
> > +++ b/tests/generic/group
> > @@ -560,3 +560,4 @@
> >  555 auto quick cap
> >  556 auto quick casefold
> >  557 auto quick log
> > +558 auto clone
> > --
> > 2.11.0
> >

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] generic: test cloning large exents to a file with many small extents
  2019-06-27 21:47   ` Filipe Manana
@ 2019-06-28  3:32     ` Darrick J. Wong
  0 siblings, 0 replies; 8+ messages in thread
From: Darrick J. Wong @ 2019-06-28  3:32 UTC (permalink / raw)
  To: Filipe Manana; +Cc: fstests, linux-btrfs, Filipe Manana

On Thu, Jun 27, 2019 at 10:47:31PM +0100, Filipe Manana wrote:
> On Thu, Jun 27, 2019 at 9:28 PM Darrick J. Wong <darrick.wong@oracle.com> wrote:
> >
> > On Thu, Jun 27, 2019 at 06:00:30PM +0100, fdmanana@kernel.org wrote:
> > > From: Filipe Manana <fdmanana@suse.com>
> > >
> > > Test that if we clone a file with some large extents into a file that has
> > > many small extents, when the fs is nearly full, the clone operation does
> > > not fail and produces the correct result.
> > >
> > > This is motivated by a bug found in btrfs wich is fixed by the following
> > > patches for the linux kernel:
> > >
> > >  [PATCH 1/2] Btrfs: factor out extent dropping code from hole punch handler
> > >  [PATCH 2/2] Btrfs: fix ENOSPC errors, leading to transaction aborts, when
> > >              cloning extents
> > >
> > > The test currently passes on xfs.
> > >
> > > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > > ---
> > >  tests/generic/558     | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  tests/generic/558.out |  5 ++++
> > >  tests/generic/group   |  1 +
> > >  3 files changed, 81 insertions(+)
> > >  create mode 100755 tests/generic/558
> > >  create mode 100644 tests/generic/558.out
> > >
> > > diff --git a/tests/generic/558 b/tests/generic/558
> > > new file mode 100755
> > > index 00000000..ee16cdf7
> > > --- /dev/null
> > > +++ b/tests/generic/558
> > > @@ -0,0 +1,75 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (C) 2019 SUSE Linux Products GmbH. All Rights Reserved.
> > > +#
> > > +# FSQA Test No. 558
> > > +#
> > > +# Test that if we clone a file with some large extents into a file that has
> > > +# many small extents, when the fs is nearly full, the clone operation does
> > > +# not fail and produces the correct result.
> > > +#
> > > +seq=`basename $0`
> > > +seqres=$RESULT_DIR/$seq
> > > +echo "QA output created by $seq"
> > > +tmp=/tmp/$$
> > > +status=1     # failure is the default!
> > > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > > +
> > > +_cleanup()
> > > +{
> > > +     cd /
> > > +     rm -f $tmp.*
> > > +}
> > > +
> > > +# get standard environment, filters and checks
> > > +. ./common/rc
> > > +. ./common/filter
> > > +. ./common/reflink
> > > +
> > > +# real QA test starts here
> > > +_supported_fs generic
> > > +_supported_os Linux
> > > +_require_scratch_reflink
> > > +
> > > +rm -f $seqres.full
> > > +
> > > +_scratch_mkfs_sized $((512 * 1024 * 1024)) >>$seqres.full 2>&1
> > > +_scratch_mount
> > > +
> > > +file_size=$(( 128 * 1024 * 1024 )) # 128Mb
> > > +extent_size=4096
> >
> > What if the fs block size is 64k?
> 
> Then we get extents of 64Kb instead of 4Kb. Works on btrfs. Is it a
> problem for xfs (or any other fs)?

It shouldn't be; I was merely wondering if 2048 extents was enough to
trigger the enospc on btrfs.

> >
> > > +num_extents=$(( $file_size / $extent_size ))
> > > +
> > > +# Create a file with many small extents.
> > > +for ((i = 0; i < $num_extents; i++)); do
> > > +     offset=$(( $i * $extent_size ))
> > > +     $XFS_IO_PROG -f -s -c "pwrite -S 0xe5 $offset $extent_size" \
> > > +             $SCRATCH_MNT/foo >>/dev/null
> > > +done
> >
> > I wouldn't have thought that this would actually succeed on xfs because
> > you could lay extents down one after the other, but then started seeing
> > this in the filefrag output:
> >
> > File size of /opt/foo is 528384 (129 blocks of 4096 bytes)
> >  ext:     logical_offset:        physical_offset: length:   expected: flags:
> >    3:       17..      17:         52..        52:      1:         37:
> >    4:       18..      18:         67..        67:      1:         53:
> >    5:       19..      19:         81..        81:      1:         68:
> >    6:       20..      20:         94..        94:      1:         82:
> >    7:       21..      21:        106..       106:      1:         95:
> >    8:       22..      22:        117..       117:      1:        107:
> >    9:       23..      23:        127..       127:      1:        118:
> >   10:       24..      24:        136..       136:      1:        128:
> >   11:       25..      25:        144..       144:      1:        137:
> >   12:       26..      26:        151..       151:      1:        145:
> >   13:       27..      27:        157..       157:      1:        152:
> >   14:       28..      28:        162..       162:      1:        158:
> >   15:       29..      29:        166..       166:      1:        163:
> >   16:       30..      30:        169..       169:      1:        167:
> >   17:       31..      32:        171..       172:      2:        170:
> >   18:       33..      33:        188..       188:      1:        173:
> >
> > 52, 67, 81, 94, 106, 117, 127, 136, 44, 151, 157, 162, 166, 169, 171...
> >
> >  +15 +14 +13 +12  +11  +10   +9   +8  +7   +6   +5   +4   +3   +2...
> >
> > Hm, I wonder what quirk of the xfs allocator this might be?
> 
> Don't ask me, I have no idea how it works :)

Ah, it's because xfs frees post-eof allocations every time we close a
file... and I never got the series that fixes this finalized and sent
upstream.

> >
> > > +
> > > +# Create file bar with the same size that file foo has but with large extents.
> > > +$XFS_IO_PROG -f -c "pwrite -S 0xc7 -b $file_size 0 $file_size" \
> > > +     $SCRATCH_MNT/bar >>/dev/null
> > > +
> > > +# Fill the fs (for btrfs we are interested in filling all unallocated space
> > > +# and most of the existing metadata block group(s), so that after this there
> > > +# will be no unallocated space and metadata space will be mostly full but with
> > > +# more than enough free space for the clone operation below to succeed).
> > > +i=1
> > > +while true; do
> > > +     $XFS_IO_PROG -f -c "pwrite 0 2K" $SCRATCH_MNT/filler_$i &> /dev/null
> > > +     [ $? -ne 0 ] && break
> > > +     i=$(( i + 1 ))
> > > +done
> >
> > _fill_fs?
> 
> No, because that will do fallocate. Writing 2Kb files here is on
> purpose because that will fill the metadata
> block group(s) (on btrfs we inline extents in the btree (metadata) as
> long as they are not bigger than 2Kb, by default).
> Using fallocate, we end up using data block groups.

Ok.

> >
> > > +
> > > +# Now clone file bar into file foo. This is supposed to succeed and not fail
> > > +# with ENOSPC for example.
> > > +$XFS_IO_PROG -c "reflink $SCRATCH_MNT/bar" $SCRATCH_MNT/foo >>/dev/null
> >
> > _reflink $SCRATCH_MNT/bar $SCRATCH_MNT/foo ?
> 
> Does exactly the same. I don't mind changing it however.

<Nod>

--D

> Thanks.
> 
> >
> > --D
> >
> > > +
> > > +_scratch_remount
> > > +
> > > +echo "File foo data after cloning and remount:"
> > > +od -A d -t x1 $SCRATCH_MNT/foo
> > > +
> > > +status=0
> > > +exit
> > > diff --git a/tests/generic/558.out b/tests/generic/558.out
> > > new file mode 100644
> > > index 00000000..d1e8e70f
> > > --- /dev/null
> > > +++ b/tests/generic/558.out
> > > @@ -0,0 +1,5 @@
> > > +QA output created by 558
> > > +File foo data after cloning and remount:
> > > +0000000 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7
> > > +*
> > > +134217728
> > > diff --git a/tests/generic/group b/tests/generic/group
> > > index 543c0627..c06c1cd1 100644
> > > --- a/tests/generic/group
> > > +++ b/tests/generic/group
> > > @@ -560,3 +560,4 @@
> > >  555 auto quick cap
> > >  556 auto quick casefold
> > >  557 auto quick log
> > > +558 auto clone
> > > --
> > > 2.11.0
> > >

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] generic: test cloning large exents to a file with many small extents
  2019-06-27 17:00 [PATCH] generic: test cloning large exents to a file with many small extents fdmanana
  2019-06-27 20:28 ` Darrick J. Wong
@ 2019-06-28 22:08 ` fdmanana
  2019-07-05  7:43   ` Eryu Guan
  1 sibling, 1 reply; 8+ messages in thread
From: fdmanana @ 2019-06-28 22:08 UTC (permalink / raw)
  To: fstests; +Cc: linux-btrfs, Filipe Manana

From: Filipe Manana <fdmanana@suse.com>

Test that if we clone a file with some large extents into a file that has
many small extents, when the fs is nearly full, the clone operation does
not fail and produces the correct result.

This is motivated by a bug found in btrfs wich is fixed by the following
patches for the linux kernel:

 [PATCH 1/2] Btrfs: factor out extent dropping code from hole punch handler
 [PATCH 2/2] Btrfs: fix ENOSPC errors, leading to transaction aborts, when
             cloning extents

The test currently passes on xfs.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---

V2: Use _scratch_cycle_mount instead of _scratch_remount, as we want to see
    if the operation was durably persisted (otherwise we are seeing content
    from the page cache).
    Use _reflink instead of calling xfs_io with the reflink command.
    Make the comment before filling the filesystem more clear about why it
    is done the way it is instead of using _fill_fs.

 tests/generic/558     | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/558.out |  5 ++++
 tests/generic/group   |  1 +
 3 files changed, 86 insertions(+)
 create mode 100755 tests/generic/558
 create mode 100644 tests/generic/558.out

diff --git a/tests/generic/558 b/tests/generic/558
new file mode 100755
index 00000000..f982930d
--- /dev/null
+++ b/tests/generic/558
@@ -0,0 +1,80 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2019 SUSE Linux Products GmbH. All Rights Reserved.
+#
+# FSQA Test No. 558
+#
+# Test that if we clone a file with some large extents into a file that has
+# many small extents, when the fs is nearly full, the clone operation does
+# not fail and produces the correct result.
+#
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/reflink
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_scratch_reflink
+
+rm -f $seqres.full
+
+_scratch_mkfs_sized $((512 * 1024 * 1024)) >>$seqres.full 2>&1
+_scratch_mount
+
+file_size=$(( 128 * 1024 * 1024 )) # 128Mb
+extent_size=4096
+num_extents=$(( $file_size / $extent_size ))
+
+# Create a file with many small extents.
+for ((i = 0; i < $num_extents; i++)); do
+	offset=$(( $i * $extent_size ))
+	$XFS_IO_PROG -f -s -c "pwrite -S 0xe5 $offset $extent_size" \
+		$SCRATCH_MNT/foo >>/dev/null
+done
+
+# Create file bar with the same size that file foo has but with large extents.
+$XFS_IO_PROG -f -c "pwrite -S 0xc7 -b $file_size 0 $file_size" \
+	$SCRATCH_MNT/bar >>/dev/null
+
+# Fill the fs (For btrfs we are interested in filling all unallocated space
+# and most of the existing metadata block group(s), so that after this there
+# will be no unallocated space and metadata space will be mostly full but with
+# more than enough free space for the clone operation below to succeed, we
+# create files with 2Kb because that results in extents inlined in the metadata
+# (btree leafs) and it's the fastest way to fill metadata space on btrfs, by
+# default btrfs inlines up to 2Kb of data).
+i=1
+while true; do
+	$XFS_IO_PROG -f -c "pwrite 0 2K" $SCRATCH_MNT/filler_$i &> /dev/null
+	[ $? -ne 0 ] && break
+	i=$(( i + 1 ))
+done
+
+# Now clone file bar into file foo. This is supposed to succeed and not fail
+# with ENOSPC for example.
+_reflink $SCRATCH_MNT/bar $SCRATCH_MNT/foo >>$seqres.full
+
+# Unmount and mount the filesystem again to verify the operation was durably
+# persisted.
+_scratch_cycle_mount
+
+echo "File foo data after cloning and remount:"
+od -A d -t x1 $SCRATCH_MNT/foo
+
+status=0
+exit
diff --git a/tests/generic/558.out b/tests/generic/558.out
new file mode 100644
index 00000000..d1e8e70f
--- /dev/null
+++ b/tests/generic/558.out
@@ -0,0 +1,5 @@
+QA output created by 558
+File foo data after cloning and remount:
+0000000 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7
+*
+134217728
diff --git a/tests/generic/group b/tests/generic/group
index 543c0627..c06c1cd1 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -560,3 +560,4 @@
 555 auto quick cap
 556 auto quick casefold
 557 auto quick log
+558 auto clone
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] generic: test cloning large exents to a file with many small extents
  2019-06-28 22:08 ` [PATCH v2] " fdmanana
@ 2019-07-05  7:43   ` Eryu Guan
  2019-07-05 10:09     ` Filipe Manana
  0 siblings, 1 reply; 8+ messages in thread
From: Eryu Guan @ 2019-07-05  7:43 UTC (permalink / raw)
  To: fdmanana; +Cc: fstests, linux-btrfs, Filipe Manana

On Fri, Jun 28, 2019 at 11:08:36PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Test that if we clone a file with some large extents into a file that has
> many small extents, when the fs is nearly full, the clone operation does
> not fail and produces the correct result.
> 
> This is motivated by a bug found in btrfs wich is fixed by the following
> patches for the linux kernel:
> 
>  [PATCH 1/2] Btrfs: factor out extent dropping code from hole punch handler
>  [PATCH 2/2] Btrfs: fix ENOSPC errors, leading to transaction aborts, when
>              cloning extents
> 
> The test currently passes on xfs.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
> 
> V2: Use _scratch_cycle_mount instead of _scratch_remount, as we want to see
>     if the operation was durably persisted (otherwise we are seeing content
>     from the page cache).
>     Use _reflink instead of calling xfs_io with the reflink command.
>     Make the comment before filling the filesystem more clear about why it
>     is done the way it is instead of using _fill_fs.
> 
>  tests/generic/558     | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/558.out |  5 ++++
>  tests/generic/group   |  1 +
>  3 files changed, 86 insertions(+)
>  create mode 100755 tests/generic/558
>  create mode 100644 tests/generic/558.out
> 
> diff --git a/tests/generic/558 b/tests/generic/558
> new file mode 100755
> index 00000000..f982930d
> --- /dev/null
> +++ b/tests/generic/558
> @@ -0,0 +1,80 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2019 SUSE Linux Products GmbH. All Rights Reserved.
> +#
> +# FSQA Test No. 558
> +#
> +# Test that if we clone a file with some large extents into a file that has
> +# many small extents, when the fs is nearly full, the clone operation does
> +# not fail and produces the correct result.
> +#
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/reflink
> +
> +# real QA test starts here
> +_supported_fs generic
> +_supported_os Linux
> +_require_scratch_reflink
> +
> +rm -f $seqres.full
> +
> +_scratch_mkfs_sized $((512 * 1024 * 1024)) >>$seqres.full 2>&1
> +_scratch_mount
> +
> +file_size=$(( 128 * 1024 * 1024 )) # 128Mb
> +extent_size=4096
> +num_extents=$(( $file_size / $extent_size ))
> +
> +# Create a file with many small extents.
> +for ((i = 0; i < $num_extents; i++)); do
> +	offset=$(( $i * $extent_size ))
> +	$XFS_IO_PROG -f -s -c "pwrite -S 0xe5 $offset $extent_size" \
> +		$SCRATCH_MNT/foo >>/dev/null
> +done

This is taking too long time (1000+s) to finish when testing on XFS. I'm
wondering if we could take use of src/punch-alternating to punch out
every other block to create a file with many small extents.

i.e. with the following diffs, test runs & passes with XFS within 90s
while still reproduces the btrfs bug (note that I have to increase the
file_size to 200M to reproduce the btrfs bug, while 256M seems bring
test time back to 1000+s). Would you please check if this works for you?

Thanks,
Eryu


diff --git a/tests/generic/558 b/tests/generic/558
index f982930d65a2..40f8a7a98d3f 100755
--- a/tests/generic/558
+++ b/tests/generic/558
@@ -30,22 +30,22 @@ _cleanup()
 _supported_fs generic
 _supported_os Linux
 _require_scratch_reflink
+_require_test_program "punch-alternating"
+_require_xfs_io_command "fpunch"

 rm -f $seqres.full

 _scratch_mkfs_sized $((512 * 1024 * 1024)) >>$seqres.full 2>&1
 _scratch_mount

-file_size=$(( 128 * 1024 * 1024 )) # 128Mb
+file_size=$(( 200 * 1024 * 1024 )) # 200MB
 extent_size=4096
 num_extents=$(( $file_size / $extent_size ))

 # Create a file with many small extents.
-for ((i = 0; i < $num_extents; i++)); do
-       offset=$(( $i * $extent_size ))
-       $XFS_IO_PROG -f -s -c "pwrite -S 0xe5 $offset $extent_size" \
-               $SCRATCH_MNT/foo >>/dev/null
-done
+$XFS_IO_PROG -f -c "pwrite -S 0xe5 -b $file_size 0 $file_size" \
+       $SCRATCH_MNT/foo >>/dev/null
+$here/src/punch-alternating $SCRATCH_MNT/foo >> $seqres.full

 # Create file bar with the same size that file foo has but with large extents.
 $XFS_IO_PROG -f -c "pwrite -S 0xc7 -b $file_size 0 $file_size" \
diff --git a/tests/generic/558.out b/tests/generic/558.out
index d1e8e70f5b79..00cb5a9744aa 100644
--- a/tests/generic/558.out
+++ b/tests/generic/558.out
@@ -2,4 +2,4 @@ QA output created by 558
 File foo data after cloning and remount:
 0000000 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7
 *
-134217728
+209715200

> +
> +# Create file bar with the same size that file foo has but with large extents.
> +$XFS_IO_PROG -f -c "pwrite -S 0xc7 -b $file_size 0 $file_size" \
> +	$SCRATCH_MNT/bar >>/dev/null
> +
> +# Fill the fs (For btrfs we are interested in filling all unallocated space
> +# and most of the existing metadata block group(s), so that after this there
> +# will be no unallocated space and metadata space will be mostly full but with
> +# more than enough free space for the clone operation below to succeed, we
> +# create files with 2Kb because that results in extents inlined in the metadata
> +# (btree leafs) and it's the fastest way to fill metadata space on btrfs, by
> +# default btrfs inlines up to 2Kb of data).
> +i=1
> +while true; do
> +	$XFS_IO_PROG -f -c "pwrite 0 2K" $SCRATCH_MNT/filler_$i &> /dev/null
> +	[ $? -ne 0 ] && break
> +	i=$(( i + 1 ))
> +done
> +
> +# Now clone file bar into file foo. This is supposed to succeed and not fail
> +# with ENOSPC for example.
> +_reflink $SCRATCH_MNT/bar $SCRATCH_MNT/foo >>$seqres.full
> +
> +# Unmount and mount the filesystem again to verify the operation was durably
> +# persisted.
> +_scratch_cycle_mount
> +
> +echo "File foo data after cloning and remount:"
> +od -A d -t x1 $SCRATCH_MNT/foo
> +
> +status=0
> +exit
> diff --git a/tests/generic/558.out b/tests/generic/558.out
> new file mode 100644
> index 00000000..d1e8e70f
> --- /dev/null
> +++ b/tests/generic/558.out
> @@ -0,0 +1,5 @@
> +QA output created by 558
> +File foo data after cloning and remount:
> +0000000 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7
> +*
> +134217728
> diff --git a/tests/generic/group b/tests/generic/group
> index 543c0627..c06c1cd1 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -560,3 +560,4 @@
>  555 auto quick cap
>  556 auto quick casefold
>  557 auto quick log
> +558 auto clone
> -- 
> 2.11.0
> 

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] generic: test cloning large exents to a file with many small extents
  2019-07-05  7:43   ` Eryu Guan
@ 2019-07-05 10:09     ` Filipe Manana
  2019-07-07 12:41       ` Eryu Guan
  0 siblings, 1 reply; 8+ messages in thread
From: Filipe Manana @ 2019-07-05 10:09 UTC (permalink / raw)
  To: Eryu Guan; +Cc: fstests, linux-btrfs, Filipe Manana

On Fri, Jul 5, 2019 at 8:43 AM Eryu Guan <guaneryu@gmail.com> wrote:
>
> On Fri, Jun 28, 2019 at 11:08:36PM +0100, fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> >
> > Test that if we clone a file with some large extents into a file that has
> > many small extents, when the fs is nearly full, the clone operation does
> > not fail and produces the correct result.
> >
> > This is motivated by a bug found in btrfs wich is fixed by the following
> > patches for the linux kernel:
> >
> >  [PATCH 1/2] Btrfs: factor out extent dropping code from hole punch handler
> >  [PATCH 2/2] Btrfs: fix ENOSPC errors, leading to transaction aborts, when
> >              cloning extents
> >
> > The test currently passes on xfs.
> >
> > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > ---
> >
> > V2: Use _scratch_cycle_mount instead of _scratch_remount, as we want to see
> >     if the operation was durably persisted (otherwise we are seeing content
> >     from the page cache).
> >     Use _reflink instead of calling xfs_io with the reflink command.
> >     Make the comment before filling the filesystem more clear about why it
> >     is done the way it is instead of using _fill_fs.
> >
> >  tests/generic/558     | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/558.out |  5 ++++
> >  tests/generic/group   |  1 +
> >  3 files changed, 86 insertions(+)
> >  create mode 100755 tests/generic/558
> >  create mode 100644 tests/generic/558.out
> >
> > diff --git a/tests/generic/558 b/tests/generic/558
> > new file mode 100755
> > index 00000000..f982930d
> > --- /dev/null
> > +++ b/tests/generic/558
> > @@ -0,0 +1,80 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (C) 2019 SUSE Linux Products GmbH. All Rights Reserved.
> > +#
> > +# FSQA Test No. 558
> > +#
> > +# Test that if we clone a file with some large extents into a file that has
> > +# many small extents, when the fs is nearly full, the clone operation does
> > +# not fail and produces the correct result.
> > +#
> > +seq=`basename $0`
> > +seqres=$RESULT_DIR/$seq
> > +echo "QA output created by $seq"
> > +tmp=/tmp/$$
> > +status=1     # failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > +     cd /
> > +     rm -f $tmp.*
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +. ./common/reflink
> > +
> > +# real QA test starts here
> > +_supported_fs generic
> > +_supported_os Linux
> > +_require_scratch_reflink
> > +
> > +rm -f $seqres.full
> > +
> > +_scratch_mkfs_sized $((512 * 1024 * 1024)) >>$seqres.full 2>&1
> > +_scratch_mount
> > +
> > +file_size=$(( 128 * 1024 * 1024 )) # 128Mb
> > +extent_size=4096
> > +num_extents=$(( $file_size / $extent_size ))
> > +
> > +# Create a file with many small extents.
> > +for ((i = 0; i < $num_extents; i++)); do
> > +     offset=$(( $i * $extent_size ))
> > +     $XFS_IO_PROG -f -s -c "pwrite -S 0xe5 $offset $extent_size" \
> > +             $SCRATCH_MNT/foo >>/dev/null
> > +done
>
> This is taking too long time (1000+s) to finish when testing on XFS.

Hum, for on 5.2-rc, debug kernel (lockdep, plenty of other debug stuff
enabled that slows things down), it takes about 350 seconds for me.

> I'm
> wondering if we could take use of src/punch-alternating to punch out
> every other block to create a file with many small extents.

That's fine, I didn't remember about that.

>
> i.e. with the following diffs, test runs & passes with XFS within 90s
> while still reproduces the btrfs bug (note that I have to increase the
> file_size to 200M to reproduce the btrfs bug, while 256M seems bring
> test time back to 1000+s). Would you please check if this works for you?

It does, thanks!
And, because of the holes, it uncovered a bug in my fix (second patch).

Now it takes ~15 seconds on btrfs instead of ~300s and ~100s on xfs for me.

>
> Thanks,
> Eryu
>
>
> diff --git a/tests/generic/558 b/tests/generic/558
> index f982930d65a2..40f8a7a98d3f 100755
> --- a/tests/generic/558
> +++ b/tests/generic/558
> @@ -30,22 +30,22 @@ _cleanup()
>  _supported_fs generic
>  _supported_os Linux
>  _require_scratch_reflink
> +_require_test_program "punch-alternating"
> +_require_xfs_io_command "fpunch"
>
>  rm -f $seqres.full
>
>  _scratch_mkfs_sized $((512 * 1024 * 1024)) >>$seqres.full 2>&1
>  _scratch_mount
>
> -file_size=$(( 128 * 1024 * 1024 )) # 128Mb
> +file_size=$(( 200 * 1024 * 1024 )) # 200MB
>  extent_size=4096
>  num_extents=$(( $file_size / $extent_size ))

These two variables, extent_size and num_extents, are no longer needed.

Do you want me to send another version of the patch or can you apply
this patch on top of it?

Thanks.

>
>  # Create a file with many small extents.
> -for ((i = 0; i < $num_extents; i++)); do
> -       offset=$(( $i * $extent_size ))
> -       $XFS_IO_PROG -f -s -c "pwrite -S 0xe5 $offset $extent_size" \
> -               $SCRATCH_MNT/foo >>/dev/null
> -done
> +$XFS_IO_PROG -f -c "pwrite -S 0xe5 -b $file_size 0 $file_size" \
> +       $SCRATCH_MNT/foo >>/dev/null
> +$here/src/punch-alternating $SCRATCH_MNT/foo >> $seqres.full
>
>  # Create file bar with the same size that file foo has but with large extents.
>  $XFS_IO_PROG -f -c "pwrite -S 0xc7 -b $file_size 0 $file_size" \
> diff --git a/tests/generic/558.out b/tests/generic/558.out
> index d1e8e70f5b79..00cb5a9744aa 100644
> --- a/tests/generic/558.out
> +++ b/tests/generic/558.out
> @@ -2,4 +2,4 @@ QA output created by 558
>  File foo data after cloning and remount:
>  0000000 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7
>  *
> -134217728
> +209715200
>
> > +
> > +# Create file bar with the same size that file foo has but with large extents.
> > +$XFS_IO_PROG -f -c "pwrite -S 0xc7 -b $file_size 0 $file_size" \
> > +     $SCRATCH_MNT/bar >>/dev/null
> > +
> > +# Fill the fs (For btrfs we are interested in filling all unallocated space
> > +# and most of the existing metadata block group(s), so that after this there
> > +# will be no unallocated space and metadata space will be mostly full but with
> > +# more than enough free space for the clone operation below to succeed, we
> > +# create files with 2Kb because that results in extents inlined in the metadata
> > +# (btree leafs) and it's the fastest way to fill metadata space on btrfs, by
> > +# default btrfs inlines up to 2Kb of data).
> > +i=1
> > +while true; do
> > +     $XFS_IO_PROG -f -c "pwrite 0 2K" $SCRATCH_MNT/filler_$i &> /dev/null
> > +     [ $? -ne 0 ] && break
> > +     i=$(( i + 1 ))
> > +done
> > +
> > +# Now clone file bar into file foo. This is supposed to succeed and not fail
> > +# with ENOSPC for example.
> > +_reflink $SCRATCH_MNT/bar $SCRATCH_MNT/foo >>$seqres.full
> > +
> > +# Unmount and mount the filesystem again to verify the operation was durably
> > +# persisted.
> > +_scratch_cycle_mount
> > +
> > +echo "File foo data after cloning and remount:"
> > +od -A d -t x1 $SCRATCH_MNT/foo
> > +
> > +status=0
> > +exit
> > diff --git a/tests/generic/558.out b/tests/generic/558.out
> > new file mode 100644
> > index 00000000..d1e8e70f
> > --- /dev/null
> > +++ b/tests/generic/558.out
> > @@ -0,0 +1,5 @@
> > +QA output created by 558
> > +File foo data after cloning and remount:
> > +0000000 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7
> > +*
> > +134217728
> > diff --git a/tests/generic/group b/tests/generic/group
> > index 543c0627..c06c1cd1 100644
> > --- a/tests/generic/group
> > +++ b/tests/generic/group
> > @@ -560,3 +560,4 @@
> >  555 auto quick cap
> >  556 auto quick casefold
> >  557 auto quick log
> > +558 auto clone
> > --
> > 2.11.0
> >

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] generic: test cloning large exents to a file with many small extents
  2019-07-05 10:09     ` Filipe Manana
@ 2019-07-07 12:41       ` Eryu Guan
  0 siblings, 0 replies; 8+ messages in thread
From: Eryu Guan @ 2019-07-07 12:41 UTC (permalink / raw)
  To: Filipe Manana; +Cc: fstests, linux-btrfs, Filipe Manana

On Fri, Jul 05, 2019 at 11:09:38AM +0100, Filipe Manana wrote:
> On Fri, Jul 5, 2019 at 8:43 AM Eryu Guan <guaneryu@gmail.com> wrote:
> >
> > On Fri, Jun 28, 2019 at 11:08:36PM +0100, fdmanana@kernel.org wrote:
> > > From: Filipe Manana <fdmanana@suse.com>
> > >
> > > Test that if we clone a file with some large extents into a file that has
> > > many small extents, when the fs is nearly full, the clone operation does
> > > not fail and produces the correct result.
> > >
> > > This is motivated by a bug found in btrfs wich is fixed by the following
> > > patches for the linux kernel:
> > >
> > >  [PATCH 1/2] Btrfs: factor out extent dropping code from hole punch handler
> > >  [PATCH 2/2] Btrfs: fix ENOSPC errors, leading to transaction aborts, when
> > >              cloning extents
> > >
> > > The test currently passes on xfs.
> > >
> > > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > > ---
> > >
> > > V2: Use _scratch_cycle_mount instead of _scratch_remount, as we want to see
> > >     if the operation was durably persisted (otherwise we are seeing content
> > >     from the page cache).
> > >     Use _reflink instead of calling xfs_io with the reflink command.
> > >     Make the comment before filling the filesystem more clear about why it
> > >     is done the way it is instead of using _fill_fs.
> > >
> > >  tests/generic/558     | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  tests/generic/558.out |  5 ++++
> > >  tests/generic/group   |  1 +
> > >  3 files changed, 86 insertions(+)
> > >  create mode 100755 tests/generic/558
> > >  create mode 100644 tests/generic/558.out
> > >
> > > diff --git a/tests/generic/558 b/tests/generic/558
> > > new file mode 100755
> > > index 00000000..f982930d
> > > --- /dev/null
> > > +++ b/tests/generic/558
> > > @@ -0,0 +1,80 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (C) 2019 SUSE Linux Products GmbH. All Rights Reserved.
> > > +#
> > > +# FSQA Test No. 558
> > > +#
> > > +# Test that if we clone a file with some large extents into a file that has
> > > +# many small extents, when the fs is nearly full, the clone operation does
> > > +# not fail and produces the correct result.
> > > +#
> > > +seq=`basename $0`
> > > +seqres=$RESULT_DIR/$seq
> > > +echo "QA output created by $seq"
> > > +tmp=/tmp/$$
> > > +status=1     # failure is the default!
> > > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > > +
> > > +_cleanup()
> > > +{
> > > +     cd /
> > > +     rm -f $tmp.*
> > > +}
> > > +
> > > +# get standard environment, filters and checks
> > > +. ./common/rc
> > > +. ./common/filter
> > > +. ./common/reflink
> > > +
> > > +# real QA test starts here
> > > +_supported_fs generic
> > > +_supported_os Linux
> > > +_require_scratch_reflink
> > > +
> > > +rm -f $seqres.full
> > > +
> > > +_scratch_mkfs_sized $((512 * 1024 * 1024)) >>$seqres.full 2>&1
> > > +_scratch_mount
> > > +
> > > +file_size=$(( 128 * 1024 * 1024 )) # 128Mb
> > > +extent_size=4096
> > > +num_extents=$(( $file_size / $extent_size ))
> > > +
> > > +# Create a file with many small extents.
> > > +for ((i = 0; i < $num_extents; i++)); do
> > > +     offset=$(( $i * $extent_size ))
> > > +     $XFS_IO_PROG -f -s -c "pwrite -S 0xe5 $offset $extent_size" \
> > > +             $SCRATCH_MNT/foo >>/dev/null
> > > +done
> >
> > This is taking too long time (1000+s) to finish when testing on XFS.
> 
> Hum, for on 5.2-rc, debug kernel (lockdep, plenty of other debug stuff
> enabled that slows things down), it takes about 350 seconds for me.
> 
> > I'm
> > wondering if we could take use of src/punch-alternating to punch out
> > every other block to create a file with many small extents.
> 
> That's fine, I didn't remember about that.
> 
> >
> > i.e. with the following diffs, test runs & passes with XFS within 90s
> > while still reproduces the btrfs bug (note that I have to increase the
> > file_size to 200M to reproduce the btrfs bug, while 256M seems bring
> > test time back to 1000+s). Would you please check if this works for you?
> 
> It does, thanks!
> And, because of the holes, it uncovered a bug in my fix (second patch).

That's great :)

> 
> Now it takes ~15 seconds on btrfs instead of ~300s and ~100s on xfs for me.

Sounds good!

> 
> >
> > Thanks,
> > Eryu
> >
> >
> > diff --git a/tests/generic/558 b/tests/generic/558
> > index f982930d65a2..40f8a7a98d3f 100755
> > --- a/tests/generic/558
> > +++ b/tests/generic/558
> > @@ -30,22 +30,22 @@ _cleanup()
> >  _supported_fs generic
> >  _supported_os Linux
> >  _require_scratch_reflink
> > +_require_test_program "punch-alternating"
> > +_require_xfs_io_command "fpunch"
> >
> >  rm -f $seqres.full
> >
> >  _scratch_mkfs_sized $((512 * 1024 * 1024)) >>$seqres.full 2>&1
> >  _scratch_mount
> >
> > -file_size=$(( 128 * 1024 * 1024 )) # 128Mb
> > +file_size=$(( 200 * 1024 * 1024 )) # 200MB
> >  extent_size=4096
> >  num_extents=$(( $file_size / $extent_size ))
> 
> These two variables, extent_size and num_extents, are no longer needed.
> 
> Do you want me to send another version of the patch or can you apply
> this patch on top of it?

I can fix the patch on commit. Thanks for the confirmation!

Eryu

> 
> Thanks.
> 
> >
> >  # Create a file with many small extents.
> > -for ((i = 0; i < $num_extents; i++)); do
> > -       offset=$(( $i * $extent_size ))
> > -       $XFS_IO_PROG -f -s -c "pwrite -S 0xe5 $offset $extent_size" \
> > -               $SCRATCH_MNT/foo >>/dev/null
> > -done
> > +$XFS_IO_PROG -f -c "pwrite -S 0xe5 -b $file_size 0 $file_size" \
> > +       $SCRATCH_MNT/foo >>/dev/null
> > +$here/src/punch-alternating $SCRATCH_MNT/foo >> $seqres.full
> >
> >  # Create file bar with the same size that file foo has but with large extents.
> >  $XFS_IO_PROG -f -c "pwrite -S 0xc7 -b $file_size 0 $file_size" \
> > diff --git a/tests/generic/558.out b/tests/generic/558.out
> > index d1e8e70f5b79..00cb5a9744aa 100644
> > --- a/tests/generic/558.out
> > +++ b/tests/generic/558.out
> > @@ -2,4 +2,4 @@ QA output created by 558
> >  File foo data after cloning and remount:
> >  0000000 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7
> >  *
> > -134217728
> > +209715200
> >
> > > +
> > > +# Create file bar with the same size that file foo has but with large extents.
> > > +$XFS_IO_PROG -f -c "pwrite -S 0xc7 -b $file_size 0 $file_size" \
> > > +     $SCRATCH_MNT/bar >>/dev/null
> > > +
> > > +# Fill the fs (For btrfs we are interested in filling all unallocated space
> > > +# and most of the existing metadata block group(s), so that after this there
> > > +# will be no unallocated space and metadata space will be mostly full but with
> > > +# more than enough free space for the clone operation below to succeed, we
> > > +# create files with 2Kb because that results in extents inlined in the metadata
> > > +# (btree leafs) and it's the fastest way to fill metadata space on btrfs, by
> > > +# default btrfs inlines up to 2Kb of data).
> > > +i=1
> > > +while true; do
> > > +     $XFS_IO_PROG -f -c "pwrite 0 2K" $SCRATCH_MNT/filler_$i &> /dev/null
> > > +     [ $? -ne 0 ] && break
> > > +     i=$(( i + 1 ))
> > > +done
> > > +
> > > +# Now clone file bar into file foo. This is supposed to succeed and not fail
> > > +# with ENOSPC for example.
> > > +_reflink $SCRATCH_MNT/bar $SCRATCH_MNT/foo >>$seqres.full
> > > +
> > > +# Unmount and mount the filesystem again to verify the operation was durably
> > > +# persisted.
> > > +_scratch_cycle_mount
> > > +
> > > +echo "File foo data after cloning and remount:"
> > > +od -A d -t x1 $SCRATCH_MNT/foo
> > > +
> > > +status=0
> > > +exit
> > > diff --git a/tests/generic/558.out b/tests/generic/558.out
> > > new file mode 100644
> > > index 00000000..d1e8e70f
> > > --- /dev/null
> > > +++ b/tests/generic/558.out
> > > @@ -0,0 +1,5 @@
> > > +QA output created by 558
> > > +File foo data after cloning and remount:
> > > +0000000 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7 c7
> > > +*
> > > +134217728
> > > diff --git a/tests/generic/group b/tests/generic/group
> > > index 543c0627..c06c1cd1 100644
> > > --- a/tests/generic/group
> > > +++ b/tests/generic/group
> > > @@ -560,3 +560,4 @@
> > >  555 auto quick cap
> > >  556 auto quick casefold
> > >  557 auto quick log
> > > +558 auto clone
> > > --
> > > 2.11.0
> > >

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-07-07 12:42 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-27 17:00 [PATCH] generic: test cloning large exents to a file with many small extents fdmanana
2019-06-27 20:28 ` Darrick J. Wong
2019-06-27 21:47   ` Filipe Manana
2019-06-28  3:32     ` Darrick J. Wong
2019-06-28 22:08 ` [PATCH v2] " fdmanana
2019-07-05  7:43   ` Eryu Guan
2019-07-05 10:09     ` Filipe Manana
2019-07-07 12:41       ` Eryu Guan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).