svn commit: r300851 - head/sys/kern

[Mateusz Guzik]

Author: mjg
Date: Fri May 27 15:03:38 2016
New Revision: 300851
URL: https://svnweb.freebsd.org/changeset/base/300851

Log:
  exec: get rid of one vnode lock/unlock pair in do_execve
  
  The lock was temporarily dropped for vrele calls, but they can be
  postponed to a point where the lock is not held in the first place.
  
  While here shuffle other code not needing the lock.

Modified:
  head/sys/kern/kern_exec.c

Modified: head/sys/kern/kern_exec.c
==============================================================================
--- head/sys/kern/kern_exec.c	Fri May 27 12:02:12 2016	(r300850)
+++ head/sys/kern/kern_exec.c	Fri May 27 15:03:38 2016	(r300851)
@@ -364,7 +364,7 @@ do_execve(td, args, mac_p)
 	struct vattr attr;
 	int (*img_first)(struct image_params *);
 	struct pargs *oldargs = NULL, *newargs = NULL;
-	struct sigacts *oldsigacts, *newsigacts;
+	struct sigacts *oldsigacts = NULL, *newsigacts = NULL;
 #ifdef KTRACE
 	struct vnode *tracevp = NULL;
 	struct ucred *tracecred = NULL;
@@ -714,8 +714,6 @@ interpret:
 		bcopy(imgp->args->begin_argv, newargs->ar_args, i);
 	}
 
-	vn_lock(imgp->vp, LK_SHARED | LK_RETRY);
-
 	/*
 	 * For security and other reasons, signal handlers cannot
 	 * be shared after an exec. The new process gets a copy of the old
@@ -726,11 +724,10 @@ interpret:
 		oldsigacts = p->p_sigacts;
 		newsigacts = sigacts_alloc();
 		sigacts_copy(newsigacts, oldsigacts);
-	} else {
-		oldsigacts = NULL;
-		newsigacts = NULL; /* satisfy gcc */
 	}
 
+	vn_lock(imgp->vp, LK_SHARED | LK_RETRY);
+
 	PROC_LOCK(p);
 	if (oldsigacts)
 		p->p_sigacts = newsigacts;
@@ -791,9 +788,9 @@ interpret:
 		VOP_UNLOCK(imgp->vp, 0);
 		fdsetugidsafety(td);
 		error = fdcheckstd(td);
-		if (error != 0)
-			goto done1;
 		vn_lock(imgp->vp, LK_SHARED | LK_RETRY);
+		if (error != 0)
+			goto exec_fail_dealloc;
 		PROC_LOCK(p);
 #ifdef MAC
 		if (will_transition) {
@@ -881,32 +878,7 @@ interpret:
 
 	SDT_PROBE1(proc, , , exec__success, args->fname);
 
-	VOP_UNLOCK(imgp->vp, 0);
-done1:
-	/*
-	 * Handle deferred decrement of ref counts.
-	 */
-	if (oldtextvp != NULL)
-		vrele(oldtextvp);
-#ifdef KTRACE
-	if (tracevp != NULL)
-		vrele(tracevp);
-	if (tracecred != NULL)
-		crfree(tracecred);
-#endif
-	vn_lock(imgp->vp, LK_SHARED | LK_RETRY);
-	pargs_drop(oldargs);
-	pargs_drop(newargs);
-	if (oldsigacts != NULL)
-		sigacts_free(oldsigacts);
-
 exec_fail_dealloc:
-	/*
-	 * free various allocated resources
-	 */
-	if (euip != NULL)
-		uifree(euip);
-
 	if (imgp->firstpage != NULL)
 		exec_unmap_first_page(imgp);
 
@@ -936,18 +908,16 @@ exec_fail_dealloc:
 		 * the S_EXEC bit set.
 		 */
 		STOPEVENT(p, S_EXEC, 0);
-		goto done2;
-	}
-
+	} else {
 exec_fail:
-	/* we're done here, clear P_INEXEC */
-	PROC_LOCK(p);
-	p->p_flag &= ~P_INEXEC;
-	PROC_UNLOCK(p);
+		/* we're done here, clear P_INEXEC */
+		PROC_LOCK(p);
+		p->p_flag &= ~P_INEXEC;
+		PROC_UNLOCK(p);
 
-	SDT_PROBE1(proc, , , exec__failure, error);
+		SDT_PROBE1(proc, , , exec__failure, error);
+	}
 
-done2:
 	if (imgp->newcred != NULL)
 		crfree(oldcred);
 #ifdef MAC
@@ -956,6 +926,24 @@ done2:
 #endif
 	exec_free_args(args);
 
+	/*
+	 * Handle deferred decrement of ref counts.
+	 */
+	if (oldtextvp != NULL)
+		vrele(oldtextvp);
+#ifdef KTRACE
+	if (tracevp != NULL)
+		vrele(tracevp);
+	if (tracecred != NULL)
+		crfree(tracecred);
+#endif
+	pargs_drop(oldargs);
+	pargs_drop(newargs);
+	if (oldsigacts != NULL)
+		sigacts_free(oldsigacts);
+	if (euip != NULL)
+		uifree(euip);
+
 	if (error && imgp->vmspace_destroyed) {
 		/* sorry, no more process anymore. exit gracefully */
 		exit1(td, 0, SIGABRT);