Re: [PATCH 1/5] filemap: drain in-flight DIO before buffered writes

[Jan Kara]

On Sun 25-10-26 20:41:10, Jiale Yao wrote:
> An asynchronous direct write can remain in flight after its submitting
> thread releases the inode lock. A buffered write can then dirty page
> cache before the direct write completes its post-I/O invalidation. The
> invalidation finds dirty pages, reports a possible data corruption, and
> records -EIO in the mapping error sequence.

Well, when you do stupid things, you get stupid results. We were always
saying mixing buffered and direct IO in the same file isn't a good idea and
we try to not screw users too badly but give no promises. This falls into
that cathegory... So I don't think there's much to fix here. The only
sensible exception here are simple filesystems which have only simple
support for direct IO and fallback to buffered IO for the rest. But I think
we should handle these on a case-by-case basis and either drop the support
for direct IO altogether or convert them to iomap and properly handle the
waiting in their .write_iter methods (like e.g. ext2 does it).

								Honza

> Drain in-flight DIO before both regular buffered writes and direct I/O
> fallbacks in __generic_file_write_iter(). Preserve IOCB_NOWAIT semantics
> by returning -EAGAIN when waiting would be required, while retaining a
> positive partial direct write result.
> 
> Fixes: 8174202b34c3 ("write_iter variants of {__,}generic_file_aio_write()")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Jiale Yao <yaojiale02@xxxxxxx>
> ---
>  mm/filemap.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/mm/filemap.c b/mm/filemap.c
> index b74bc1e5015c..289b8ca44ada 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -4502,10 +4502,19 @@ ssize_t __generic_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>  		 */
>  		if (ret < 0 || !iov_iter_count(from) || IS_DAX(inode))
>  			return ret;
> +		if ((iocb->ki_flags & IOCB_NOWAIT) &&
> +		    atomic_read(&inode->i_dio_count))
> +			return ret ?: -EAGAIN;
> +		inode_dio_wait(inode);
>  		return direct_write_fallback(iocb, from, ret,
>  				generic_perform_write(iocb, from));
>  	}
>  
> +	if ((iocb->ki_flags & IOCB_NOWAIT) &&
> +	    atomic_read(&inode->i_dio_count))
> +		return -EAGAIN;
> +	inode_dio_wait(inode);
> +
>  	return generic_perform_write(iocb, from);
>  }
>  EXPORT_SYMBOL(__generic_file_write_iter);
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR