Module Name: src
Committed By: knakahara
Date: Wed Apr 5 08:51:04 UTC 2017
Modified Files:
src/sys/opencrypto: crypto.c cryptodev.c
Log Message:
fix processes accessing /dev/crypto stall when over three processes run with a
hardware encryption driver
The process has stalled at cv_wait(&crp->crp_cv) because cryptodev_cb()
is not called as cryptoret() kthread keep waiting at cv_wait(&cryptoret_cv).
Previous opencrypto implementation assumes the thread from cryptodev.c
does all processing in the same context, so skips enqueueing and sending
cryptoret_cv. However, the context can be switched, e.g. when we use
a hardware encryption driver.
And add debug messages.
To generate a diff of this commit:
cvs rdiff -u -r1.51 -r1.52 src/sys/opencrypto/crypto.c
cvs rdiff -u -r1.85 -r1.86 src/sys/opencrypto/cryptodev.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/opencrypto/crypto.c
diff -u src/sys/opencrypto/crypto.c:1.51 src/sys/opencrypto/crypto.c:1.52
--- src/sys/opencrypto/crypto.c:1.51 Wed Mar 29 23:02:43 2017
+++ src/sys/opencrypto/crypto.c Wed Apr 5 08:51:04 2017
@@ -1,4 +1,4 @@
-/* $NetBSD: crypto.c,v 1.51 2017/03/29 23:02:43 knakahara Exp $ */
+/* $NetBSD: crypto.c,v 1.52 2017/04/05 08:51:04 knakahara Exp $ */
/* $FreeBSD: src/sys/opencrypto/crypto.c,v 1.4.2.5 2003/02/26 00:14:05 sam Exp $ */
/* $OpenBSD: crypto.c,v 1.41 2002/07/17 23:52:38 art Exp $ */
@@ -53,7 +53,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.51 2017/03/29 23:02:43 knakahara Exp $");
+__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.52 2017/04/05 08:51:04 knakahara Exp $");
#include <sys/param.h>
#include <sys/reboot.h>
@@ -418,6 +418,9 @@ crypto_newsession(u_int64_t *sid, struct
(*sid) <<= 32;
(*sid) |= (lid & 0xffffffff);
crypto_drivers[hid].cc_sessions++;
+ } else {
+ DPRINTF(("%s: crypto_drivers[%d].cc_newsession() failed. error=%d\n",
+ __func__, hid, error));
}
goto done;
/*break;*/
@@ -1124,19 +1127,20 @@ crypto_done(struct cryptop *crp)
} else {
mutex_spin_enter(&crypto_ret_q_mtx);
crp->crp_flags |= CRYPTO_F_DONE;
-
+#if 0
if (crp->crp_flags & CRYPTO_F_USER) {
- /* the request has completed while
- * running in the user context
- * so don't queue it - the user
- * thread won't sleep when it sees
- * the CRYPTO_F_DONE flag.
- * This is an optimization to avoid
- * unecessary context switches.
+ /*
+ * TODO:
+ * If crp->crp_flags & CRYPTO_F_USER and the used
+ * encryption driver does all the processing in
+ * the same context, we can skip enqueueing crp_ret_q
+ * and cv_signal(&cryptoret_cv).
*/
DPRINTF(("crypto_done[%u]: crp %p CRYPTO_F_USER\n",
CRYPTO_SESID2LID(crp->crp_sid), crp));
- } else {
+ } else
+#endif
+ {
wasempty = TAILQ_EMPTY(&crp_ret_q);
DPRINTF(("crypto_done[%u]: queueing %p\n",
CRYPTO_SESID2LID(crp->crp_sid), crp));
Index: src/sys/opencrypto/cryptodev.c
diff -u src/sys/opencrypto/cryptodev.c:1.85 src/sys/opencrypto/cryptodev.c:1.86
--- src/sys/opencrypto/cryptodev.c:1.85 Thu Jul 7 06:55:43 2016
+++ src/sys/opencrypto/cryptodev.c Wed Apr 5 08:51:04 2017
@@ -1,4 +1,4 @@
-/* $NetBSD: cryptodev.c,v 1.85 2016/07/07 06:55:43 msaitoh Exp $ */
+/* $NetBSD: cryptodev.c,v 1.86 2017/04/05 08:51:04 knakahara Exp $ */
/* $FreeBSD: src/sys/opencrypto/cryptodev.c,v 1.4.2.4 2003/06/03 00:09:02 sam Exp $ */
/* $OpenBSD: cryptodev.c,v 1.53 2002/07/10 22:21:30 mickey Exp $ */
@@ -64,7 +64,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.85 2016/07/07 06:55:43 msaitoh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.86 2017/04/05 08:51:04 knakahara Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -578,10 +578,10 @@ cryptodev_op(struct csession *cse, struc
crp->crp_ilen = cop->len;
- /* The reqest is flagged as CRYPTO_F_USER as long as it is running
- * in the user IOCTL thread. This flag lets us skip using the retq for
- * the request if it completes immediately. If the request ends up being
- * delayed or is not completed immediately the flag is removed.
+ /*
+ * The request is flagged as CRYPTO_F_USER as long as it is running
+ * in the user IOCTL thread. However, whether the request completes
+ * immediately or belatedly is depends on the used encryption driver.
*/
crp->crp_flags = CRYPTO_F_IOV | (cop->flags & COP_F_BATCH) | CRYPTO_F_USER |
flags;
@@ -643,12 +643,9 @@ eagain:
mutex_enter(&crypto_mtx);
/*
- * If the request was going to be completed by the
- * ioctl thread then it would have been done by now.
- * Remove the F_USER flag so crypto_done() is not confused
- * if the crypto device calls it after this point.
+ * Don't touch crp before returned by any error or recieved
+ * cv_signal(&crp->crp_cv). It is required to restructure locks.
*/
- crp->crp_flags &= ~(CRYPTO_F_USER);
switch (error) {
#ifdef notyet /* don't loop forever -- but EAGAIN not possible here yet */
