Bug 12033 - smbd should support copy_file_range() for FSCTL_SRV_COPYCHUNK
Summary: smbd should support copy_file_range() for FSCTL_SRV_COPYCHUNK
Status: NEW
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: unspecified
Hardware: All All
: P5 enhancement (vote)
Target Milestone: ---
Assignee: Ralph Böhme
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-07-25 09:29 UTC by Björn Jacke
Modified: 2020-01-14 09:47 UTC (History)
5 users (show)

See Also:


Attachments
patch for master (5.61 KB, patch)
2017-02-09 08:13 UTC, Björn Jacke
no flags Details
waf check only part (1.13 KB, patch)
2020-01-14 09:46 UTC, Björn Jacke
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Björn Jacke 2016-07-25 09:29:02 UTC
copy_file_range is supported since kernel 4.5. We should check if this call is supported and use it for server side copychung operation.

Interesting also for Kernel 4.7, where the NFS client code also supports that call and can speed up Samba servers with NFS file stores.

Side note: if copy_file_range support is found, btrfs should not override the copy_chunk functions but use the generic code.
Comment 1 Björn Jacke 2016-09-07 09:35:54 UTC
with linux kernel 4.8 even xfs will profit immensely from the use of copy_file_range with the introduction of reflink copies.
Comment 2 Björn Jacke 2017-02-09 08:13:29 UTC
Created attachment 12912 [details]
patch for master
Comment 3 Ralph Böhme 2017-06-02 14:17:48 UTC
As just discussed on the phone:

* in vfs_default.c:vfswrap_copy_chunk_send() we should use copy_file_range() if flags does not contain VFS_COPY_CHUNK_FL_MUST_CLONE

* if flags contains VFS_COPY_CHUNK_FL_MUST_CLONE we must use the IOC_CLONE_RANGE ioctl()

* we then can remove the copy_chunk handler from vfs_btrfs
Comment 4 Björn Jacke 2020-01-14 09:46:53 UTC
Created attachment 15729 [details]
waf check only part
Comment 5 Björn Jacke 2020-01-14 09:47:20 UTC
Ralph: some weeks after my patch you made all that copy chunk async, which requires this patch to be reworked for this. Can you have a look at his, I actually don't completely get throug the new code to adopt the patch myself.

I also think that for local copies the copy chunk operation should be not split into 16MiB small pieces if this is possible because even a local copy of *any* file size is an atomic operation with most filesystems these days with copy_file_range. Maybe COPYCHUNK_MAX_TOTAL_LEN can be at least multiplied by 10 or so to make copies of very large files not too fragmented (and with unneeded overhead).