Re: [PATCH 05/13] btrfs: Add CoW in iomap based writes

[Dave Chinner <david@fromorbit.com>]

On Fri, Aug 02, 2019 at 05:00:40PM -0500, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
> 
> Set iomap->type to IOMAP_COW and fill up the source map in case
> the I/O is not page aligned.
.....
>  static void btrfs_buffered_page_done(struct inode *inode, loff_t pos,
>  		unsigned copied, struct page *page,
>  		struct iomap *iomap)
> @@ -188,6 +217,7 @@ static int btrfs_buffered_iomap_begin(struct inode *inode, loff_t pos,
>  	int ret;
>  	size_t write_bytes = length;
>  	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
> +	size_t end;
>  	size_t sector_offset = pos & (fs_info->sectorsize - 1);
>  	struct btrfs_iomap *bi;
>  
> @@ -255,6 +285,17 @@ static int btrfs_buffered_iomap_begin(struct inode *inode, loff_t pos,
>  	iomap->private = bi;
>  	iomap->length = round_up(write_bytes, fs_info->sectorsize);
>  	iomap->offset = round_down(pos, fs_info->sectorsize);
> +	end = pos + write_bytes;
> +	/* Set IOMAP_COW if start/end is not page aligned */
> +	if (((pos & (PAGE_SIZE - 1)) || (end & (PAGE_SIZE - 1)))) {
> +		iomap->type = IOMAP_COW;
> +		ret = get_iomap(inode, pos, length, srcmap);
> +		if (ret < 0)
> +			goto release;

I suspect you didn't test this case, because....

> +	} else {
> +		iomap->type = IOMAP_DELALLOC;
> +	}
> +
>  	iomap->addr = IOMAP_NULL_ADDR;
>  	iomap->type = IOMAP_DELALLOC;

The iomap->type is overwritten here and so IOMAP_COW will never be
seen by the iomap infrastructure...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com