Details

Reviewers

asomers
kib

Commits

rS366302: Clip the "len" argument to vn_generic_copy_file_range() at a

Summary

By clipping the len argument of copy_file_range() to an exact
multiple of hole size, holes are more likely to be maintained
during the copy.
A hole can still straddle the boundary at the end of the
copy range, resulting in a block being allocated in the
output file, but this will reduce the likelyhood of this
happening.

Test Plan

Tested using various small copy programs using
copy_file_range(), plus a patched "cp" that does
copy_file_range() with the len argument set to
SSIZE_MAX.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

rmacklem created this revision.Sep 28 2020, 1:24 AM

Herald added a subscriber: imp. · View Herald TranscriptSep 28 2020, 1:24 AM

Harbormaster completed remote builds in B33845: Diff 77582.Sep 28 2020, 1:24 AM

rmacklem requested review of this revision.Sep 28 2020, 1:24 AM

What happens if inoffp is not a multiple of the hole size? Shouldn't you try to ensure that len + inoffp is a multiple of the hole size, instead of len?

sys/kern/vfs_vnops.c.sav3
3089 ↗	(On Diff #77582)	This expression is also defined as `rounddown(len, blksize)`.

Fixed the calculation to take *inoffp into account,
as suggested by asomers@.

Harbormaster completed remote builds in B33865: Diff 77615.Sep 29 2020, 12:59 AM

Use rounddown() as suggested by asomers@.

Harbormaster completed remote builds in B33866: Diff 77616.Sep 29 2020, 1:05 AM

rmacklem marked an inline comment as done.Sep 29 2020, 1:06 AM

asomers added inline comments.Sep 29 2020, 1:10 AM

sys/kern/vfs_vnops.c.sav3
3088 ↗	(On Diff #77616)	According to pathconf(2), `_PC_MIN_HOLE_SIZE` has a special meaning for 1. So that's probably a common value. You can optimize this block by changing the test to `if (blksize > 1)`

Change the blksize test to handle the special case
where _PC_MIN_HOLE_SIZE is returned as 1.
As well as asomers@ suggested change, I also
changed the blksize code just below it.

I was not aware that the special case of 1 could
be returned.

Harbormaster completed remote builds in B33867: Diff 77618.Sep 29 2020, 2:47 AM

rmacklem marked an inline comment as done.Sep 29 2020, 2:48 AM

asomers accepted this revision.Sep 29 2020, 3:05 AM

This revision is now accepted and ready to land.Sep 29 2020, 3:05 AM

Closed by commit rS366302: Clip the "len" argument to vn_generic_copy_file_range() at a (authored by rmacklem). · Explain WhyOct 1 2020, 12:34 AM

This revision was automatically updated to reflect the committed changes.

rmacklem added a commit: rS366302: Clip the "len" argument to vn_generic_copy_file_range() at a.