[PATCH] generic: test the correctness of several cases of RWF_NOWAIT writes

[PATCH] generic: test the correctness of several cases of RWF_NOWAIT writes

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Verify some attempts to write into a file using RWF_NOWAIT:

1) Writing into a fallocated extent that starts at eof should work;
2) Writing into a hole should fail;
3) Writing into a range that is partially allocated should fail.

This is motivated by several bugs that btrfs and ext4 had and were fixed
by the following kernel commits:

  4b1946284dd6 ("btrfs: fix failure of RWF_NOWAIT write into prealloc extent beyond eof")
  260a63395f90 ("btrfs: fix RWF_NOWAIT write not failling when we need to cow")
  0b3171b6d195 ("ext4: do not block RWF_NOWAIT dio write on unallocated space")

At the moment, on a 5.9-rc6 kernel at least, ext4 is failing for case 1),
but when I found and fixed case 1) in btrfs, around kernel 5.7, it was not
failing on ext4, so some regression happened in the meanwhile. For xfs and
btrfs on a 5.9 kernel, all the three cases pass.

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

diff --git a/tests/generic/613 b/tests/generic/613
new file mode 100755
index 00000000..931876dc
--- /dev/null
+++ b/tests/generic/613
@@ -0,0 +1,74 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2020 SUSE Linux Products GmbH. All Rights Reserved.
+#
+# FS QA Test No. 613
+#
+# Verify some attempts to write into a file using RWF_NOWAIT:
+#
+# 1) Writing into a fallocated extent that starts at eof should work;
+# 2) Writing into a hole should fail;
+# 3) Writing into a range that is partially allocated should fail.
+#
+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
+
+# real QA test starts here
+_supported_fs generic
+_require_scratch
+_require_odirect
+_require_xfs_io_command pwrite -N
+_require_xfs_io_command falloc -k
+_require_xfs_io_command fpunch
+
+rm -f $seqres.full
+
+_scratch_mkfs >>$seqres.full 2>&1
+_scratch_mount
+
+echo "Creating file"
+$XFS_IO_PROG -f -d -c "pwrite -S 0xab 0 512K" $SCRATCH_MNT/foo | _filter_xfs_io
+
+# Now add an unwritten extent using fallocate without bumping the file size and
+# then attempt to do a  RWF_NOWAIT write into this extent. It should not fail.
+echo "Writing into fallocated extent at eof"
+$XFS_IO_PROG -c "falloc -k 512K 512K" \
+	     -d -c "pwrite -N -V 1 -S 0xcd -b 512K 512K 512K" \
+	     $SCRATCH_MNT/foo | _filter_xfs_io
+
+# Now punch a hole and try a RWF_NOWAIT write into the hole. It should fail.
+echo "Writing into a hole"
+$XFS_IO_PROG -c "fpunch 0 256K" \
+	     -d -c "pwrite -N -V 1 -S 0xff -b 128K 0 128K" \
+	     $SCRATCH_MNT/foo | _filter_xfs_io
+
+# Allocate an extent for the first half of the hole, then attempt to write into
+# a range that covers the new extent and the hole. It should fail.
+echo "Writing into a range partially allocated (ending with a hole)"
+$XFS_IO_PROG -c "falloc 0 128K" \
+	     -d -c "pwrite -N -V 1 -S 0xef -b 256K 0 256K" \
+	     $SCRATCH_MNT/foo | _filter_xfs_io
+
+# Just double check none of the writes above that should fail did not change the
+# file data in an unexpected way. First 256K of file data should be all zeros,
+# the range from 256K to 512K should have all bytes with a value of 0xab and the
+# range from 512K to 1M should have all bytes with a value of 0xcd.
+echo "Final file content:"
+od -A d -t x1 $SCRATCH_MNT/foo
+
+status=0
+exit
diff --git a/tests/generic/613.out b/tests/generic/613.out
new file mode 100644
index 00000000..a542fa5b
--- /dev/null
+++ b/tests/generic/613.out
@@ -0,0 +1,19 @@
+QA output created by 613
+Creating file
+wrote 524288/524288 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Writing into fallocated extent at eof
+wrote 524288/524288 bytes at offset 524288
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Writing into a hole
+pwrite: Resource temporarily unavailable
+Writing into a range partially allocated (ending with a hole)
+pwrite: Resource temporarily unavailable
+Final file content:
+0000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+*
+0262144 ab ab ab ab ab ab ab ab ab ab ab ab ab ab ab ab
+*
+0524288 cd cd cd cd cd cd cd cd cd cd cd cd cd cd cd cd
+*
+1048576
diff --git a/tests/generic/group b/tests/generic/group
index 8054d874..b8bf8ec1 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -615,3 +615,4 @@
 610 auto quick prealloc zero
 611 auto quick attr
 612 auto quick clone
+613 auto quick rw prealloc punch
-- 
2.28.0

Re: [PATCH] generic: test the correctness of several cases of RWF_NOWAIT writes

[Jan Kara]

On Thu 15-10-20 16:36:38, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Verify some attempts to write into a file using RWF_NOWAIT:
> 
> 1) Writing into a fallocated extent that starts at eof should work;

Why? We need to update i_size which requires transaction start and e.g.
ext4 does not support that in non-blocking mode...

								Honza

> 2) Writing into a hole should fail;
> 3) Writing into a range that is partially allocated should fail.
> 
> This is motivated by several bugs that btrfs and ext4 had and were fixed
> by the following kernel commits:
> 
>   4b1946284dd6 ("btrfs: fix failure of RWF_NOWAIT write into prealloc extent beyond eof")
>   260a63395f90 ("btrfs: fix RWF_NOWAIT write not failling when we need to cow")
>   0b3171b6d195 ("ext4: do not block RWF_NOWAIT dio write on unallocated space")
> 
> At the moment, on a 5.9-rc6 kernel at least, ext4 is failing for case 1),
> but when I found and fixed case 1) in btrfs, around kernel 5.7, it was not
> failing on ext4, so some regression happened in the meanwhile. For xfs and
> btrfs on a 5.9 kernel, all the three cases pass.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
>  tests/generic/613     | 74 +++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/613.out | 19 +++++++++++
>  tests/generic/group   |  1 +
>  3 files changed, 94 insertions(+)
>  create mode 100755 tests/generic/613
>  create mode 100644 tests/generic/613.out
> 
> diff --git a/tests/generic/613 b/tests/generic/613
> new file mode 100755
> index 00000000..931876dc
> --- /dev/null
> +++ b/tests/generic/613
> @@ -0,0 +1,74 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2020 SUSE Linux Products GmbH. All Rights Reserved.
> +#
> +# FS QA Test No. 613
> +#
> +# Verify some attempts to write into a file using RWF_NOWAIT:
> +#
> +# 1) Writing into a fallocated extent that starts at eof should work;
> +# 2) Writing into a hole should fail;
> +# 3) Writing into a range that is partially allocated should fail.
> +#
> +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
> +
> +# real QA test starts here
> +_supported_fs generic
> +_require_scratch
> +_require_odirect
> +_require_xfs_io_command pwrite -N
> +_require_xfs_io_command falloc -k
> +_require_xfs_io_command fpunch
> +
> +rm -f $seqres.full
> +
> +_scratch_mkfs >>$seqres.full 2>&1
> +_scratch_mount
> +
> +echo "Creating file"
> +$XFS_IO_PROG -f -d -c "pwrite -S 0xab 0 512K" $SCRATCH_MNT/foo | _filter_xfs_io
> +
> +# Now add an unwritten extent using fallocate without bumping the file size and
> +# then attempt to do a  RWF_NOWAIT write into this extent. It should not fail.
> +echo "Writing into fallocated extent at eof"
> +$XFS_IO_PROG -c "falloc -k 512K 512K" \
> +	     -d -c "pwrite -N -V 1 -S 0xcd -b 512K 512K 512K" \
> +	     $SCRATCH_MNT/foo | _filter_xfs_io
> +
> +# Now punch a hole and try a RWF_NOWAIT write into the hole. It should fail.
> +echo "Writing into a hole"
> +$XFS_IO_PROG -c "fpunch 0 256K" \
> +	     -d -c "pwrite -N -V 1 -S 0xff -b 128K 0 128K" \
> +	     $SCRATCH_MNT/foo | _filter_xfs_io
> +
> +# Allocate an extent for the first half of the hole, then attempt to write into
> +# a range that covers the new extent and the hole. It should fail.
> +echo "Writing into a range partially allocated (ending with a hole)"
> +$XFS_IO_PROG -c "falloc 0 128K" \
> +	     -d -c "pwrite -N -V 1 -S 0xef -b 256K 0 256K" \
> +	     $SCRATCH_MNT/foo | _filter_xfs_io
> +
> +# Just double check none of the writes above that should fail did not change the
> +# file data in an unexpected way. First 256K of file data should be all zeros,
> +# the range from 256K to 512K should have all bytes with a value of 0xab and the
> +# range from 512K to 1M should have all bytes with a value of 0xcd.
> +echo "Final file content:"
> +od -A d -t x1 $SCRATCH_MNT/foo
> +
> +status=0
> +exit
> diff --git a/tests/generic/613.out b/tests/generic/613.out
> new file mode 100644
> index 00000000..a542fa5b
> --- /dev/null
> +++ b/tests/generic/613.out
> @@ -0,0 +1,19 @@
> +QA output created by 613
> +Creating file
> +wrote 524288/524288 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Writing into fallocated extent at eof
> +wrote 524288/524288 bytes at offset 524288
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Writing into a hole
> +pwrite: Resource temporarily unavailable
> +Writing into a range partially allocated (ending with a hole)
> +pwrite: Resource temporarily unavailable
> +Final file content:
> +0000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> +*
> +0262144 ab ab ab ab ab ab ab ab ab ab ab ab ab ab ab ab
> +*
> +0524288 cd cd cd cd cd cd cd cd cd cd cd cd cd cd cd cd
> +*
> +1048576
> diff --git a/tests/generic/group b/tests/generic/group
> index 8054d874..b8bf8ec1 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -615,3 +615,4 @@
>  610 auto quick prealloc zero
>  611 auto quick attr
>  612 auto quick clone
> +613 auto quick rw prealloc punch
> -- 
> 2.28.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] generic: test the correctness of several cases of RWF_NOWAIT writes

[Filipe Manana]

On Thu, Oct 15, 2020 at 5:13 PM Jan Kara <jack@suse.cz> wrote:
>
> On Thu 15-10-20 16:36:38, fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> >
> > Verify some attempts to write into a file using RWF_NOWAIT:
> >
> > 1) Writing into a fallocated extent that starts at eof should work;
>
> Why? We need to update i_size which requires transaction start and e.g.
> ext4 does not support that in non-blocking mode...

Ah, ok it makes sense then. In btrfs we do it asynchronously (in a
work queue) and always have to update the inode metadata anyway.
Ok, I'll remove that case from the test then.

Thanks.

>
>                                                                 Honza
>
> > 2) Writing into a hole should fail;
> > 3) Writing into a range that is partially allocated should fail.
> >
> > This is motivated by several bugs that btrfs and ext4 had and were fixed
> > by the following kernel commits:
> >
> >   4b1946284dd6 ("btrfs: fix failure of RWF_NOWAIT write into prealloc extent beyond eof")
> >   260a63395f90 ("btrfs: fix RWF_NOWAIT write not failling when we need to cow")
> >   0b3171b6d195 ("ext4: do not block RWF_NOWAIT dio write on unallocated space")
> >
> > At the moment, on a 5.9-rc6 kernel at least, ext4 is failing for case 1),
> > but when I found and fixed case 1) in btrfs, around kernel 5.7, it was not
> > failing on ext4, so some regression happened in the meanwhile. For xfs and
> > btrfs on a 5.9 kernel, all the three cases pass.
> >
> > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > ---
> >  tests/generic/613     | 74 +++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/613.out | 19 +++++++++++
> >  tests/generic/group   |  1 +
> >  3 files changed, 94 insertions(+)
> >  create mode 100755 tests/generic/613
> >  create mode 100644 tests/generic/613.out
> >
> > diff --git a/tests/generic/613 b/tests/generic/613
> > new file mode 100755
> > index 00000000..931876dc
> > --- /dev/null
> > +++ b/tests/generic/613
> > @@ -0,0 +1,74 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (C) 2020 SUSE Linux Products GmbH. All Rights Reserved.
> > +#
> > +# FS QA Test No. 613
> > +#
> > +# Verify some attempts to write into a file using RWF_NOWAIT:
> > +#
> > +# 1) Writing into a fallocated extent that starts at eof should work;
> > +# 2) Writing into a hole should fail;
> > +# 3) Writing into a range that is partially allocated should fail.
> > +#
> > +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
> > +
> > +# real QA test starts here
> > +_supported_fs generic
> > +_require_scratch
> > +_require_odirect
> > +_require_xfs_io_command pwrite -N
> > +_require_xfs_io_command falloc -k
> > +_require_xfs_io_command fpunch
> > +
> > +rm -f $seqres.full
> > +
> > +_scratch_mkfs >>$seqres.full 2>&1
> > +_scratch_mount
> > +
> > +echo "Creating file"
> > +$XFS_IO_PROG -f -d -c "pwrite -S 0xab 0 512K" $SCRATCH_MNT/foo | _filter_xfs_io
> > +
> > +# Now add an unwritten extent using fallocate without bumping the file size and
> > +# then attempt to do a  RWF_NOWAIT write into this extent. It should not fail.
> > +echo "Writing into fallocated extent at eof"
> > +$XFS_IO_PROG -c "falloc -k 512K 512K" \
> > +          -d -c "pwrite -N -V 1 -S 0xcd -b 512K 512K 512K" \
> > +          $SCRATCH_MNT/foo | _filter_xfs_io
> > +
> > +# Now punch a hole and try a RWF_NOWAIT write into the hole. It should fail.
> > +echo "Writing into a hole"
> > +$XFS_IO_PROG -c "fpunch 0 256K" \
> > +          -d -c "pwrite -N -V 1 -S 0xff -b 128K 0 128K" \
> > +          $SCRATCH_MNT/foo | _filter_xfs_io
> > +
> > +# Allocate an extent for the first half of the hole, then attempt to write into
> > +# a range that covers the new extent and the hole. It should fail.
> > +echo "Writing into a range partially allocated (ending with a hole)"
> > +$XFS_IO_PROG -c "falloc 0 128K" \
> > +          -d -c "pwrite -N -V 1 -S 0xef -b 256K 0 256K" \
> > +          $SCRATCH_MNT/foo | _filter_xfs_io
> > +
> > +# Just double check none of the writes above that should fail did not change the
> > +# file data in an unexpected way. First 256K of file data should be all zeros,
> > +# the range from 256K to 512K should have all bytes with a value of 0xab and the
> > +# range from 512K to 1M should have all bytes with a value of 0xcd.
> > +echo "Final file content:"
> > +od -A d -t x1 $SCRATCH_MNT/foo
> > +
> > +status=0
> > +exit
> > diff --git a/tests/generic/613.out b/tests/generic/613.out
> > new file mode 100644
> > index 00000000..a542fa5b
> > --- /dev/null
> > +++ b/tests/generic/613.out
> > @@ -0,0 +1,19 @@
> > +QA output created by 613
> > +Creating file
> > +wrote 524288/524288 bytes at offset 0
> > +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> > +Writing into fallocated extent at eof
> > +wrote 524288/524288 bytes at offset 524288
> > +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> > +Writing into a hole
> > +pwrite: Resource temporarily unavailable
> > +Writing into a range partially allocated (ending with a hole)
> > +pwrite: Resource temporarily unavailable
> > +Final file content:
> > +0000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > +*
> > +0262144 ab ab ab ab ab ab ab ab ab ab ab ab ab ab ab ab
> > +*
> > +0524288 cd cd cd cd cd cd cd cd cd cd cd cd cd cd cd cd
> > +*
> > +1048576
> > diff --git a/tests/generic/group b/tests/generic/group
> > index 8054d874..b8bf8ec1 100644
> > --- a/tests/generic/group
> > +++ b/tests/generic/group
> > @@ -615,3 +615,4 @@
> >  610 auto quick prealloc zero
> >  611 auto quick attr
> >  612 auto quick clone
> > +613 auto quick rw prealloc punch
> > --
> > 2.28.0
> >
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR

Re: [PATCH] generic: test the correctness of several cases of RWF_NOWAIT writes

[Dave Chinner]

On Thu, Oct 15, 2020 at 06:13:56PM +0200, Jan Kara wrote:
> On Thu 15-10-20 16:36:38, fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> > 
> > Verify some attempts to write into a file using RWF_NOWAIT:
> > 
> > 1) Writing into a fallocated extent that starts at eof should work;
> 
> Why? We need to update i_size which requires transaction start and e.g.
> ext4 does not support that in non-blocking mode...

Right, different filesystems behave differently given similar
pre-conditions. That's not a bug, that's exactly how RWF_NOWAIT is
expected to be implemented by each filesystem....

> > 2) Writing into a hole should fail;
> > 3) Writing into a range that is partially allocated should fail.

Same for these - these are situations where a -specific filesystem
implementation- might block, not a situation where the RWF_NOWAIT
API specification says that IO submission "should fail" and hence
return EAGAIN.

> > This is motivated by several bugs that btrfs and ext4 had and were fixed
> > by the following kernel commits:
> > 
> >   4b1946284dd6 ("btrfs: fix failure of RWF_NOWAIT write into prealloc extent beyond eof")
> >   260a63395f90 ("btrfs: fix RWF_NOWAIT write not failling when we need to cow")
> >   0b3171b6d195 ("ext4: do not block RWF_NOWAIT dio write on unallocated space")
> > 
> > At the moment, on a 5.9-rc6 kernel at least, ext4 is failing for case 1),
> > but when I found and fixed case 1) in btrfs, around kernel 5.7, it was not
> > failing on ext4, so some regression happened in the meanwhile. For xfs and
> > btrfs on a 5.9 kernel, all the three cases pass.

Sure, until we propagate IOMAP_NOWAIT far enough into the allocation
code that allocation will either succeed without blocking or fail
without changing anything.  At which point, the filesystem behaviour
is absolutely correct according to the RWF_NOWAIT specification, but
the test is most definitely wrong.

IOWs, I think any test that says "RWF_NOWAIT IO in a <specific
situation> must do <specific thing>" is incorrect. RWF_NOWAIT simply
does not not define behaviour like this, and different filesystems
will do different things given the same file layouts...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] generic: test the correctness of several cases of RWF_NOWAIT writes

[Jan Kara]

On Fri 16-10-20 16:57:57, Dave Chinner wrote:
> On Thu, Oct 15, 2020 at 06:13:56PM +0200, Jan Kara wrote:
> > On Thu 15-10-20 16:36:38, fdmanana@kernel.org wrote:
> > > From: Filipe Manana <fdmanana@suse.com>
> > > 
> > > Verify some attempts to write into a file using RWF_NOWAIT:
> > > 
> > > 1) Writing into a fallocated extent that starts at eof should work;
> > 
> > Why? We need to update i_size which requires transaction start and e.g.
> > ext4 does not support that in non-blocking mode...
> 
> Right, different filesystems behave differently given similar
> pre-conditions. That's not a bug, that's exactly how RWF_NOWAIT is
> expected to be implemented by each filesystem....
> 
> > > 2) Writing into a hole should fail;
> > > 3) Writing into a range that is partially allocated should fail.
> 
> Same for these - these are situations where a -specific filesystem
> implementation- might block, not a situation where the RWF_NOWAIT
> API specification says that IO submission "should fail" and hence
> return EAGAIN.
> 
> > > This is motivated by several bugs that btrfs and ext4 had and were fixed
> > > by the following kernel commits:
> > > 
> > >   4b1946284dd6 ("btrfs: fix failure of RWF_NOWAIT write into prealloc extent beyond eof")
> > >   260a63395f90 ("btrfs: fix RWF_NOWAIT write not failling when we need to cow")
> > >   0b3171b6d195 ("ext4: do not block RWF_NOWAIT dio write on unallocated space")
> > > 
> > > At the moment, on a 5.9-rc6 kernel at least, ext4 is failing for case 1),
> > > but when I found and fixed case 1) in btrfs, around kernel 5.7, it was not
> > > failing on ext4, so some regression happened in the meanwhile. For xfs and
> > > btrfs on a 5.9 kernel, all the three cases pass.
> 
> Sure, until we propagate IOMAP_NOWAIT far enough into the allocation
> code that allocation will either succeed without blocking or fail
> without changing anything.  At which point, the filesystem behaviour
> is absolutely correct according to the RWF_NOWAIT specification, but
> the test is most definitely wrong.
> 
> IOWs, I think any test that says "RWF_NOWAIT IO in a <specific
> situation> must do <specific thing>" is incorrect. RWF_NOWAIT simply
> does not not define behaviour like this, and different filesystems
> will do different things given the same file layouts...

I agree with this. That being said it would be still worthwhile to have
some tests verifying RWF_NOWAIT behavior is sane - that we don't block with
RWF_NOWAIT (this is a requirement), and that what used to work with
RWF_NOWAIT didn't unexpectedly regress (this is more a sanity check)... I'm
not sure how to test that in an automated way through.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] generic: test the correctness of several cases of RWF_NOWAIT writes

[Filipe Manana]

On Fri, Oct 16, 2020 at 9:46 AM Jan Kara <jack@suse.cz> wrote:
>
> On Fri 16-10-20 16:57:57, Dave Chinner wrote:
> > On Thu, Oct 15, 2020 at 06:13:56PM +0200, Jan Kara wrote:
> > > On Thu 15-10-20 16:36:38, fdmanana@kernel.org wrote:
> > > > From: Filipe Manana <fdmanana@suse.com>
> > > >
> > > > Verify some attempts to write into a file using RWF_NOWAIT:
> > > >
> > > > 1) Writing into a fallocated extent that starts at eof should work;
> > >
> > > Why? We need to update i_size which requires transaction start and e.g.
> > > ext4 does not support that in non-blocking mode...
> >
> > Right, different filesystems behave differently given similar
> > pre-conditions. That's not a bug, that's exactly how RWF_NOWAIT is
> > expected to be implemented by each filesystem....
> >
> > > > 2) Writing into a hole should fail;
> > > > 3) Writing into a range that is partially allocated should fail.
> >
> > Same for these - these are situations where a -specific filesystem
> > implementation- might block, not a situation where the RWF_NOWAIT
> > API specification says that IO submission "should fail" and hence
> > return EAGAIN.
> >
> > > > This is motivated by several bugs that btrfs and ext4 had and were fixed
> > > > by the following kernel commits:
> > > >
> > > >   4b1946284dd6 ("btrfs: fix failure of RWF_NOWAIT write into prealloc extent beyond eof")
> > > >   260a63395f90 ("btrfs: fix RWF_NOWAIT write not failling when we need to cow")
> > > >   0b3171b6d195 ("ext4: do not block RWF_NOWAIT dio write on unallocated space")
> > > >
> > > > At the moment, on a 5.9-rc6 kernel at least, ext4 is failing for case 1),
> > > > but when I found and fixed case 1) in btrfs, around kernel 5.7, it was not
> > > > failing on ext4, so some regression happened in the meanwhile. For xfs and
> > > > btrfs on a 5.9 kernel, all the three cases pass.
> >
> > Sure, until we propagate IOMAP_NOWAIT far enough into the allocation
> > code that allocation will either succeed without blocking or fail
> > without changing anything.  At which point, the filesystem behaviour
> > is absolutely correct according to the RWF_NOWAIT specification, but
> > the test is most definitely wrong.
> >
> > IOWs, I think any test that says "RWF_NOWAIT IO in a <specific
> > situation> must do <specific thing>" is incorrect. RWF_NOWAIT simply
> > does not not define behaviour like this, and different filesystems
> > will do different things given the same file layouts...
>
> I agree with this. That being said it would be still worthwhile to have
> some tests verifying RWF_NOWAIT behavior is sane - that we don't block with
> RWF_NOWAIT (this is a requirement), and that what used to work with
> RWF_NOWAIT didn't unexpectedly regress (this is more a sanity check)... I'm
> not sure how to test that in an automated way through.

Yes, my intention was to serve more as a regression test than anything
else (as it's not a new feature anyway).
I only excluded here cases that are obviously btrfs specific like when
trying to write into a file range that has extents shared by
snapshots.

Thanks.

>
>                                                                 Honza
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR