Re: [PATCH v1 2/3] NFSD: Implement the COPY call

[Anna Schumaker <Anna.Schumaker@netapp.com>]

On 12/04/2015 11:49 AM, J. Bruce Fields wrote:
> On Fri, Dec 04, 2015 at 10:49:24AM -0500, Anna Schumaker wrote:
>> On 12/04/2015 10:45 AM, J. Bruce Fields wrote:
>>> On Thu, Dec 03, 2015 at 03:55:35PM -0500, Anna Schumaker wrote:
>>>> @@ -498,6 +499,22 @@ __be32 nfsd4_set_nfs4_label(struct svc_rqst *rqstp, struct svc_fh *fhp,
>>>>  }
>>>>  #endif
>>>>  
>>>> +ssize_t nfsd_copy_range(struct file *src, u64 src_pos,
>>>> +		       struct file *dst, u64 dst_pos,
>>>> +		       u64 count)
>>>> +{
>>>> +	ssize_t bytes;
>>>> +	u64 limit = 0x10000000;
>>>
>>> Why that value?  Could I get a comment here?
>>
>> Whoops!  I had a comment there at one point, but I must have deleted it :(.  That value is to cap copies to 256MB.
> 
> Could you include some justification for the choice of that particular
> value?

Yeah, I can run tests with different values and include the results in v2.

> 
>>>> +	if (count > limit)
>>>> +		count = limit;
>>>> +
>>>> +	bytes = vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0);
>>>
>>> Sorry, I lost track of the copy discussions: does this only work on
>>> filesystems with special support, or does it fall back on doing the copy
>>> by hand?  Which filesystems (of the exportable filesystems) support
>>> this?
>>
>> The system call falls back on doing the copy by hand if there is no filesystem acceleration.
> 
> Is this practical?  It means a huge range in possible latency of the
> single COPY call depending on filesystem.

That's why I'm breaking copies into smaller chunks, rather than doing everything at once.

> 
> I guess I can live with it and we can see if people run into problems in
> practice.  But let's make sure this is documented.

Okay.  I'll add documentation about this!

Anna

> 
> --b.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>