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.
