manage kernel threads properly; especially shutting them down on module unload

Reviewed by:	jhb

1 files changed, 133 insertions, 52 deletions

diff --git a/sys/opencrypto/crypto.c b/sys/opencrypto/crypto.c
index 6de814f..fd3fd85 100644
--- a/sys/opencrypto/crypto.c
+++ b/sys/opencrypto/crypto.c
@@ -91,6 +91,12 @@ SYSCTL_INT(_kern, OID_AUTO, cryptodevallowsoft, CTLFLAG_RW,
 
 MALLOC_DEFINE(M_CRYPTO_DATA, "crypto", "crypto session records");
 
+static	void crypto_proc(void);
+static	struct proc *cryptoproc;
+static	void crypto_ret_proc(void);
+static	struct proc *cryptoretproc;
+static	void crypto_destroy(void);
+
 static	struct cryptostats cryptostats;
 SYSCTL_STRUCT(_kern, OID_AUTO, crypto_stats, CTLFLAG_RW, &cryptostats,
 	    cryptostats, "Crypto system statistics");
@@ -101,34 +107,115 @@ SYSCTL_INT(_debug, OID_AUTO, crypto_timing, CTLFLAG_RW,
 	   &crypto_timing, 0, "Enable/disable crypto timing support");
 #endif
 
-static void
+static int
 crypto_init(void)
 {
+	int error;
+
+	mtx_init(&crypto_drivers_mtx, "crypto driver table",
+		NULL, MTX_DEF|MTX_QUIET);
+
+	TAILQ_INIT(&crp_q);
+	TAILQ_INIT(&crp_kq);
+	mtx_init(&crypto_q_mtx, "crypto op queues", NULL, MTX_DEF);
+
+	TAILQ_INIT(&crp_ret_q);
+	TAILQ_INIT(&crp_ret_kq);
+	mtx_init(&crypto_ret_q_mtx, "crypto return queues", NULL, MTX_DEF);
+
 	cryptop_zone = uma_zcreate("cryptop", sizeof (struct cryptop),
 				    0, 0, 0, 0,
 				    UMA_ALIGN_PTR, UMA_ZONE_ZINIT);
 	cryptodesc_zone = uma_zcreate("cryptodesc", sizeof (struct cryptodesc),
 				    0, 0, 0, 0,
 				    UMA_ALIGN_PTR, UMA_ZONE_ZINIT);
-	if (cryptodesc_zone == NULL || cryptop_zone == NULL)
-		panic("cannot setup crypto zones");
-
-	mtx_init(&crypto_drivers_mtx, "crypto driver table",
-		NULL, MTX_DEF|MTX_QUIET);
+	if (cryptodesc_zone == NULL || cryptop_zone == NULL) {
+		printf("crypto_init: cannot setup crypto zones\n");
+		error = ENOMEM;
+		goto bad;
+	}
 
 	crypto_drivers_num = CRYPTO_DRIVERS_INITIAL;
 	crypto_drivers = malloc(crypto_drivers_num *
 	    sizeof(struct cryptocap), M_CRYPTO_DATA, M_NOWAIT | M_ZERO);
-	if (crypto_drivers == NULL)
-		panic("cannot setup crypto drivers");
+	if (crypto_drivers == NULL) {
+		printf("crypto_init: cannot setup crypto drivers\n");
+		error = ENOMEM;
+		goto bad;
+	}
 
-	TAILQ_INIT(&crp_q);
-	TAILQ_INIT(&crp_kq);
-	mtx_init(&crypto_q_mtx, "crypto op queues", NULL, MTX_DEF);
+	error = kthread_create((void (*)(void *)) crypto_proc, NULL,
+		    &cryptoproc, 0, 0, "crypto");
+	if (error) {
+		printf("crypto_init: cannot start crypto thread; error %d",
+			error);
+		goto bad;
+	}
 
-	TAILQ_INIT(&crp_ret_q);
-	TAILQ_INIT(&crp_ret_kq);
-	mtx_init(&crypto_ret_q_mtx, "crypto return queues", NULL, MTX_DEF);
+	error = kthread_create((void (*)(void *)) crypto_ret_proc, NULL,
+		    &cryptoretproc, 0, 0, "crypto returns");
+	if (error) {
+		printf("crypto_init: cannot start cryptoret thread; error %d",
+			error);
+		goto bad;
+	}
+	return 0;
+bad:
+	crypto_destroy();
+	return error;
+}
+
+/*
+ * Signal a crypto thread to terminate.  We use the driver
+ * table lock to synchronize the sleep/wakeups so that we
+ * are sure the threads have terminated before we release
+ * the data structures they use.  See crypto_finis below
+ * for the other half of this song-and-dance.
+ */
+static void
+crypto_terminate(struct proc **pp, void *q)
+{
+	struct proc *p;
+
+	mtx_assert(&crypto_drivers_mtx, MA_OWNED);
+	p = *pp;
+	*pp = NULL;
+	if (p) {
+		wakeup_one(q);
+		PROC_LOCK(p);		/* NB: insure we don't miss wakeup */
+		CRYPTO_DRIVER_UNLOCK();	/* let crypto_finis progress */
+		msleep(p, &p->p_mtx, PWAIT, "crypto_destroy", 0);
+		PROC_UNLOCK(p);
+		CRYPTO_DRIVER_LOCK();
+	}
+}
+
+static void
+crypto_destroy(void)
+{
+	/*
+	 * Terminate any crypto threads.
+	 */
+	CRYPTO_DRIVER_LOCK();
+	crypto_terminate(&cryptoproc, &crp_q);
+	crypto_terminate(&cryptoretproc, &crp_ret_q);
+	CRYPTO_DRIVER_UNLOCK();
+
+	/* XXX flush queues??? */
+
+	/* 
+	 * Reclaim dynamically allocated resources.
+	 */
+	if (crypto_drivers != NULL)
+		free(crypto_drivers, M_CRYPTO_DATA);
+
+	if (cryptodesc_zone != NULL)
+		uma_zdestroy(cryptodesc_zone);
+	if (cryptop_zone != NULL)
+		uma_zdestroy(cryptop_zone);
+	mtx_destroy(&crypto_q_mtx);
+	mtx_destroy(&crypto_ret_q_mtx);
+	mtx_destroy(&crypto_drivers_mtx);
 }
 
 /*
@@ -137,18 +224,21 @@ crypto_init(void)
 static int
 crypto_modevent(module_t mod, int type, void *unused)
 {
+	int error = EINVAL;
+
 	switch (type) {
 	case MOD_LOAD:
-		crypto_init();
-		if (bootverbose)
+		error = crypto_init();
+		if (error == 0 && bootverbose)
 			printf("crypto: <crypto core>\n");
-		return 0;
+		break;
 	case MOD_UNLOAD:
 		/*XXX disallow if active sessions */
-		/*XXX kill kthreads */
+		error = 0;
+		crypto_destroy();
 		return 0;
 	}
-	return EINVAL;
+	return error;
 }
 
 static moduledata_t crypto_mod = {
@@ -859,12 +949,23 @@ out:
 	return (0);
 }
 
-static struct proc *cryptoproc;
-
+/*
+ * Terminate a thread at module unload.  The process that
+ * initiated this is waiting for us to signal that we're gone;
+ * wake it up and exit.  We use the driver table lock to insure
+ * we don't do the wakeup before they're waiting.  There is no
+ * race here because the waiter sleeps on the proc lock for the
+ * thread so it gets notified at the right time because of an
+ * extra wakeup that's done in exit1().
+ */
 static void
-crypto_shutdown(void *arg, int howto)
+crypto_finis(void *chan)
 {
-	/* XXX flush queues */
+	CRYPTO_DRIVER_LOCK();
+	wakeup_one(chan);
+	CRYPTO_DRIVER_UNLOCK();
+	mtx_lock(&Giant);
+	kthread_exit(0);
 }
 
 /*
@@ -878,11 +979,7 @@ crypto_proc(void)
 	struct cryptocap *cap;
 	int result, hint;
 
-	EVENTHANDLER_REGISTER(shutdown_pre_sync, crypto_shutdown, NULL,
-			      SHUTDOWN_PRI_FIRST);
-
 	CRYPTO_Q_LOCK();
-
 	for (;;) {
 		/*
 		 * Find the first element in the queue that can be
@@ -985,24 +1082,14 @@ crypto_proc(void)
 			 * and some become blocked while others do not.
 			 */
 			msleep(&crp_q, &crypto_q_mtx, PWAIT, "crypto_wait", 0);
+			if (cryptoproc == NULL)
+				break;
 			cryptostats.cs_intrs++;
 		}
 	}
-}
-static struct kproc_desc crypto_kp = {
-	"crypto",
-	crypto_proc,
-	&cryptoproc
-};
-SYSINIT(crypto_proc, SI_SUB_KTHREAD_IDLE, SI_ORDER_THIRD,
-	kproc_start, &crypto_kp)
-
-static struct proc *cryptoretproc;
+	CRYPTO_Q_UNLOCK();
 
-static void
-crypto_ret_shutdown(void *arg, int howto)
-{
-	/* XXX flush queues */
+	crypto_finis(&crp_q);
 }
 
 /*
@@ -1016,11 +1103,7 @@ crypto_ret_proc(void)
 	struct cryptop *crpt;
 	struct cryptkop *krpt;
 
-	EVENTHANDLER_REGISTER(shutdown_pre_sync, crypto_ret_shutdown, NULL,
-			      SHUTDOWN_PRI_FIRST);
-
 	CRYPTO_RETQ_LOCK();
-
 	for (;;) {
 		/* Harvest return q's for completed ops */
 		crpt = TAILQ_FIRST(&crp_ret_q);
@@ -1062,14 +1145,12 @@ crypto_ret_proc(void)
 			 */
 			msleep(&crp_ret_q, &crypto_ret_q_mtx, PWAIT,
 				"crypto_ret_wait", 0);
+			if (cryptoretproc == NULL)
+				break;
 			cryptostats.cs_rets++;
 		}
 	}
+	CRYPTO_RETQ_UNLOCK();
+
+	crypto_finis(&crp_ret_q);
 }
-static struct kproc_desc crypto_ret_kp = {
-	"crypto returns",
-	crypto_ret_proc,
-	&cryptoretproc
-};
-SYSINIT(crypto_ret_proc, SI_SUB_KTHREAD_IDLE, SI_ORDER_THIRD,
-	kproc_start, &crypto_ret_kp)