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.

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

Add more constraints for block cloning #15251

Merged
merged 1 commit into from
Sep 9, 2023
Merged

Conversation

amotin
Copy link
Member

@amotin amotin commented Sep 8, 2023

  • We cannot clone into files with smaller block size if there is more than one block, since we can not grow the block size.
  • Block size must be power-of-2 if destination offset != 0, since there can be no multiple blocks of non-power-of-2 size.

The first should handle the case when destination file has several blocks but still is not bigger than one block of the source file. The second fixes panic in dmu_buf_hold_array_by_dnode() on attempt to concatenate files with equal but non-power-of-2 block sizes.

While there, assert that error is reported if we made no progress.

How Has This Been Tested?

Reproduced kernel panic on FreeBSD with the script from https://lists.freebsd.org/archives/freebsd-current/2023-September/004610.html and confirmed that it works after.

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:

 - We cannot clone into files with smaller block size if there is
more than one block, since we can not grow the block size.
 - Block size must be power-of-2 if destination offset != 0, since
there can be no multiple blocks of non-power-of-2 size.

The first should handle the case when destination file has several
blocks but still is not bigger than one block of the source file.
The second fixes panic in dmu_buf_hold_array_by_dnode() on attempt
to concatenate files with equal but non-power-of-2 block sizes.

While there, assert that error is reported if we made no progress.

Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.
@amotin amotin added the Status: Code Review Needed Ready for review and testing label Sep 8, 2023
@behlendorf
Copy link
Contributor

cc: @robn @oromenahar.

@amotin
Copy link
Member Author

amotin commented Sep 8, 2023

Another theoretical guess while thinking about this area: if we use block cloning to concatenate one 512 byte file to another 512 byte file we'll get a file with two 512 byte blocks, but I am not sure we really want that. We may wish to restrict minimal copy size to allow block sizes to grow towards recordsize in normal way instead.

@pjd
Copy link
Contributor

pjd commented Sep 9, 2023

Looks good to me. Thank you Alex for looking into this!

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Sep 9, 2023
@behlendorf behlendorf merged commit 5cc1876 into openzfs:master Sep 9, 2023
@amotin amotin deleted the bclone branch September 10, 2023 02:02
@amotin amotin mentioned this pull request Sep 27, 2023
13 tasks
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Dec 12, 2023
- We cannot clone into files with smaller block size if there is
more than one block, since we can not grow the block size.
 - Block size must be power-of-2 if destination offset != 0, since
there can be no multiple blocks of non-power-of-2 size.

The first should handle the case when destination file has several
blocks but still is not bigger than one block of the source file.
The second fixes panic in dmu_buf_hold_array_by_dnode() on attempt
to concatenate files with equal but non-power-of-2 block sizes.

While there, assert that error is reported if we made no progress.

Reviewed-by: Pawel Jakub Dawidek <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Rob Norris <[email protected]>
Reviewed-by: Kay Pedersen <[email protected]>
Reviewed-by: Umer Saleem <[email protected]>
Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.
Closes openzfs#15251
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants