Re: [PATCH 1/6] iomap: Use a IOMAP_COW/srcmap for a read-modify-write I/O

[Goldwyn Rodrigues <rgoldwyn@suse.de>]

On  9:07 24/06, Christoph Hellwig wrote:
> xfs will need to be updated to fill in the additional iomap for the
> COW case.  Has this series been tested on xfs?
> 

No, I have not tested this, or make xfs set IOMAP_COW. I will try to do
it in the next iteration.

> I can't say I'm a huge fan of this two iomaps in one method call
> approach.  I always though two separate iomap iterations would be nicer,
> but compared to that even the older hack with just the additional
> src_addr seems a little better.

I am just expanding on your idea of using multiple iterations for the Cow case
in the hope we can come out of a good design:

1. iomap_file_buffered_write calls iomap_apply with IOMAP_WRITE flag.
   which calls iomap_begin() for the respective filesystem.
2. btrfs_iomap_begin() sets up iomap->type as IOMAP_COW and fills iomap
   struct with read addr information.
3. iomap_apply() conditionally for IOMAP_COW calls do_cow(new function)
   and calls ops->iomap_begin() with flag IOMAP_COW_READ_DONE(new flag).
4. btrfs_iomap_begin() fills up iomap structure with write information.

Step 3 seems out of place because iomap_apply should be iomap.type agnostic.
Right?
Should we be adding another flag IOMAP_COW_DONE, just to figure out that
this is the "real" write for iomap_begin to fill iomap?

If this is not how you imagined, could you elaborate on the dual iteration
sequence?


-- 
Goldwyn