From owner-svn-src-head@FreeBSD.ORG Sun Apr 28 19:12:10 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 492E8D8A; Sun, 28 Apr 2013 19:12:10 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) by mx1.freebsd.org (Postfix) with ESMTP id 3AF1311C2; Sun, 28 Apr 2013 19:12:10 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.6/8.14.6) with ESMTP id r3SJC94I030637; Sun, 28 Apr 2013 19:12:09 GMT (envelope-from kib@svn.freebsd.org) Received: (from kib@localhost) by svn.freebsd.org (8.14.6/8.14.5/Submit) id r3SJC9bL030636; Sun, 28 Apr 2013 19:12:09 GMT (envelope-from kib@svn.freebsd.org) Message-Id: <201304281912.r3SJC9bL030636@svn.freebsd.org> From: Konstantin Belousov Date: Sun, 28 Apr 2013 19:12:09 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r250027 - head/sys/kern X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 28 Apr 2013 19:12:10 -0000 Author: kib Date: Sun Apr 28 19:12:09 2013 New Revision: 250027 URL: http://svnweb.freebsd.org/changeset/base/250027 Log: 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. Note that this is the last misuse of the vnp_size in the tree, the others were removed from the ELF image activator by r230246. Reviewed by: alc Tested by: pho, bf (previous version) MFC after: 1 week Modified: head/sys/kern/uipc_syscalls.c Modified: head/sys/kern/uipc_syscalls.c ============================================================================== --- head/sys/kern/uipc_syscalls.c Sun Apr 28 18:40:55 2013 (r250026) +++ head/sys/kern/uipc_syscalls.c Sun Apr 28 19:12:09 2013 (r250027) @@ -1902,8 +1902,10 @@ kern_sendfile(struct thread *td, struct 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 bsize; struct sendfile_sync *sfs = NULL; /* @@ -2102,6 +2104,16 @@ retry_space: */ space -= hdrlen; + error = vn_lock(vp, LK_SHARED); + if (error != 0) + goto done; + error = VOP_GETATTR(vp, &va, td->td_ucred); + if (error != 0) { + VOP_UNLOCK(vp, 0); + goto done; + } + bsize = vp->v_mount->mnt_stat.f_iosize; + /* * Loop and construct maximum sized mbuf chain to be bulk * dumped into socket buffer. @@ -2111,7 +2123,6 @@ retry_space: vm_offset_t pgoff; struct mbuf *m0; - VM_OBJECT_WLOCK(obj); /* * Calculate the amount to transfer. * Not to exceed a page, the EOF, @@ -2121,12 +2132,11 @@ retry_space: if (uap->nbytes) rem = (uap->nbytes - fsbytes - loopbytes); else - rem = obj->un_pager.vnp.vnp_size - + rem = va.va_size - uap->offset - fsbytes - loopbytes; xfsize = omin(PAGE_SIZE - pgoff, rem); xfsize = omin(space - loopbytes, xfsize); if (xfsize <= 0) { - VM_OBJECT_WUNLOCK(obj); done = 1; /* all data sent */ break; } @@ -2136,7 +2146,6 @@ retry_space: * Let the outer loop figure out how to handle it. */ if (space <= loopbytes) { - VM_OBJECT_WUNLOCK(obj); done = 0; break; } @@ -2146,6 +2155,7 @@ retry_space: * if not found or wait and loop if busy. */ pindex = OFF_TO_IDX(off); + VM_OBJECT_WLOCK(obj); pg = vm_page_grab(obj, pindex, VM_ALLOC_NOBUSY | VM_ALLOC_NORMAL | VM_ALLOC_WIRED | VM_ALLOC_RETRY); @@ -2163,7 +2173,6 @@ retry_space: else if (uap->flags & SF_NODISKIO) error = EBUSY; else { - int bsize; ssize_t resid; /* @@ -2175,13 +2184,6 @@ retry_space: /* * Get the page from backing store. - */ - 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 @@ -2191,8 +2193,6 @@ retry_space: 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: VM_OBJECT_WLOCK(obj); vm_page_io_finish(pg); if (!error) @@ -2281,6 +2281,8 @@ retry_space: } } + VOP_UNLOCK(vp, 0); + /* Add the buffer chain to the socket buffer. */ if (m != NULL) { int mlen, err;