Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

linux/copy_file_range: properly request a fallback copy on Linux <5.3 #15131

Merged
merged 2 commits into from
Aug 1, 2023
Merged

linux/copy_file_range: properly request a fallback copy on Linux <5.3 #15131

merged 2 commits into from
Aug 1, 2023

Conversation

robn
Copy link
Member

@robn robn commented Aug 1, 2023

Motivation and Context

Before Linux 5.3, the filesystem's copy_file_range handler had to signal back to the kernel that we can't fulfull the request and it should fallback to a content copy. This is done by returning -EOPNOTSUPP.

Description

This commit converts the EXDEV return from zfs_clone_range to EOPNOTSUPP, to force the kernel to fallback for all the valid reasons it might be unable to clone. Without it the copy_file_range() syscall will return EXDEV to userspace, breaking its semantics.

How Has This Been Tested?

This PR adds a new test, block_cloning_copyfilerange_fallback. On Linux 5.3+, it passes with and without this change. On <5.3, it fails without the change, and passes with.

At least, it does on this specific kernel:

debian@debian10:~/zfs$ uname -a
Linux debian10 4.19.0-24-amd64 #1 SMP Debian 4.19.282-1 (2023-04-29) x86_64 GNU/Linux

debian@debian10:~/zfs$ /usr/local/share/zfs/zfs-tests.sh -x -v -T block_cloning
...
Test (Linux): /usr/local/share/zfs/zfs-tests/tests/functional/block_cloning/block_cloning_copyfilerange_fallback (run as root)[00:01] [FAIL]
...
Results Summary
PASS	 10
FAIL	  1
SKIP	  1

Running Time:	00:00:09
Percent passed:	83.3%
Log directory:	/var/tmp/test_results/20230801T071416

Tests with results other than PASS that are expected:
    SKIP block_cloning/block_cloning_copyfilerange_cross_dataset (copy_file_range(2) cross-filesystem needs kernel 5.3+)

Tests with result of PASS that are unexpected:

Tests with results other than PASS that are unexpected:
    FAIL block_cloning/block_cloning_copyfilerange_fallback (expected PASS)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

@robn
tests: add test for copy_file_range fallbacks
89e861b 
copy_file_range should always fallback to a content copy whenever ZFS
can't service the request with cloning.

Signed-off-by: Rob Norris <[email protected]>
@robn
linux/copy_file_range: properly request a fallback copy on Linux <5.3
88de7b3 
Before Linux 5.3, the filesystem's copy_file_range handler had to signal
back to the kernel that we can't fulfull the request and it should
fallback to a content copy. This is done by returning -EOPNOTSUPP.

This commit converts the EXDEV return from zfs_clone_range to
EOPNOTSUPP, to force the kernel to fallback for all the valid reasons it
might be unable to clone. Without it the copy_file_range() syscall will
return EXDEV to userspace, breaking its semantics.

Signed-off-by: Rob Norris <[email protected]>
behlendorf
behlendorf approved these changes Aug 1, 2023
View reviewed changes
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for running this case down.

@behlendorf behlendorf added the Status: Accepted Ready to integrate (reviewed, tested) label Aug 1, 2023
@behlendorf behlendorf merged commit ead3eea into openzfs:master Aug 1, 2023
19 checks passed
behlendorf pushed a commit to behlendorf/zfs that referenced this pull request Aug 1, 2023
@robn @behlendorf
linux/copy_file_range: properly request a fallback copy on Linux <5.3
789f95f 
Before Linux 5.3, the filesystem's copy_file_range handler had to signal
back to the kernel that we can't fulfill the request and it should
fallback to a content copy. This is done by returning -EOPNOTSUPP.

This commit converts the EXDEV return from zfs_clone_range to
EOPNOTSUPP, to force the kernel to fallback for all the valid reasons it
might be unable to clone. Without it the copy_file_range() syscall will
return EXDEV to userspace, breaking its semantics.

Add test for copy_file_range fallbacks.  copy_file_range should always
fallback to a content copy whenever ZFS can't service the request with
cloning.

Reviewed-by: Brian Atkinson <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Kay Pedersen <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes openzfs#15131
behlendorf pushed a commit that referenced this pull request Aug 2, 2023
@robn @behlendorf
linux/copy_file_range: properly request a fallback copy on Linux <5.3
c47f0f4 
Before Linux 5.3, the filesystem's copy_file_range handler had to signal
back to the kernel that we can't fulfill the request and it should
fallback to a content copy. This is done by returning -EOPNOTSUPP.

This commit converts the EXDEV return from zfs_clone_range to
EOPNOTSUPP, to force the kernel to fallback for all the valid reasons it
might be unable to clone. Without it the copy_file_range() syscall will
return EXDEV to userspace, breaking its semantics.

Add test for copy_file_range fallbacks.  copy_file_range should always
fallback to a content copy whenever ZFS can't service the request with
cloning.

Reviewed-by: Brian Atkinson <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Kay Pedersen <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes #15131
usaleem-ix pushed a commit to truenas/zfs that referenced this pull request Aug 17, 2023
@robn @usaleem-ix
linux/copy_file_range: properly request a fallback copy on Linux <5.3
dea08d8 
Before Linux 5.3, the filesystem's copy_file_range handler had to signal
back to the kernel that we can't fulfill the request and it should
fallback to a content copy. This is done by returning -EOPNOTSUPP.

This commit converts the EXDEV return from zfs_clone_range to
EOPNOTSUPP, to force the kernel to fallback for all the valid reasons it
might be unable to clone. Without it the copy_file_range() syscall will
return EXDEV to userspace, breaking its semantics.

Add test for copy_file_range fallbacks.  copy_file_range should always
fallback to a content copy whenever ZFS can't service the request with
cloning.

Reviewed-by: Brian Atkinson <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Kay Pedersen <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes openzfs#15131
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Dec 12, 2023
@robn @lundman
linux/copy_file_range: properly request a fallback copy on Linux <5.3
411d9ca 
Before Linux 5.3, the filesystem's copy_file_range handler had to signal
back to the kernel that we can't fulfill the request and it should
fallback to a content copy. This is done by returning -EOPNOTSUPP.

This commit converts the EXDEV return from zfs_clone_range to
EOPNOTSUPP, to force the kernel to fallback for all the valid reasons it
might be unable to clone. Without it the copy_file_range() syscall will
return EXDEV to userspace, breaking its semantics.

Add test for copy_file_range fallbacks.  copy_file_range should always
fallback to a content copy whenever ZFS can't service the request with
cloning.

Reviewed-by: Brian Atkinson <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Kay Pedersen <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes openzfs#15131
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@behlendorf behlendorf behlendorf approved these changes

@bwatkinson bwatkinson bwatkinson approved these changes

@oromenahar oromenahar oromenahar approved these changes

Assignees
No one assigned
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@robn @behlendorf @bwatkinson @oromenahar