FSTests Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] generic/501: make the test work on machines with a non 4K page size
@ 2020-07-22 14:12 fdmanana
  2020-07-23  5:09 ` Zorro Lang
  2020-07-23  6:16 ` Murphy Zhou
  0 siblings, 2 replies; 3+ messages in thread
From: fdmanana @ 2020-07-22 14:12 UTC (permalink / raw)
  To: fstests; +Cc: linux-btrfs, Filipe Manana, Murphy Zhou

From: Filipe Manana <fdmanana@suse.com>

Currently generic/501 fails on machines with a page size different from 4K
(like ppc64le), because the clone operation fails with -EINVAL due to the
fact we pass it an offset that is 4K aligned but not aligned to the page
size of the machine.

The test doesn't actually need offsets and lengths to be 4K aligned, so
just update the test to use offsets and lenghts that work for page size.
Also add a comment mentioning that a file size of at least 16Mb was a
necessary condition to trigger the btrfs bug.

The test is a regression test for a btrfs issue fixed by kernel commit
bd3599a0e142cd ("Btrfs: fix file data corruption after cloning a range
and fsync"), which landed in kernel 4.18.

Since I couldn't compile a 4.17 kernel on debian testing, I tried this
with a 4.18 kernel with that commit reverted, and it fails as expected
on a x86_64 box:

$ ./check generic/501
FSTYP         -- btrfs
PLATFORM      -- Linux/x86_64 debian9 4.18.0-btrfs-next-64 #1 SMP (...)
MKFS_OPTIONS  -- /dev/sdc
MOUNT_OPTIONS -- /dev/sdc /home/fdmanana/btrfs-tests/scratch_1

generic/501 1s ... - output mismatch (see .../xfstests/results//generic/501.out.bad)
    --- tests/generic/501.out	2020-07-22 14:50:12.585674202 +0100
    +++ /home/fdmanana/git/hub/xfstests/results//generic/501.out.bad ...
    @@ -2,4 +2,4 @@
     File bar digest before power failure:
     69319d0343ab8f5ea564167da445addc  SCRATCH_MNT/bar
     File bar digest after power failure:
    -69319d0343ab8f5ea564167da445addc  SCRATCH_MNT/bar
    +21de7d7325fe4dae1f3311d5a76f819f  SCRATCH_MNT/bar
    ...
    (Run 'diff -u /home/fdmanana/git/hub/xfstests/tests/generic/501.out ...
Ran: generic/501
Failures: generic/501
Failed 1 of 1 tests

Without the commit reverted it passes as expected.

Reported-by: Murphy Zhou <jencce.kernel@gmail.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 tests/generic/501     | 14 ++++++++------
 tests/generic/501.out |  4 ++--
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/tests/generic/501 b/tests/generic/501
index 0d1f6ffe..bf020095 100755
--- a/tests/generic/501
+++ b/tests/generic/501
@@ -42,14 +42,16 @@ _require_metadata_journaling $SCRATCH_DEV
 _init_flakey
 _mount_flakey
 
-$XFS_IO_PROG -f -c "pwrite -S 0x18 9000K 6908K" $SCRATCH_MNT/foo >>$seqres.full
-$XFS_IO_PROG -f -c "pwrite -S 0x20 2572K 156K" $SCRATCH_MNT/bar >>$seqres.full
+# Use file sizes and offsets/lengths for the clone operation that are multiples
+# of 64Kb, so that the test works on machine with any page size.
+$XFS_IO_PROG -f -s -c "pwrite -S 0x18 0 2M" $SCRATCH_MNT/foo >>$seqres.full
+$XFS_IO_PROG -f -s -c "pwrite -S 0x20 0 20M" $SCRATCH_MNT/bar >>$seqres.full
 
 # We clone from file foo into a range of file bar that overlaps the existing
-# extent at file bar. The destination offset of the reflink operation matches
-# the eof position of file bar minus 4Kb.
-$XFS_IO_PROG -c "fsync" \
-	     -c "reflink ${SCRATCH_MNT}/foo 0 2724K 15908K" \
+# extent at file bar. The clone operation must also extend the size of file bar.
+# Note: in order to trigger the original bug on btrfs, the destination file size
+# must be at least 16Mb and the destination file must have been fsynced before.
+$XFS_IO_PROG -c "reflink ${SCRATCH_MNT}/foo 0 19M 2M" \
 	     -c "fsync" \
 	     $SCRATCH_MNT/bar >>$seqres.full
 
diff --git a/tests/generic/501.out b/tests/generic/501.out
index 5d7da017..778aba4b 100644
--- a/tests/generic/501.out
+++ b/tests/generic/501.out
@@ -1,5 +1,5 @@
 QA output created by 501
 File bar digest before power failure:
-95a95813a8c2abc9aa75a6c2914a077e  SCRATCH_MNT/bar
+69319d0343ab8f5ea564167da445addc  SCRATCH_MNT/bar
 File bar digest after power failure:
-95a95813a8c2abc9aa75a6c2914a077e  SCRATCH_MNT/bar
+69319d0343ab8f5ea564167da445addc  SCRATCH_MNT/bar
-- 
2.26.2


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

* Re: [PATCH] generic/501: make the test work on machines with a non 4K page size
  2020-07-22 14:12 [PATCH] generic/501: make the test work on machines with a non 4K page size fdmanana
@ 2020-07-23  5:09 ` Zorro Lang
  2020-07-23  6:16 ` Murphy Zhou
  1 sibling, 0 replies; 3+ messages in thread
From: Zorro Lang @ 2020-07-23  5:09 UTC (permalink / raw)
  To: fdmanana; +Cc: fstests, linux-btrfs, Filipe Manana, Murphy Zhou

On Wed, Jul 22, 2020 at 03:12:54PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Currently generic/501 fails on machines with a page size different from 4K
> (like ppc64le), because the clone operation fails with -EINVAL due to the
> fact we pass it an offset that is 4K aligned but not aligned to the page
> size of the machine.
> 
> The test doesn't actually need offsets and lengths to be 4K aligned, so
> just update the test to use offsets and lenghts that work for page size.
> Also add a comment mentioning that a file size of at least 16Mb was a
> necessary condition to trigger the btrfs bug.
> 
> The test is a regression test for a btrfs issue fixed by kernel commit
> bd3599a0e142cd ("Btrfs: fix file data corruption after cloning a range
> and fsync"), which landed in kernel 4.18.
> 
> Since I couldn't compile a 4.17 kernel on debian testing, I tried this
> with a 4.18 kernel with that commit reverted, and it fails as expected
> on a x86_64 box:
> 
> $ ./check generic/501
> FSTYP         -- btrfs
> PLATFORM      -- Linux/x86_64 debian9 4.18.0-btrfs-next-64 #1 SMP (...)
> MKFS_OPTIONS  -- /dev/sdc
> MOUNT_OPTIONS -- /dev/sdc /home/fdmanana/btrfs-tests/scratch_1
> 
> generic/501 1s ... - output mismatch (see .../xfstests/results//generic/501.out.bad)
>     --- tests/generic/501.out	2020-07-22 14:50:12.585674202 +0100
>     +++ /home/fdmanana/git/hub/xfstests/results//generic/501.out.bad ...
>     @@ -2,4 +2,4 @@
>      File bar digest before power failure:
>      69319d0343ab8f5ea564167da445addc  SCRATCH_MNT/bar
>      File bar digest after power failure:
>     -69319d0343ab8f5ea564167da445addc  SCRATCH_MNT/bar
>     +21de7d7325fe4dae1f3311d5a76f819f  SCRATCH_MNT/bar
>     ...
>     (Run 'diff -u /home/fdmanana/git/hub/xfstests/tests/generic/501.out ...
> Ran: generic/501
> Failures: generic/501
> Failed 1 of 1 tests
> 
> Without the commit reverted it passes as expected.
> 
> Reported-by: Murphy Zhou <jencce.kernel@gmail.com>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---

Looks good to me, although I think you don't need to list the whole test
process in the commit log. Anyway, that's fine for me.

Reviewed-by: Zorro Lang <zlang@redhat.com>

>  tests/generic/501     | 14 ++++++++------
>  tests/generic/501.out |  4 ++--
>  2 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/generic/501 b/tests/generic/501
> index 0d1f6ffe..bf020095 100755
> --- a/tests/generic/501
> +++ b/tests/generic/501
> @@ -42,14 +42,16 @@ _require_metadata_journaling $SCRATCH_DEV
>  _init_flakey
>  _mount_flakey
>  
> -$XFS_IO_PROG -f -c "pwrite -S 0x18 9000K 6908K" $SCRATCH_MNT/foo >>$seqres.full
> -$XFS_IO_PROG -f -c "pwrite -S 0x20 2572K 156K" $SCRATCH_MNT/bar >>$seqres.full
> +# Use file sizes and offsets/lengths for the clone operation that are multiples
> +# of 64Kb, so that the test works on machine with any page size.
> +$XFS_IO_PROG -f -s -c "pwrite -S 0x18 0 2M" $SCRATCH_MNT/foo >>$seqres.full
> +$XFS_IO_PROG -f -s -c "pwrite -S 0x20 0 20M" $SCRATCH_MNT/bar >>$seqres.full
>  
>  # We clone from file foo into a range of file bar that overlaps the existing
> -# extent at file bar. The destination offset of the reflink operation matches
> -# the eof position of file bar minus 4Kb.
> -$XFS_IO_PROG -c "fsync" \
> -	     -c "reflink ${SCRATCH_MNT}/foo 0 2724K 15908K" \
> +# extent at file bar. The clone operation must also extend the size of file bar.
> +# Note: in order to trigger the original bug on btrfs, the destination file size
> +# must be at least 16Mb and the destination file must have been fsynced before.
> +$XFS_IO_PROG -c "reflink ${SCRATCH_MNT}/foo 0 19M 2M" \
>  	     -c "fsync" \
>  	     $SCRATCH_MNT/bar >>$seqres.full
>  
> diff --git a/tests/generic/501.out b/tests/generic/501.out
> index 5d7da017..778aba4b 100644
> --- a/tests/generic/501.out
> +++ b/tests/generic/501.out
> @@ -1,5 +1,5 @@
>  QA output created by 501
>  File bar digest before power failure:
> -95a95813a8c2abc9aa75a6c2914a077e  SCRATCH_MNT/bar
> +69319d0343ab8f5ea564167da445addc  SCRATCH_MNT/bar
>  File bar digest after power failure:
> -95a95813a8c2abc9aa75a6c2914a077e  SCRATCH_MNT/bar
> +69319d0343ab8f5ea564167da445addc  SCRATCH_MNT/bar
> -- 
> 2.26.2
> 


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

* Re: [PATCH] generic/501: make the test work on machines with a non 4K page size
  2020-07-22 14:12 [PATCH] generic/501: make the test work on machines with a non 4K page size fdmanana
  2020-07-23  5:09 ` Zorro Lang
@ 2020-07-23  6:16 ` Murphy Zhou
  1 sibling, 0 replies; 3+ messages in thread
From: Murphy Zhou @ 2020-07-23  6:16 UTC (permalink / raw)
  To: fdmanana; +Cc: fstests, linux-btrfs, Filipe Manana, Murphy Zhou

On Wed, Jul 22, 2020 at 03:12:54PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Currently generic/501 fails on machines with a page size different from 4K
> (like ppc64le), because the clone operation fails with -EINVAL due to the
> fact we pass it an offset that is 4K aligned but not aligned to the page
> size of the machine.

Tested OK. Take much longer time though.

Ack. Thanks!
> 
> The test doesn't actually need offsets and lengths to be 4K aligned, so
> just update the test to use offsets and lenghts that work for page size.
> Also add a comment mentioning that a file size of at least 16Mb was a
> necessary condition to trigger the btrfs bug.
> 
> The test is a regression test for a btrfs issue fixed by kernel commit
> bd3599a0e142cd ("Btrfs: fix file data corruption after cloning a range
> and fsync"), which landed in kernel 4.18.
> 
> Since I couldn't compile a 4.17 kernel on debian testing, I tried this
> with a 4.18 kernel with that commit reverted, and it fails as expected
> on a x86_64 box:
> 
> $ ./check generic/501
> FSTYP         -- btrfs
> PLATFORM      -- Linux/x86_64 debian9 4.18.0-btrfs-next-64 #1 SMP (...)
> MKFS_OPTIONS  -- /dev/sdc
> MOUNT_OPTIONS -- /dev/sdc /home/fdmanana/btrfs-tests/scratch_1
> 
> generic/501 1s ... - output mismatch (see .../xfstests/results//generic/501.out.bad)
>     --- tests/generic/501.out	2020-07-22 14:50:12.585674202 +0100
>     +++ /home/fdmanana/git/hub/xfstests/results//generic/501.out.bad ...
>     @@ -2,4 +2,4 @@
>      File bar digest before power failure:
>      69319d0343ab8f5ea564167da445addc  SCRATCH_MNT/bar
>      File bar digest after power failure:
>     -69319d0343ab8f5ea564167da445addc  SCRATCH_MNT/bar
>     +21de7d7325fe4dae1f3311d5a76f819f  SCRATCH_MNT/bar
>     ...
>     (Run 'diff -u /home/fdmanana/git/hub/xfstests/tests/generic/501.out ...
> Ran: generic/501
> Failures: generic/501
> Failed 1 of 1 tests
> 
> Without the commit reverted it passes as expected.
> 
> Reported-by: Murphy Zhou <jencce.kernel@gmail.com>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
>  tests/generic/501     | 14 ++++++++------
>  tests/generic/501.out |  4 ++--
>  2 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/generic/501 b/tests/generic/501
> index 0d1f6ffe..bf020095 100755
> --- a/tests/generic/501
> +++ b/tests/generic/501
> @@ -42,14 +42,16 @@ _require_metadata_journaling $SCRATCH_DEV
>  _init_flakey
>  _mount_flakey
>  
> -$XFS_IO_PROG -f -c "pwrite -S 0x18 9000K 6908K" $SCRATCH_MNT/foo >>$seqres.full
> -$XFS_IO_PROG -f -c "pwrite -S 0x20 2572K 156K" $SCRATCH_MNT/bar >>$seqres.full
> +# Use file sizes and offsets/lengths for the clone operation that are multiples
> +# of 64Kb, so that the test works on machine with any page size.
> +$XFS_IO_PROG -f -s -c "pwrite -S 0x18 0 2M" $SCRATCH_MNT/foo >>$seqres.full
> +$XFS_IO_PROG -f -s -c "pwrite -S 0x20 0 20M" $SCRATCH_MNT/bar >>$seqres.full
>  
>  # We clone from file foo into a range of file bar that overlaps the existing
> -# extent at file bar. The destination offset of the reflink operation matches
> -# the eof position of file bar minus 4Kb.
> -$XFS_IO_PROG -c "fsync" \
> -	     -c "reflink ${SCRATCH_MNT}/foo 0 2724K 15908K" \
> +# extent at file bar. The clone operation must also extend the size of file bar.
> +# Note: in order to trigger the original bug on btrfs, the destination file size
> +# must be at least 16Mb and the destination file must have been fsynced before.
> +$XFS_IO_PROG -c "reflink ${SCRATCH_MNT}/foo 0 19M 2M" \
>  	     -c "fsync" \
>  	     $SCRATCH_MNT/bar >>$seqres.full
>  
> diff --git a/tests/generic/501.out b/tests/generic/501.out
> index 5d7da017..778aba4b 100644
> --- a/tests/generic/501.out
> +++ b/tests/generic/501.out
> @@ -1,5 +1,5 @@
>  QA output created by 501
>  File bar digest before power failure:
> -95a95813a8c2abc9aa75a6c2914a077e  SCRATCH_MNT/bar
> +69319d0343ab8f5ea564167da445addc  SCRATCH_MNT/bar
>  File bar digest after power failure:
> -95a95813a8c2abc9aa75a6c2914a077e  SCRATCH_MNT/bar
> +69319d0343ab8f5ea564167da445addc  SCRATCH_MNT/bar
> -- 
> 2.26.2
> 

-- 
Murphy

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

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-22 14:12 [PATCH] generic/501: make the test work on machines with a non 4K page size fdmanana
2020-07-23  5:09 ` Zorro Lang
2020-07-23  6:16 ` Murphy Zhou

FSTests Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/fstests/0 fstests/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 fstests fstests/ https://lore.kernel.org/fstests \
		fstests@vger.kernel.org
	public-inbox-index fstests

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.fstests


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git