Skip site navigation (1)Skip section navigation (2) 
Date:      Wed, 27 Apr 2016 02:25:21 +0000 (UTC)
From:      Jamie Gritton <jamie@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r298683 - in head/sys: kern sys
Message-ID:  <201604270225.u3R2PLG9045458@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: jamie
Date: Wed Apr 27 02:25:21 2016
New Revision: 298683
URL: https://svnweb.freebsd.org/changeset/base/298683

Log:
  Delay revmoing the last jail reference in prison_proc_free, and instead
  put it off into the pr_task.  This is similar to prison_free, and in fact
  uses the same task even though they do something slightly different.
  
  This resolves a LOR between the process lock and allprison_lock, which
  came about in r298565.
  
  PR:		48471

Modified:
  head/sys/kern/kern_jail.c
  head/sys/sys/jail.h

Modified: head/sys/kern/kern_jail.c
==============================================================================
--- head/sys/kern/kern_jail.c	Wed Apr 27 02:13:57 2016	(r298682)
+++ head/sys/kern/kern_jail.c	Wed Apr 27 02:25:21 2016	(r298683)
@@ -1328,6 +1328,7 @@ kern_jail_set(struct thread *td, struct 
 
 		LIST_INIT(&pr->pr_children);
 		mtx_init(&pr->pr_mtx, "jail mutex", NULL, MTX_DEF | MTX_DUPOK);
+		TASK_INIT(&pr->pr_task, 0, prison_complete, pr);
 
 #ifdef VIMAGE
 		/* Allocate a new vnet if specified. */
@@ -2575,16 +2576,13 @@ prison_allow(struct ucred *cred, unsigne
 void
 prison_free_locked(struct prison *pr)
 {
+	int ref;
 
 	mtx_assert(&pr->pr_mtx, MA_OWNED);
-	pr->pr_ref--;
-	if (pr->pr_ref == 0) {
-		mtx_unlock(&pr->pr_mtx);
-		TASK_INIT(&pr->pr_task, 0, prison_complete, pr);
-		taskqueue_enqueue(taskqueue_thread, &pr->pr_task);
-		return;
-	}
+	ref = --pr->pr_ref;
 	mtx_unlock(&pr->pr_mtx);
+	if (ref == 0)
+		taskqueue_enqueue(taskqueue_thread, &pr->pr_task);
 }
 
 void
@@ -2595,11 +2593,17 @@ prison_free(struct prison *pr)
 	prison_free_locked(pr);
 }
 
+/*
+ * Complete a call to either prison_free or prison_proc_free.
+ */
 static void
 prison_complete(void *context, int pending)
 {
+	struct prison *pr = context;
 
-	prison_deref((struct prison *)context, 0);
+	mtx_lock(&pr->pr_mtx);
+	prison_deref(pr, pr->pr_uref
+	    ? PD_DEREF | PD_DEUREF | PD_LOCKED : PD_LOCKED);
 }
 
 /*
@@ -2618,6 +2622,9 @@ prison_deref(struct prison *pr, int flag
 		mtx_lock(&pr->pr_mtx);
 	for (;;) {
 		if (flags & PD_DEUREF) {
+			KASSERT(pr->pr_uref > 0,
+			    ("prison_deref PD_DEUREF on a dead prison (jid=%d)",
+			     pr->pr_id));
 			pr->pr_uref--;
 			lasturef = pr->pr_uref == 0;
 			if (lasturef)
@@ -2625,8 +2632,12 @@ prison_deref(struct prison *pr, int flag
 			KASSERT(prison0.pr_uref != 0, ("prison0 pr_uref=0"));
 		} else
 			lasturef = 0;
-		if (flags & PD_DEREF)
+		if (flags & PD_DEREF) {
+			KASSERT(pr->pr_ref > 0,
+			    ("prison_deref PD_DEREF on a dead prison (jid=%d)",
+			     pr->pr_id));
 			pr->pr_ref--;
+		}
 		ref = pr->pr_ref;
 		mtx_unlock(&pr->pr_mtx);
 
@@ -2740,7 +2751,20 @@ prison_proc_free(struct prison *pr)
 	mtx_lock(&pr->pr_mtx);
 	KASSERT(pr->pr_uref > 0,
 	    ("Trying to kill a process in a dead prison (jid=%d)", pr->pr_id));
-	prison_deref(pr, PD_DEUREF | PD_LOCKED);
+	if (pr->pr_uref > 1)
+		pr->pr_uref--;
+	else {
+		/*
+		 * Don't remove the last user reference in this context, which
+		 * is expected to be a process that is not only locked, but
+		 * also half dead.
+		 */
+		pr->pr_ref++;
+		mtx_unlock(&pr->pr_mtx);
+		taskqueue_enqueue(taskqueue_thread, &pr->pr_task);
+		return;
+	}
+	mtx_unlock(&pr->pr_mtx);
 }
 
 

Modified: head/sys/sys/jail.h
==============================================================================
--- head/sys/sys/jail.h	Wed Apr 27 02:13:57 2016	(r298682)
+++ head/sys/sys/jail.h	Wed Apr 27 02:25:21 2016	(r298683)
@@ -149,7 +149,6 @@ struct prison_racct;
  *   (p) locked by pr_mtx
  *   (c) set only during creation before the structure is shared, no mutex
  *       required to read
- *   (d) set only during destruction of jail, no mutex needed
  */
 struct prison {
 	TAILQ_ENTRY(prison) pr_list;			/* (a) all prisons */
@@ -161,7 +160,7 @@ struct prison {
 	LIST_ENTRY(prison) pr_sibling;			/* (a) next in parent's list */
 	struct prison	*pr_parent;			/* (c) containing jail */
 	struct mtx	 pr_mtx;
-	struct task	 pr_task;			/* (d) destroy task */
+	struct task	 pr_task;			/* (c) destroy task */
 	struct osd	 pr_osd;			/* (p) additional data */
 	struct cpuset	*pr_cpuset;			/* (p) cpuset */
 	struct vnet	*pr_vnet;			/* (c) network stack */

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