Re: [PATCH 04/15] dax: Introduce IOMAP_F_COW for copy-on-write

[Dave Chinner <david@fromorbit.com>]

On Tue, Apr 02, 2019 at 08:56:31PM -0500, Goldwyn Rodrigues wrote:
> On 10:06 02/04, Dave Chinner wrote:
> > On Mon, Apr 01, 2019 at 04:41:02PM -0500, Goldwyn Rodrigues wrote:
> > > After Darrick's suggestion, we can even do away with cow_pos, so
> > > only the read address of cow_addr will exist.
> > 
> > As I mentioned earlier, even that is not necessary.
> > 
> > This is DAX - the iomap API and mapping functions can already return
> > pointers to inline data, and DAX can effectively be considered
> > inline data for the purposes of reading data.
> > 
> > As I said, the problem here is you are trying to use flags to define
> > a new type of iomap operation requires two mappings rather than one.
> > IMO, we should be defining an IOMAP_DAX_COW /type/ and then define
> > it to contain and behave as follows:
> > 
> > 	- new destination region for data to be copied into is the
> > 	  same setup as IOMAP_MAPPED
> > 	- existing shared data that may be needed for reading is
> > 	  mapped direct to device address by ->iomap_begin into
> > 	  iomap->inline_data
> > 	- if the iomap infrastructure needs to copy original source
> > 	  data into destination, it copies directly from the memory
> > 	  address in iomap->inline_data into the directly mapped DAX
> > 	  desitination via memcpy().
> > 
> > This covers both the partial write COW case you are concerned with
> > here, and the full-extent range copy case that
> > FALLOC_FL_UNSHARE_RANGE requires.
> > 
> 
> Ok, understood. However, we may have to differentiate between a CoW
> with FALLOC_FL_UNSHARE_RANGE because CoW will only (conditionally)
> copy the first and the last block wheras FALLOC_FL_UNSHARE_RANGE
> will copy the whole range. So, should it be another type?

I don't see why that would be necessary. The calling contexts are
completely different, but both know exactly what they have to copy
from the iomap's inline data. They fact they copy different parts of
data from the iomap doesn't mean we need different types of iomaps
for them...

> Also, this would require iomap_begin to translate block->dax address
> which I suppose can be a function in the dax code.

bdev_dax_pgoff() + dax_direct_access() should already provide what
we need here.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com