Skip site navigation (1)Skip section navigation (2) 
Date:      Wed, 13 Jun 2012 21:32:35 +0000 (UTC)
From:      Pawel Jakub Dawidek <pjd@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r237033 - in head/sys: kern sys
Message-ID:  <201206132132.q5DLWZfo050195@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
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);

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201206132132.q5DLWZfo050195>