3:01 a.m.
On 5/28/19 5:17 PM, Jan Kara wrote:
On Mon 27-05-19 16:25:41, Shiyang Ruan wrote:
> 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.
I'm sorry but I don't follow what you suggest. One COW operation is a call
to dax_iomap_rw(), isn't it? That may call iomap_apply() several times,
each invocation calls ->iomap_begin(), ->actor() (dax_iomap_actor()),
->iomap_end() once. So I don't see a difference between doing something in
->actor() and ->iomap_end() (besides the passed arguments but that does not
seem to be your concern). So what do you exactly want to do?
Hi Jan,
Thanks for pointing out, and I'm sorry for my mistake. It's
->dax_iomap_rw(), not ->dax_iomap_actor().
I want to call the callback function at the end of ->dax_iomap_rw().
Like this:
dax_iomap_rw(..., callback) {
...
while (...) {
iomap_apply(...);
}
if (callback != null) {
callback();
}
return ...;
}
Honza
--
Thanks,
Shiyang Ruan.