On 5/23/19 7:51 PM, Goldwyn Rodrigues wrote:
Hi,
I'm working on reflink & dax in XFS, here are some thoughts on this:
As mentioned above: the second iomap's offset and length must match the
first. I thought so at the beginning, but later found that the only
difference between these two iomaps is @addr. So, what about adding a
@saddr, which means the source address of COW extent, into the struct iomap.
The ->iomap_begin() fills @saddr if the extent is COW, and 0 if not. Then
handle this @saddr in each ->actor(). No more modifications in other
functions.
Yes, I started of with the exact idea before being recommended this by Dave.
I used two fields instead of one namely cow_pos and cow_addr which defined
the source details. I had put it as a iomap flag as opposed to a type
which of course did not appeal well.
We may want to use iomaps for cases where two inodes are involved.
An example of the other scenario where offset may be different is file
comparison for dedup: vfs_dedup_file_range_compare(). However, it would
need two inodes in iomap as well.
Yes, it is reasonable. Thanks for your explanation.
One more thing RFC:
I'd like to add an end-io callback argument in ->dax_iomap_actor() to
update the metadata after one whole COW operation is completed. The
end-io can also be called in ->iomap_end(). But one COW operation may
call ->iomap_apply() many times, and so does the end-io. Thus, I think
it would be nice to move it to the bottom of ->dax_iomap_actor(), called
just once in each COW operation.
--
Thanks,
Shiyang Ruan.
My RFC patchset[1] is implemented in this way and works for me, though it is
far away from perfectness.
[1]: https://patchwork.kernel.org/cover/10904307/
