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.
with linux kernel 4.8 even xfs will profit immensely from the use of copy_file_range with the introduction of reflink copies.
Created attachment 12912 [details] patch for master
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
Created attachment 15729 [details] waf check only part
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).
*** Bug 12785 has been marked as a duplicate of this bug. ***
(In reply to Björn Jacke from comment #5) ... > 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. Chunking large requests makes sense to me. copy_file_range() is not atomic. It *may* result in a reflink / clone on XFS and Btrfs, may be handled as splice / sendfile IO or as an offloaded copy for network filesystems.
(In reply to David Disseldorp from comment #7) David, I have a WIP patchset that adds copy_file_range() support here: <https://gitlab.com/samba-team/samba/-/merge_requests/2044>. The final patch removes BTRFS_IOC_CLONE_RANGE support from vfs_btrfs and I wanted to check back with you if you think we're ready for that jump? In theory an older kernel could lack both the copy_file_range() function and the raw syscall, BUT could have btrfs version that supports BTRFS_IOC_CLONE_RANGE. I don't know how common such a combination can be. Thoughts?
This bug was referenced in samba master: 4dcc04228df233be15efe9c31bcbaba822b140c4 e2d524d4baee193b0d03df3e7edda48b3cb4bddf 4f1a02909b8694dcc30fd5c7c6772fcfa1092ed9 e72be5213335ab1ea0f9f396ab071669231c151b accaa2f1f67a7f064a4ce03a120d7b2f8e847ccf
Created attachment 16669 [details] Patch for 4.14 cherry-picked from master
Created attachment 16670 [details] Patch for 4.13 backported from master
Created attachment 16671 [details] Patch for 4.13 backported from master
Comment on attachment 16669 [details] Patch for 4.14 cherry-picked from master These need the follow-up patch changing pathref -> io. Sorry for missing that in the original review.
Comment on attachment 16670 [details] Patch for 4.13 backported from master LGTM. No pathref functions here :-).
(In reply to Jeremy Allison from comment #13) Ah, drat, that is *exactly* what I wanted to do, but I got confused...
This bug was referenced in samba master: 0e3ddc27ed6d603a21cb2b187f3295506d560604
Created attachment 16672 [details] Patch for 4.14 cherry-picked from master This one includes the additional fix. Sorry for missing this in the initial backport!
Comment on attachment 16672 [details] Patch for 4.14 cherry-picked from master LGTM. Thanks.
Re-assigning to Karolin for inclusion in 4.14.next.
(In reply to Jeremy Allison from comment #20) Pushed to autobuild-v4-14-test. Just to make sure: This one should not go into 4.13?
That's correct, 4.13.x doesn't have fsp_get_io_fd().
This bug was referenced in samba v4-14-test: d5d6bbaa9395458114a5f2b4f3c398ef889f67ae 0772ff448fc054250e580a6e2e48b69485eca506 a25b75b2ca26f02a05ac318a837475bb6835a081 c44d2e8dbdc4e2828e4fb233d67d05bca7bd0779 f9bcec6298d3ba42963b3c30f280c1f7ff5d20de 0fca66858de2df9b832ac257a2ccd104f90e3b74
Pushed to v4-14-test. Closing out bug report. Thanks!
This bug was referenced in samba v4-14-stable (Release samba-4.14.7): d5d6bbaa9395458114a5f2b4f3c398ef889f67ae 0772ff448fc054250e580a6e2e48b69485eca506 a25b75b2ca26f02a05ac318a837475bb6835a081 c44d2e8dbdc4e2828e4fb233d67d05bca7bd0779 f9bcec6298d3ba42963b3c30f280c1f7ff5d20de 0fca66858de2df9b832ac257a2ccd104f90e3b74