Re: [PATCH 0/3] Use copy_file_range() for copy

[Goldwyn Rodrigues]

On 06/03/2018 04:59 PM, Pádraig Brady wrote:
> On 31/05/18 13:42, Goldwyn Rodrigues wrote:
>> This is a patch set to use copy_file_range() system call to copy
>> file from source to destination. The advantage is lower system time
>> because it avoids copy to and from userspace.
>>
>> In my tests, I am getting lower system times consumed while performing
>> with the copy_file_range.
> 
> When copy_file_range() was being proposed I asked for flags
> to control how sparse, unallocated, CoW regions were handled.

What do you mean by unallocated regions? Delayed allocations?

The filesystem is supposed to keep the same type of range as far as
possible. Which means holes will be copied as is. I recently proposed
patches to preserve holes if splice() is used.

> I still see no flags defined in the latest man pages at least,
> so I don't think it's generally usable as is.
> It would be good to get confirmation as to how copy_file_range()
> currently deals with the 3 region types above.

Do you really need it? You could have filesystems which could perform
inline dedupe which will eventually not perform a copy. I suppose it is
best for filesystem to decide how it wants to keep and manage data.

As for sparse, it will be preserved if you perform extent mapping. If
that is not good enough, we could (and should) perform SEEK_HOLE/DATA to
check for sparse.

> 
> These cases were discussed previously at:
> https://lists.gnu.org/archive/html/coreutils/2015-09/msg00025.html

Thanks for the link. I did not see any conclusion. However, I don't
think it is necessary either.

> 
> In regard to reflinking, we might actually change to trying that
> by default in cp (mv already does it by default).
> The reason being that depending on file system, or physical
> device, you may not be actually getting an extra resiliency
> from a copy, as an actual copy is not guaranteed.
> Now that default change could have significant consequences,
> so it would be something that would be most appropriate
> in a major version change I think.
> 
> For copy() in coreutils, we might do something like:
> 
>  if ((remote(dst) && remote(src))
>      || sparse_mode==SPARSE_NEVER
>      || (sparse_mode!=SPARSE_ALWAYS && !is_sparse(src)))
>    ok = copy_file_range()
>  else
>    ok = reflink()
>  if (!ok)
>    existing_copy()
> 

Yes, I think that makes sense. I realized that the patches I sent did
not use the allocated buffers if copy_file_range is used.

-- 
Goldwyn