FSTests Archive on lore.kernel.org
 help / color / Atom feed
* ext4 dio RWF_NOWAIT change
@ 2020-02-05  9:13 Murphy Zhou
  2020-02-07 15:38 ` Theodore Y. Ts'o
  0 siblings, 1 reply; 3+ messages in thread
From: Murphy Zhou @ 2020-02-05  9:13 UTC (permalink / raw)
  To: linux-ext4, fstests

Hi,

Kernel commit 378f32bab3714f04c4e0c3aee4129f6703805550
Author: Matthew Bobrowski <mbobrowski@mbobrowski.org>
Date:   Tue Nov 5 23:02:39 2019 +1100

    ext4: introduce direct I/O write using iomap infrastructure


Changed the logic of dio+RWF_NOWAIT

from:

-       if (!inode_trylock(inode)) {
-               if (iocb->ki_flags & IOCB_NOWAIT)
-                       return -EAGAIN;
-               inode_lock(inode);
-       }


to:

+       if (iocb->ki_flags & IOCB_NOWAIT) {
+               if (!inode_trylock(inode))
+                       return -EAGAIN;
+       } else {
+               inode_lock(inode);
+       }


fstests generic/471 expecet EAGAIN on this situation, so it started to
fail since than.

The current logic is similar with other filesystems, but only ext4 fails
on geneirc/471.

Any thoughts how to fix this?

Thanks,
Murphy

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

* Re: ext4 dio RWF_NOWAIT change
  2020-02-05  9:13 ext4 dio RWF_NOWAIT change Murphy Zhou
@ 2020-02-07 15:38 ` Theodore Y. Ts'o
  2020-02-12 10:50   ` Murphy Zhou
  0 siblings, 1 reply; 3+ messages in thread
From: Theodore Y. Ts'o @ 2020-02-07 15:38 UTC (permalink / raw)
  To: Murphy Zhou; +Cc: linux-ext4, fstests

On Wed, Feb 05, 2020 at 05:13:44PM +0800, Murphy Zhou wrote:
> Hi,
> 
> Kernel commit 378f32bab3714f04c4e0c3aee4129f6703805550
> Author: Matthew Bobrowski <mbobrowski@mbobrowski.org>
> Date:   Tue Nov 5 23:02:39 2019 +1100
> 
>     ext4: introduce direct I/O write using iomap infrastructure
> 
> 
> Changed the logic of dio+RWF_NOWAIT
> 
> from:
> 
> -       if (!inode_trylock(inode)) {
> -               if (iocb->ki_flags & IOCB_NOWAIT)
> -                       return -EAGAIN;
> -               inode_lock(inode);
> -       }
> 
> 
> to:
> 
> +       if (iocb->ki_flags & IOCB_NOWAIT) {
> +               if (!inode_trylock(inode))
> +                       return -EAGAIN;
> +       } else {
> +               inode_lock(inode);
> +       }
> 
> 
> fstests generic/471 expecet EAGAIN on this situation, so it started to
> fail since than.

I don't understand why this specific change would cause the situation.
In the generic/471 test, here iocb->ki_flags will have IOCB_NOWAIT
set, and in that case I don't see how there would be a change in
behavior with respect to EAGAIN being returned.

In any case, I've been suppressing generic/471 because of concerns
that the test is bogus so I hadn't noticed.  From
kvm-xfstests/test-appliance/files/root/fs/global_exclude:

# The test generic/471 tests the RWF_NOWAIT flag; however
# how it is supposed to work with file systems is disputed,
# and not all device drivers support it.  As a result
# it doesn't work if an LVM device driver is in use (as is the
# case with gce-xfstests).  So let's suppress it for now.  For
# more details see:
# https://lore.kernel.org/linux-block/20190723220502.GX7777@dread.disaster.area/
generic/471

							- Ted

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

* Re: ext4 dio RWF_NOWAIT change
  2020-02-07 15:38 ` Theodore Y. Ts'o
@ 2020-02-12 10:50   ` Murphy Zhou
  0 siblings, 0 replies; 3+ messages in thread
From: Murphy Zhou @ 2020-02-12 10:50 UTC (permalink / raw)
  To: Theodore Y. Ts'o; +Cc: Murphy Zhou, linux-ext4, fstests

On Fri, Feb 07, 2020 at 10:38:24AM -0500, Theodore Y. Ts'o wrote:
> On Wed, Feb 05, 2020 at 05:13:44PM +0800, Murphy Zhou wrote:
> > Hi,
> > 
> > Kernel commit 378f32bab3714f04c4e0c3aee4129f6703805550
> > Author: Matthew Bobrowski <mbobrowski@mbobrowski.org>
> > Date:   Tue Nov 5 23:02:39 2019 +1100
> > 
> >     ext4: introduce direct I/O write using iomap infrastructure
> > 
> > 
> > Changed the logic of dio+RWF_NOWAIT
> > 
> > from:
> > 
> > -       if (!inode_trylock(inode)) {
> > -               if (iocb->ki_flags & IOCB_NOWAIT)
> > -                       return -EAGAIN;
> > -               inode_lock(inode);
> > -       }
> > 
> > 
> > to:
> > 
> > +       if (iocb->ki_flags & IOCB_NOWAIT) {
> > +               if (!inode_trylock(inode))
> > +                       return -EAGAIN;
> > +       } else {
> > +               inode_lock(inode);
> > +       }
> > 
> > 
> > fstests generic/471 expecet EAGAIN on this situation, so it started to
> > fail since than.
> 
> I don't understand why this specific change would cause the situation.
> In the generic/471 test, here iocb->ki_flags will have IOCB_NOWAIT
> set, and in that case I don't see how there would be a change in
> behavior with respect to EAGAIN being returned.
> 
> In any case, I've been suppressing generic/471 because of concerns
> that the test is bogus so I hadn't noticed.  From
> kvm-xfstests/test-appliance/files/root/fs/global_exclude:
> 
> # The test generic/471 tests the RWF_NOWAIT flag; however
> # how it is supposed to work with file systems is disputed,
> # and not all device drivers support it.  As a result
> # it doesn't work if an LVM device driver is in use (as is the
> # case with gce-xfstests).  So let's suppress it for now.  For
> # more details see:
> # https://lore.kernel.org/linux-block/20190723220502.GX7777@dread.disaster.area/
> generic/471
> 
> 							- Ted

Thanks for all the details! Ted.

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-02-05  9:13 ext4 dio RWF_NOWAIT change Murphy Zhou
2020-02-07 15:38 ` Theodore Y. Ts'o
2020-02-12 10:50   ` 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