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

[Filipe Manana <fdmanana@kernel.org>]

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