From owner-svn-src-all@FreeBSD.ORG Wed Jun 13 21:32:35 2012 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (unknown [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id ADDCA106566C; Wed, 13 Jun 2012 21:32:35 +0000 (UTC) (envelope-from pjd@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [69.147.83.44]) by mx1.freebsd.org (Postfix) with ESMTP id 999178FC0C; Wed, 13 Jun 2012 21:32:35 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.4/8.14.4) with ESMTP id q5DLWZxF050199; Wed, 13 Jun 2012 21:32:35 GMT (envelope-from pjd@svn.freebsd.org) Received: (from pjd@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id q5DLWZfo050195; Wed, 13 Jun 2012 21:32:35 GMT (envelope-from pjd@svn.freebsd.org) Message-Id: <201206132132.q5DLWZfo050195@svn.freebsd.org> From: Pawel Jakub Dawidek Date: Wed, 13 Jun 2012 21:32:35 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r237033 - in head/sys: kern sys X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 13 Jun 2012 21:32:35 -0000 Author: pjd Date: Wed Jun 13 21:32:35 2012 New Revision: 237033 URL: http://svn.freebsd.org/changeset/base/237033 Log: Allocate descriptor number in dupfdopen() itself instead of depending on the caller using finstall(). This saves us the filedesc lock/unlock cycle, fhold()/fdrop() cycle and closes a race between finstall() and dupfdopen(). MFC after: 1 month Modified: head/sys/kern/kern_descrip.c head/sys/kern/vfs_syscalls.c head/sys/sys/filedesc.h Modified: head/sys/kern/kern_descrip.c ============================================================================== --- head/sys/kern/kern_descrip.c Wed Jun 13 21:22:35 2012 (r237032) +++ head/sys/kern/kern_descrip.c Wed Jun 13 21:32:35 2012 (r237033) @@ -2588,13 +2588,13 @@ done2: * Duplicate the specified descriptor to a free descriptor. */ int -dupfdopen(struct thread *td, struct filedesc *fdp, int indx, int dfd, int mode, int error) +dupfdopen(struct thread *td, struct filedesc *fdp, int dfd, int mode, int openerror, int *indxp) { - struct file *wfp; struct file *fp; + int error, indx; - KASSERT(error == ENODEV || error == ENXIO, - ("unexpected error %d in %s", error, __func__)); + KASSERT(openerror == ENODEV || openerror == ENXIO, + ("unexpected error %d in %s", openerror, __func__)); /* * If the to-be-dup'd fd number is greater than the allowed number @@ -2603,11 +2603,17 @@ dupfdopen(struct thread *td, struct file */ FILEDESC_XLOCK(fdp); if ((unsigned int)dfd >= fdp->fd_nfiles || - (wfp = fdp->fd_ofiles[dfd]) == NULL) { + (fp = fdp->fd_ofiles[dfd]) == NULL) { FILEDESC_XUNLOCK(fdp); return (EBADF); } + error = fdalloc(td, 0, &indx); + if (error != 0) { + FILEDESC_XUNLOCK(fdp); + return (error); + } + /* * There are two cases of interest here. * @@ -2616,26 +2622,26 @@ dupfdopen(struct thread *td, struct file * For ENXIO steal away the file structure from (dfd) and store it in * (indx). (dfd) is effectively closed by this operation. */ - fp = fdp->fd_ofiles[indx]; - switch (error) { + switch (openerror) { case ENODEV: /* * Check that the mode the file is being opened for is a * subset of the mode of the existing descriptor. */ - if (((mode & (FREAD|FWRITE)) | wfp->f_flag) != wfp->f_flag) { + if (((mode & (FREAD|FWRITE)) | fp->f_flag) != fp->f_flag) { + fdunused(fdp, indx); FILEDESC_XUNLOCK(fdp); return (EACCES); } - fdp->fd_ofiles[indx] = wfp; + fdp->fd_ofiles[indx] = fp; fdp->fd_ofileflags[indx] = fdp->fd_ofileflags[dfd]; - fhold(wfp); + fhold(fp); break; case ENXIO: /* * Steal away the file pointer from dfd and stuff it into indx. */ - fdp->fd_ofiles[indx] = wfp; + fdp->fd_ofiles[indx] = fp; fdp->fd_ofiles[dfd] = NULL; fdp->fd_ofileflags[indx] = fdp->fd_ofileflags[dfd]; fdp->fd_ofileflags[dfd] = 0; @@ -2643,11 +2649,7 @@ dupfdopen(struct thread *td, struct file break; } FILEDESC_XUNLOCK(fdp); - /* - * We now own the reference to fp that the ofiles[] array used to own. - * Release it. - */ - fdrop(fp, td); + *indxp = indx; return (0); } Modified: head/sys/kern/vfs_syscalls.c ============================================================================== --- head/sys/kern/vfs_syscalls.c Wed Jun 13 21:22:35 2012 (r237032) +++ head/sys/kern/vfs_syscalls.c Wed Jun 13 21:32:35 2012 (r237033) @@ -1093,7 +1093,7 @@ kern_openat(struct thread *td, int fd, c struct file *fp; struct vnode *vp; int cmode; - int type, indx = -1, error, error_open; + int type, indx = -1, error; struct flock lf; struct nameidata nd; int vfslocked; @@ -1143,9 +1143,7 @@ kern_openat(struct thread *td, int fd, c goto success; /* - * Handle special fdopen() case. bleh. dupfdopen() is - * responsible for dropping the old contents of ofiles[indx] - * if it succeeds. + * Handle special fdopen() case. bleh. * * Don't do this for relative (capability) lookups; we don't * understand exactly what would happen, and we don't think @@ -1154,14 +1152,12 @@ kern_openat(struct thread *td, int fd, c if (nd.ni_strictrelative == 0 && (error == ENODEV || error == ENXIO) && td->td_dupfd >= 0) { - /* XXX from fdopen */ - error_open = error; - if ((error = finstall(td, fp, &indx, flags)) != 0) - goto bad_unlocked; - if ((error = dupfdopen(td, fdp, indx, td->td_dupfd, - flags, error_open)) == 0) + error = dupfdopen(td, fdp, td->td_dupfd, flags, error, + &indx); + if (error == 0) goto success; } + if (error == ERESTART) error = EINTR; goto bad_unlocked; @@ -4514,11 +4510,11 @@ sys_fhopen(td, uap) VFS_UNLOCK_GIANT(vfslocked); return (error); } - /* * An extra reference on `fp' has been held for us by * falloc_noinstall(). */ + #ifdef INVARIANTS td->td_dupfd = -1; #endif Modified: head/sys/sys/filedesc.h ============================================================================== --- head/sys/sys/filedesc.h Wed Jun 13 21:22:35 2012 (r237032) +++ head/sys/sys/filedesc.h Wed Jun 13 21:32:35 2012 (r237033) @@ -109,8 +109,8 @@ struct filedesc_to_leader { struct thread; int closef(struct file *fp, struct thread *td); -int dupfdopen(struct thread *td, struct filedesc *fdp, int indx, int dfd, - int mode, int error); +int dupfdopen(struct thread *td, struct filedesc *fdp, int dfd, int mode, + int openerror, int *indxp); int falloc(struct thread *td, struct file **resultfp, int *resultfd, int flags); int falloc_noinstall(struct thread *td, struct file **resultfp);