On Wed, Jun 14, 2017 at 3:32 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> On Wed, Jun 14, 2017 at 9:53 PM, Olga Kornievskaia <kolga@xxxxxxxxxx> wrote:
>>
>> This is a proposal to allow 64bit count to copy and return as
>> a result of a copy_file_range. No attempt was made to share code
>> with the 32bit function because 32bit interface should probably
>> get depreciated.
>>
>> Why use 64bit? Current uses of 32bit are by clone_file_range()
>> which could use 64bit count and NFS copy_file_range also supports
>> 64bit count value.
>>
>> Also with this proposal off-the-bet allow the userland to copy
>> between different mount points.
>>
>> Signed-off-by: Olga Kornievskaia <kolga@xxxxxxxxxx>
>> ---
> ...
>> +
>> + /*
>> + * Try cloning first, this is supported by more file systems, and
>> + * more efficient if both clone and copy are supported (e.g. NFS).
>> + */
>> + if (file_in->f_op->clone_file_range) {
>> + ret = file_in->f_op->clone_file_range(file_in, pos_in,
>> + file_out, pos_out, len);
>> + if (ret == 0) {
>> + ret = len;
>> + goto done;
>> + }
>> + }
>> +
>> + if (file_out->f_op->copy_file_range64) {
>> + ret = file_out->f_op->copy_file_range64(file_in, pos_in,
>> + file_out, pos_out, len, flags);
>> + if (ret != -EOPNOTSUPP)
>> + goto done;
>> + }
>> +
>> + ret = do_splice_direct(file_in, &pos_in, file_out, &pos_out,
>> + len > MAX_RW_COUNT ? MAX_RW_COUNT : len, 0);
>> +
>
> Olga,
Hi Amir,
Thanks for the comments.
> I know this is copied from vfs_copy_file_range() as is, so my comment
> applies to the origin function as well, but it is easier to change the new
> function without changing userspace behavior, so..
>
> A while back, either Dave Chinner or Christoph suggested that I use
> vfs_copy_file_range() from ovl_copy_up_data() instead of calling
> vfs_clone_file_range() and falling back to do_splice_direct() loop.
> Then Christoph added the clone_file_range attempt into
> vfs_copy_file_range(), which was one part of the work.
>
> However, ovl_copy_up_data() uses a killable loop of do_splice_direct()
> calls with smaller chunks, so it is not the same as vfs_copy_file_range().
>
> I wonder if it makes sense to use a similar killable loop in the new
> function?
I'm currently not proposing to change the semantics of
copy_file_range() call to in some cases return a partial copy and thus
making the application responsible for looping to finish the copy.
Basically copy_file_range() follows the same semantics as read/write
where it is possible that not all requested bytes are completed.
However, if it seems more desirable to shift the responsibility to
completing the copy into the kernel, then I can definitely add the
same looping as does ovl_copy_up_data().
> I wonder, for reference, if NFS implementation of copy_file_range64()
> is killable?
I believe the current implementation of "intra" (same server) copy is
not killable. The outstanding proposal to add asynchronous copy as
well as "inter" (server to server) copy is killable.
