- User Since
- Jan 13 2015, 10:58 PM (247 w, 5 d)
Thanks for the comment.
The new revision makes changes in the vm part that I don't understand, so I am afraid I can't
Sun, Oct 6
Added a reply to the inline comment.
Sat, Oct 5
Other than the one inline comment, it looks good to me. However, I don't know enough about the locking/vm stuff
to say if nfs_lock() is correct?
Delaying doing the vnode_pager_setsize() until a lock operation on the vnode sounds reasonable to me.
Fri, Oct 4
Thu, Oct 3
Mon, Sep 30
This version of the patch replaces nfscl_checksetsize() with vnode_pager_setsize_nonblock().
The latter is a macro which just calls vnode_pager_setsize_mightsleep() with can_sleep == false.
Sat, Sep 28
As an alternative to creating a function called nfscl_checksetsize(), what about adding a "can_sleep"
flag argument to vnode_pager_setsize().
If can_sleep is "false", then it would return EWOULDBLOCK for the "shrinking" case.
(If can_sleep is "true" it would retain current behaviour.) Yes, the function would no
longer be void, but would return (0) except for this case.
Fri, Sep 27
Thu, Sep 26
Unfortunately testing found a deadlock problem.
- One of the iod threads is sleeping on "vmopar", so it is in vnode_pager_setsize() when the size is being reduced.
- The rest of the iod threads are waiting for the NFS node lock.
Wed, Sep 25
This version of the patch changes the n_mtx lock to an sx lock (n_sx) and
the ncl_iod_mutex to ncl_iod_sx. The former is done so that it can safely be
locked while calling vnode_pager_setsize().
The latter is changed to an sx lock since it is held when n_sx is acquired.
All the msleep()s are changed to sx_sleep() calls.
Tue, Sep 24
Mon, Sep 23
So, do you think that just replacing n_mtx with n_solock makes more sense?
(I'll admit I was thinking it was a lot of editing, but so what.)
Sun, Sep 15
Sep 13 2019
Sep 9 2019
Sep 8 2019
Sep 7 2019
Sep 6 2019
Sep 5 2019
Sep 4 2019
Sep 2 2019
Sep 1 2019
Aug 31 2019
Aug 27 2019
Aug 22 2019
Aug 21 2019
Updated the patch to only map ENOTTY to EINVAL, per Mark's comments.
Actually, I only mapped ENOTTY when I first did the patch. The only reason I changed it
is that any error returned by VOP_GETATTR() also gets returned to lseek(2). For NFS,
this could be EACCES, for example, since file permissions are checked for every operation
on NFS, nit just at open(2) time.
Aug 20 2019
Aug 19 2019
Aug 18 2019
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().
Aug 17 2019
Aug 15 2019
Aug 12 2019
Aug 8 2019
Only change is a modified comment explaining how FIOSEEKHOLE might
find the same offset as FIOSEEKDATA already has returned for the file.
Aug 5 2019
Aug 4 2019
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.
Aug 2 2019
Jul 31 2019
Jul 29 2019
Jul 28 2019
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"
Jul 27 2019
Jul 26 2019
Jul 25 2019
Jul 22 2019
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.