[patch] dax: pass NOWAIT flag to iomap_apply

[patch] dax: pass NOWAIT flag to iomap_apply

[Jeff Moyer]

fstests generic/471 reports a failure when run with MOUNT_OPTIONS="-o
dax".  The reason is that the initial pwrite to an empty file with the
RWF_NOWAIT flag set does not return -EAGAIN.  It turns out that
dax_iomap_rw doesn't pass that flag through to iomap_apply.

With this patch applied, generic/471 passes for me.

Signed-off-by: Jeff Moyer <jmoyer@redhat.com>

diff --git a/fs/dax.c b/fs/dax.c
index 1f1f0201cad1..0b0d8819cb1b 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -1207,6 +1207,9 @@ dax_iomap_rw(struct kiocb *iocb, struct iov_iter *iter,
 		lockdep_assert_held(&inode->i_rwsem);
 	}
 
+	if (iocb->ki_flags & IOCB_NOWAIT)
+		flags |= IOMAP_NOWAIT;
+
 	while (iov_iter_count(iter)) {
 		ret = iomap_apply(inode, pos, iov_iter_count(iter), flags, ops,
 				iter, dax_iomap_actor);

Re: [patch] dax: pass NOWAIT flag to iomap_apply

[Christoph Hellwig]

On Wed, Feb 05, 2020 at 02:15:58PM -0500, Jeff Moyer wrote:
> fstests generic/471 reports a failure when run with MOUNT_OPTIONS="-o
> dax".  The reason is that the initial pwrite to an empty file with the
> RWF_NOWAIT flag set does not return -EAGAIN.  It turns out that
> dax_iomap_rw doesn't pass that flag through to iomap_apply.
> 
> With this patch applied, generic/471 passes for me.
> 
> Signed-off-by: Jeff Moyer <jmoyer@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [patch] dax: pass NOWAIT flag to iomap_apply

[Dan Williams]

On Wed, Feb 5, 2020 at 11:28 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Wed, Feb 05, 2020 at 02:15:58PM -0500, Jeff Moyer wrote:
> > fstests generic/471 reports a failure when run with MOUNT_OPTIONS="-o
> > dax".  The reason is that the initial pwrite to an empty file with the
> > RWF_NOWAIT flag set does not return -EAGAIN.  It turns out that
> > dax_iomap_rw doesn't pass that flag through to iomap_apply.
> >
> > With this patch applied, generic/471 passes for me.
> >
> > Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
>
> Looks good,
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Applied.

Re: [patch] dax: pass NOWAIT flag to iomap_apply

[Jan Kara]

On Wed 05-02-20 14:15:58, Jeff Moyer wrote:
> fstests generic/471 reports a failure when run with MOUNT_OPTIONS="-o
> dax".  The reason is that the initial pwrite to an empty file with the
> RWF_NOWAIT flag set does not return -EAGAIN.  It turns out that
> dax_iomap_rw doesn't pass that flag through to iomap_apply.
> 
> With this patch applied, generic/471 passes for me.
> 
> Signed-off-by: Jeff Moyer <jmoyer@redhat.com>

The patch looks good to me. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

BTW, I've just noticed ext4 seems to be buggy in this regard and even this
patch doesn't fix it. So I guess you've been using XFS for testing this?

									Honza

> diff --git a/fs/dax.c b/fs/dax.c
> index 1f1f0201cad1..0b0d8819cb1b 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -1207,6 +1207,9 @@ dax_iomap_rw(struct kiocb *iocb, struct iov_iter *iter,
>  		lockdep_assert_held(&inode->i_rwsem);
>  	}
>  
> +	if (iocb->ki_flags & IOCB_NOWAIT)
> +		flags |= IOMAP_NOWAIT;
> +
>  	while (iov_iter_count(iter)) {
>  		ret = iomap_apply(inode, pos, iov_iter_count(iter), flags, ops,
>  				iter, dax_iomap_actor);
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [patch] dax: pass NOWAIT flag to iomap_apply

[Jeff Moyer]

Jan Kara <jack@suse.cz> writes:

> On Wed 05-02-20 14:15:58, Jeff Moyer wrote:
>> fstests generic/471 reports a failure when run with MOUNT_OPTIONS="-o
>> dax".  The reason is that the initial pwrite to an empty file with the
>> RWF_NOWAIT flag set does not return -EAGAIN.  It turns out that
>> dax_iomap_rw doesn't pass that flag through to iomap_apply.
>> 
>> With this patch applied, generic/471 passes for me.
>> 
>> Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
>
> The patch looks good to me. You can add:
>
> Reviewed-by: Jan Kara <jack@suse.cz>
>
> BTW, I've just noticed ext4 seems to be buggy in this regard and even this
> patch doesn't fix it. So I guess you've been using XFS for testing this?

That's right, sorry I didn't mention that.  Will you send a patch for
ext4, or do you want me to look into it?

Thanks!
Jeff

Re: [patch] dax: pass NOWAIT flag to iomap_apply

[Jan Kara]

On Thu 06-02-20 09:33:39, Jeff Moyer wrote:
> Jan Kara <jack@suse.cz> writes:
> 
> > On Wed 05-02-20 14:15:58, Jeff Moyer wrote:
> >> fstests generic/471 reports a failure when run with MOUNT_OPTIONS="-o
> >> dax".  The reason is that the initial pwrite to an empty file with the
> >> RWF_NOWAIT flag set does not return -EAGAIN.  It turns out that
> >> dax_iomap_rw doesn't pass that flag through to iomap_apply.
> >> 
> >> With this patch applied, generic/471 passes for me.
> >> 
> >> Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
> >
> > The patch looks good to me. You can add:
> >
> > Reviewed-by: Jan Kara <jack@suse.cz>
> >
> > BTW, I've just noticed ext4 seems to be buggy in this regard and even this
> > patch doesn't fix it. So I guess you've been using XFS for testing this?
> 
> That's right, sorry I didn't mention that.  Will you send a patch for
> ext4, or do you want me to look into it?

I've taken down a note in todo list to eventually look into that but if you
can have a look, I'm more than happy to remove that entry :).

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

Re: [patch] dax: pass NOWAIT flag to iomap_apply

[Jeff Moyer]

Jan Kara <jack@suse.cz> writes:

> On Thu 06-02-20 09:33:39, Jeff Moyer wrote:
>> Jan Kara <jack@suse.cz> writes:
>> 
>> > On Wed 05-02-20 14:15:58, Jeff Moyer wrote:
>> >> fstests generic/471 reports a failure when run with MOUNT_OPTIONS="-o
>> >> dax".  The reason is that the initial pwrite to an empty file with the
>> >> RWF_NOWAIT flag set does not return -EAGAIN.  It turns out that
>> >> dax_iomap_rw doesn't pass that flag through to iomap_apply.
>> >> 
>> >> With this patch applied, generic/471 passes for me.
>> >> 
>> >> Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
>> >
>> > The patch looks good to me. You can add:
>> >
>> > Reviewed-by: Jan Kara <jack@suse.cz>
>> >
>> > BTW, I've just noticed ext4 seems to be buggy in this regard and even this
>> > patch doesn't fix it. So I guess you've been using XFS for testing this?
>> 
>> That's right, sorry I didn't mention that.  Will you send a patch for
>> ext4, or do you want me to look into it?
>
> I've taken down a note in todo list to eventually look into that but if you
> can have a look, I'm more than happy to remove that entry :).

OK, I'll take a look.

-Jeff