[Midnightbsd-cvs] src [9415] trunk/sys/kern/uipc_syscalls.c: Several fixes and improvements to sendfile()
laffer1 at midnightbsd.org
laffer1 at midnightbsd.org
Sat Mar 4 17:47:23 EST 2017
Revision: 9415
http://svnweb.midnightbsd.org/src/?rev=9415
Author: laffer1
Date: 2017-03-04 17:47:23 -0500 (Sat, 04 Mar 2017)
Log Message:
-----------
Several fixes and improvements to sendfile()
1. If we wanted to send exactly as many bytes as the socket buffer is
sized for, the inner loop of kern_sendfile() would see that the
socket is full before seeing that it had no more bytes left to send.
This would cause it to return EAGAIN to the caller instead of
success. Fix by changing the order that these conditions are tested.
2. Simplify the calculation for the bytes to send in each iteration of
the inner loop of kern_sendfile()
3. Fix some calls with bogus arguments to sf_buf_ext(). These would
only trigger on mbuf allocation failure, but would be hilariously
bad if they did trigger.
Eliminate the layering violation in the kern_sendfile(). When quering
the file size, use VOP_GETATTR() instead of accessing vnode vm_object
un_pager.vnp.vnp_size.
Take the shared vnode lock earlier to cover the added VOP_GETATTR()
call and, as consequence, the whole internal sendfile loop. Reduce vm
object lock scope to not protect the local calculations.
Item 1 in r248830 causes earlier exits from the sendfile(2), before
all requested data was sent. The reason is that xfsize <= 0 condition
must not be tested at all if space == loopbytes. Otherwise, the done
is set to 1, and sendfile(2) is aborted too early.
Instead of moving the condition to exiting the inner loop after the
xfersize check, directly check for the completed transfer before the
testing of the available space in the socket buffer, and revert item 1
of r248830. It is arguably another bug to sleep waiting for socket
buffer space (or return EAGAIN for non-blocking socket) if all bytes
are already transferred.
Obtained from: FreeBSD
Revision Links:
--------------
http://svnweb.midnightbsd.org/src/?rev=248830
http://svnweb.midnightbsd.org/src/?rev=248830
Modified Paths:
--------------
trunk/sys/kern/uipc_syscalls.c
Modified: trunk/sys/kern/uipc_syscalls.c
===================================================================
--- trunk/sys/kern/uipc_syscalls.c 2017-03-04 21:49:42 UTC (rev 9414)
+++ trunk/sys/kern/uipc_syscalls.c 2017-03-04 22:47:23 UTC (rev 9415)
@@ -1832,9 +1832,11 @@
struct mbuf *m = NULL;
struct sf_buf *sf;
struct vm_page *pg;
+ struct vattr va;
off_t off, xfsize, fsbytes = 0, sbytes = 0, rem = 0;
int error, hdrlen = 0, mnw = 0;
int vfslocked;
+ int bsize;
struct sendfile_sync *sfs = NULL;
/*
@@ -1849,6 +1851,18 @@
vfslocked = VFS_LOCK_GIANT(vp->v_mount);
vn_lock(vp, LK_SHARED | LK_RETRY);
if (vp->v_type == VREG) {
+ bsize = vp->v_mount->mnt_stat.f_iosize;
+ if (uap->nbytes == 0) {
+ error = VOP_GETATTR(vp, &va, td->td_ucred);
+ if (error != 0) {
+ VOP_UNLOCK(vp, 0);
+ VFS_UNLOCK_GIANT(vfslocked);
+ obj = NULL;
+ goto out;
+ }
+ rem = va.va_size;
+ } else
+ rem = uap->nbytes;
obj = vp->v_object;
if (obj != NULL) {
/*
@@ -1866,7 +1880,8 @@
obj = NULL;
}
}
- }
+ } else
+ bsize = 0; /* silence gcc */
VOP_UNLOCK(vp, 0);
VFS_UNLOCK_GIANT(vfslocked);
if (obj == NULL) {
@@ -1959,12 +1974,21 @@
* The outer loop checks the state and available space of the socket
* and takes care of the overall progress.
*/
- for (off = uap->offset, rem = uap->nbytes; ; ) {
- struct mbuf *mtail = NULL;
- int loopbytes = 0;
- int space = 0;
- int done = 0;
+ for (off = uap->offset; ; ) {
+ struct mbuf *mtail;
+ int loopbytes;
+ int space;
+ int done;
+ if ((uap->nbytes != 0 && uap->nbytes == fsbytes) ||
+ (uap->nbytes == 0 && va.va_size == fsbytes))
+ break;
+
+ mtail = NULL;
+ loopbytes = 0;
+ space = 0;
+ done = 0;
+
/*
* Check the socket state for ongoing connection,
* no errors and space in socket buffer.
@@ -2031,6 +2055,20 @@
*/
space -= hdrlen;
+ vfslocked = VFS_LOCK_GIANT(vp->v_mount);
+ error = vn_lock(vp, LK_SHARED);
+ if (error != 0) {
+ VFS_UNLOCK_GIANT(vfslocked);
+ goto done;
+ }
+ error = VOP_GETATTR(vp, &va, td->td_ucred);
+ if (error != 0 || off >= va.va_size) {
+ VOP_UNLOCK(vp, 0);
+ VFS_UNLOCK_GIANT(vfslocked);
+ goto done;
+ }
+ VFS_UNLOCK_GIANT(vfslocked);
+
/*
* Loop and construct maximum sized mbuf chain to be bulk
* dumped into socket buffer.
@@ -2040,7 +2078,6 @@
vm_offset_t pgoff;
struct mbuf *m0;
- VM_OBJECT_LOCK(obj);
/*
* Calculate the amount to transfer.
* Not to exceed a page, the EOF,
@@ -2047,18 +2084,14 @@
* or the passed in nbytes.
*/
pgoff = (vm_offset_t)(off & PAGE_MASK);
- xfsize = omin(PAGE_SIZE - pgoff,
- obj->un_pager.vnp.vnp_size - uap->offset -
- fsbytes - loopbytes);
if (uap->nbytes != 0)
rem = (uap->nbytes - fsbytes - loopbytes);
else
- rem = obj->un_pager.vnp.vnp_size -
+ rem = va.va_size -
uap->offset - fsbytes - loopbytes;
- xfsize = omin(rem, xfsize);
+ xfsize = omin(PAGE_SIZE - pgoff, rem);
xfsize = omin(space - loopbytes, xfsize);
if (xfsize <= 0) {
- VM_OBJECT_UNLOCK(obj);
done = 1; /* all data sent */
break;
}
@@ -2068,6 +2101,7 @@
* if not found or wait and loop if busy.
*/
pindex = OFF_TO_IDX(off);
+ VM_OBJECT_LOCK(obj);
pg = vm_page_grab(obj, pindex, VM_ALLOC_NOBUSY |
VM_ALLOC_NORMAL | VM_ALLOC_WIRED | VM_ALLOC_RETRY);
@@ -2085,7 +2119,6 @@
else if (uap->flags & SF_NODISKIO)
error = EBUSY;
else {
- int bsize;
ssize_t resid;
/*
@@ -2097,25 +2130,16 @@
/*
* Get the page from backing store.
- */
- vfslocked = VFS_LOCK_GIANT(vp->v_mount);
- error = vn_lock(vp, LK_SHARED);
- if (error != 0)
- goto after_read;
- bsize = vp->v_mount->mnt_stat.f_iosize;
-
- /*
* XXXMAC: Because we don't have fp->f_cred
* here, we pass in NOCRED. This is probably
* wrong, but is consistent with our original
* implementation.
*/
+ vfslocked = VFS_LOCK_GIANT(vp->v_mount);
error = vn_rdwr(UIO_READ, vp, NULL, MAXBSIZE,
trunc_page(off), UIO_NOCOPY, IO_NODELOCKED |
IO_VMIO | ((MAXBSIZE / bsize) << IO_SEQSHIFT),
td->td_ucred, NOCRED, &resid, td);
- VOP_UNLOCK(vp, 0);
- after_read:
VFS_UNLOCK_GIANT(vfslocked);
VM_OBJECT_LOCK(obj);
vm_page_io_finish(pg);
@@ -2171,7 +2195,7 @@
m0 = m_get((mnw ? M_NOWAIT : M_WAITOK), MT_DATA);
if (m0 == NULL) {
error = (mnw ? EAGAIN : ENOBUFS);
- sf_buf_mext((void *)sf_buf_kva(sf), sf);
+ sf_buf_mext(NULL, sf);
break;
}
MEXTADD(m0, sf_buf_kva(sf), PAGE_SIZE, sf_buf_mext,
@@ -2199,6 +2223,8 @@
}
}
+ VOP_UNLOCK(vp, 0);
+
/* Add the buffer chain to the socket buffer. */
if (m != NULL) {
int mlen, err;
More information about the Midnightbsd-cvs
mailing list