Re: [PATCH] ext2: preserve IOMAP_F_DIRTY for extending direct writes

[Jan Kara]

 On Thu 29-10-26 01:36:16, Hungyu Lin wrote:
> ext2_iomap_begin() sets IOMAP_F_DIRTY for writes that may extend i_size,
> but clears the flag after ext2_get_blocks(). When the write uses an
> existing mapped block, iomap can then use FUA and skip
> generic_write_sync(), even though ext2_dio_write_end_io() updates the
> inode size. An O_DSYNC write can therefore return without persisting
> the new size.
> 
> For example, on a FUA-capable device, a 512-byte O_DIRECT|O_DSYNC write
> at offset 512 into a fsynced one-byte file returns successfully and
> extends the in-memory size to 1024, while an immediate read of the
> on-disk inode still shows size 1. An explicit fsync persists the new
> size.
> 
> Initialize iomap->flags before setting IOMAP_F_DIRTY so that the flag
> survives and the required metadata synchronization takes place.
> 
> Fixes: fb5de4358e1a ("ext2: Move direct-io to use iomap")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Hungyu Lin <dennylin0707@xxxxxxxxx>

Thanks for the fix. I've just move the setting of the IOMAP_F_DIRTY bit
instead of moving the initialization. Functionally this is equivalent and
the iomap modification is better localized that way (the resulting patch is
attached).

								Honza

> ---
>  fs/ext2/inode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index 12ac4cfe1500..ba23cadb157b 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -805,6 +805,7 @@ static int ext2_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
>  	    (first_block << blkbits) < i_size_read(inode))
>  		create = 0;
>  
> +	iomap->flags = 0;
>  	/*
>  	 * Writes that span EOF might trigger an IO size update on completion,
>  	 * so consider them to be dirty for the purposes of O_DSYNC even if
> @@ -818,7 +819,6 @@ static int ext2_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
>  	if (ret < 0)
>  		return ret;
>  
> -	iomap->flags = 0;
>  	iomap->offset = (u64)first_block << blkbits;
>  	iomap->bdev = inode->i_sb->s_bdev;
>  
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR