- User Since
- Jan 13 2015, 10:58 PM (239 w, 5 d)
This version of the patch has the changes suggested by kib@ incorporated in it.
The only semantic change is that it now returns whatever error VOP_GETATTR()
returns if VOP_GETATTR() fails. This is consistent with the behaviour of vn_bmap_seekhole().
Sat, Aug 17
Thu, Aug 15
Mon, Aug 12
Thu, Aug 8
Only change is a modified comment explaining how FIOSEEKHOLE might
find the same offset as FIOSEEKDATA already has returned for the file.
Mon, Aug 5
Sun, Aug 4
This version of the patch adds an additional sanity check for
the results from VOP_IOCTL(). This change can only have an effect
for a non-INVARIANTS kernel where one of the VOP_IOCTL()s
return a bogus offset.
It ensures a reasonable positive value for xfer2 is returned for this case.
Replied to inline comment.
This version of the patch has the INVARIANTS printf() replaced by a KASSERT().
It also has KASSERT()s added for the results of FIOSEEKDATA/FIOSEEKHOLE.
Replied to kib@'s comments inline.
Fri, Aug 2
Wed, Jul 31
Mon, Jul 29
Sun, Jul 28
I don't really know anything about this, but it looks fine to me.
The only change is uint32_t replaces size_t, which makes sense to 32bits.
(I'm guessing this is only used for amd64, where "int" and "unsigned int"
Sat, Jul 27
Fri, Jul 26
Thu, Jul 25
Mon, Jul 22
Marked done and replied to inline comments.
This version incorporates a change to VOP_COPY_FILE_RANGE.9 suggested by asomers@.
This updates incorporates changes suggested by kib@.
Replace 1048576 with 1024 * 1024 for readability and move
vn_generic_copy_file_range() and helper functions from
vfs_subr.c to vfs_vnops.c.
Replied and checked done to inline comments.
This version incorporates changes suggested by asomers@.
The previous version of the patch had a single bwillwrite() call before the copy
done by the VOP call or vn_generic_copy_file_range().
kib@ pointed out that bwillwrite() calls are needed in the copy loop, which makes
sense, since the copy loop could be writing a large amount of data and constipate
the buffer cache for file systems using it.
Jul 18 2019
vn_copy_file_range() erroneously returned the error with *lenp set to the requested
length instead of 0 (the number of bytes copied).
This version of the patch has this fixed and a couple of extraneous brackets removed.
Jul 13 2019
One minor additional fix. For the case where the "len" argument is 0, the Linux
syscall returns 0. The previous patch did that, but it did 0 length (ie. start and end equal)
range locks, which the range lock code doesn't handle correctly.
Jul 7 2019
Marked "check for flags == 0" as done on the inline comment.
Jul 6 2019
This version of the patch checks for flags == 0 and returns EINVAL when not 0.
It also documents "must be 0" in the man page. This change seemed to be the
consensus from a discussion on FreeBSD-current@.
Jul 5 2019
Taken comment w.r.t. handling of flags argument over to freebsd-current@, since more people will see it there.
Jul 3 2019
One more bug found while proofreading the code.
- It erroneously used inoffp instead of outoffp when calculating the size of hole to write. My main test program copies byte ranges of a file and then compares the file, so inoff and outoff are the same. I did run a small test program copying from/to different offsets, but the file had data so I could easily check if it worked.
This version of the patch has this fixed. It also has a minor optimization, where it uses vn_truncate_locked() to grow the
output file when a hole is at the end of the range and the hole is entirely beyond the original EOF for the output file
instead of writing 0 bytes to grow the file.
Jul 2 2019
Marked inline comments as done.
Made a minor change to VOP_COPY_FILE_RANGE.9 as recommended by asomers@.
This version of the patch incorporates the changes recommended by kib@.
I have also added a couple of sentences related to copying sparse files to copy_file_range.2.
More fixes. When testing using a file with a large (> 4Gbyte) hole in it, a couple
of bugs needed to be fixed.
- If the "len" argument is > SSIZE_MAX, it can't be returned --> clip "len" at SSIZE_MAX
- If the hole size is greater than what can be stored in size_t… --> change xfer, xfer2 to off_t
- If the hole extended past the end of the range being copied, xfer was set too large and would cause the len variable to wrap around. --> clip xfer at len after being calculated as the hole size.
Jul 1 2019
This variant of the patch has the rangelock code moved from vn_copy_file_range() to
kern_copy_file_range(). The reason I did this is that the NFSv4.2 server needs to lock
the file ranges slightly differently (for the "to EOF" case) and I wanted it to be able
to call vn_copy_file_range() to do most of the work.
Jun 30 2019
This version of the patch uses vn_truncate_locked(), which is created by D20808.
There should be no semantic change from the previous variant of the patch.
Jun 27 2019
Adjust patch to use the vn_rangelock_tryrlock() naming now used by D20645.
This version of the patch incorporates kib@'s suggestions.
bwillwrite() call added.
VOP_ADD_WRITECOUNT() added around VOP_SETATTR() of size.
Jun 22 2019
I've taken the "pig picture" discussion related to Sean's points over to the email@example.com
mailing list. (I tried to add that mailing list when I first put this patch in phabricator, but it would only
seem to accept freebsd-bugs@ for some reason.)
Sean, the email I had for you didn't work, so please go look at the mailing list.
Added inline comment w.r.t. handling of errors returned by VOP_COPY_FILE_RANGE().
The code that did the actual copying, which was after the VOP_COPY_FILE_RANGE() call in
vn_copy_file_range() is now factored out into a separate function called vn_generic_copy_file_range().
This allows the vn_generic_copy_range() call to be done from within a VOP_COPY_FILE_RANGE() and,
in fact, vop_stdcopy_file_range() does just that.
Jun 19 2019
Replied to some inline comments.
This version of the patch incorporates some of asomers@ suggestions.
Jun 18 2019
One more patch update. This one is an optimization. If a hole is found that extends to
the end of the byte range being copied, all subsequent reads will return all 0 bytes.
This patch adds a Boolean variable called readzeros to flag this case and avoid doing
the unnecessary reads.
Oops, found one more bug during proofreading. If the hole being punched by writing 0s was
greater than blksize in length, the memset() would have gone off the end of the dat buffer.
This version has the above fixed. To make the code cleaner, I factored out the writing of a chunk
of outvp into a separate function.
During a proofread, I spotted that vn_copy_file_range() was broken if the input file had a hole
that straddled the end of the byte range being copied.
This version of the patch has that fixed, plus a typo in a comment and little bit or reordering
of the code (that doesn't affect correctness).
Jun 17 2019
Jun 16 2019
Change the mem_iszero() helper function to return bool instead of int, since C99 bool is now fashionable
in the FreeBSD kernel.
Changed an int to a bool, since C99 bools now appear to be fashionable in the FreeBSD kernel.
int arguments changed to C99 bool as requested by kib@.
Jun 15 2019
Added replies to inline comments.
This variant allows a sleep to be done for allocation of a new list entry. It only returns NULL
if it would need to sleep waiting to be able to lock the range.
Replace vn_rangelock_rlock_nonblock() with vn_rangelock_rlock_trylock() to be consistent with D20645.
This variant of the patch has the "_nonblock" suffixes replaced by "_trylock" as suggested by asomers@.
A quick grep of src/sys/sys shows that "nonblock" is used for flags, but "_trylock" is used as a suffix
for non-blocking lock functions.
This variant of the patch rangelocks the byte ranges for both the input and output files.
It uses the vn_rangelock_rlock_nonblock() call in the D20645 patch to do this without a
risk of deadlock.
Jun 13 2019
Added inline replies to the inline comments that still apply to this version.
This version of the patch avoids the problems w.r.t. concurrent locking of two vnodes
by locking/unlocking the vnodes (which also implies rangelocks and vn_start_write())
I am going to take the "does this sycall need to be atomic?" over to FreeBSD-fs@.
(I will copy kib@'s and asomers@ comments into the post.)
Jun 11 2019
Yes, I did understand the comment w.r.t. range_locks. I fixed the rest of the stuff for two vnodes locked
concurrently, but I couldn't see an easy way to do range_locks on both of them without a LOR.
Added a question w.r.t. which way I should handle the locking of the two vnodes.
Jun 10 2019
Jun 3 2019
Jun 2 2019
The COMPARE_ARRAYS() macro has been changed per suggestions by kib@.