Re: [RFCv1][WIP] ext2: Move direct-io to use iomap

[Christoph Hellwig <hch@infradead.org>]

On Thu, Mar 16, 2023 at 08:10:29PM +0530, Ritesh Harjani (IBM) wrote:
> +extern void ext2_write_failed(struct address_space *mapping, loff_t to);

Nit: please don't bother with extents for function prototypes.

> +static int ext2_dio_write_end_io(struct kiocb *iocb, ssize_t size,
> +				 int error, unsigned int flags)
> +{
> +	loff_t pos = iocb->ki_pos;
> +	struct inode *inode = file_inode(iocb->ki_filp);
> +
> +	if (error)
> +		return error;
> +
> +	pos += size;
> +	if (pos > i_size_read(inode))
> +		i_size_write(inode, pos);

Doesn't i_size_write need i_mutex protection?

> +	/*
> +	 * We pass IOMAP_DIO_NOSYNC because otherwise iomap_dio_rw()
> +	 * calls for generic_write_sync in iomap_dio_complete().
> +	 * Since ext2_fsync nmust be called w/o inode lock,
> +	 * hence we pass IOMAP_DIO_NOSYNC and handle generic_write_sync()
> +	 * ourselves.
> +	 */
> +	flags = IOMAP_DIO_NOSYNC;

So we added IOMAP_DIO_NOSYNC for btrfs initially, but even btrfs did
not manage to mak it work.  I suspect the right thing here as well
is to call __iomap_dio_rw and then separately after dropping the
lock.  Without that you for example can't take i_mutex in the
end_io handler, which I think we need to do.

We should then remove IOMAP_DIO_NOSYNC again.