On Thu 03-10-19 21:34:18, Matthew Bobrowski wrote:
> In preparation for implementing the iomap direct I/O write path
> modifications, the inode extension/truncate code needs to be moved out
> from ext4_iomap_end(). For direct I/O, if the current code remained
> within ext4_iomap_end() it would behave incorrectly. Updating the
> inode size prior to converting unwritten extents to written extents
> will potentially allow a racing direct I/O read operation to find
> unwritten extents before they've been correctly converted.
>
> The inode extension/truncate code has been moved out into a new helper
> ext4_handle_inode_extension(). This function has been designed so that
> it can be used by both DAX and direct I/O paths.
>
> Signed-off-by: Matthew Bobrowski <mbobrowski@xxxxxxxxxxxxxx>
Looks good to me. Fell free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Just small nits below:
> +static int ext4_handle_inode_extension(struct inode *inode, loff_t offset,
> + ssize_t written, size_t count)
> +{
> + int ret = 0;
I think both the function and callsites may be slightly simpler if you let
the function return 'written' or error (not 0 or error). But I'll leave
that decision upto you.
> + handle_t *handle;
> + bool truncate = false;
> + u8 blkbits = inode->i_blkbits;
> + ext4_lblk_t written_blk, end_blk;
> +
> + /*
> + * Note that EXT4_I(inode)->i_disksize can get extended up to
> + * inode->i_size while the IO was running due to writeback of
> + * delalloc blocks. But the code in ext4_iomap_alloc() is careful
> + * to use zeroed / unwritten extents if this is possible and thus
> + * we won't leave uninitialized blocks in a file even if we didn't
> + * succeed in writing as much as we planned.
> + */
Whitespace damaged here...
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
