[src/trunk]: src/sys/opencrypto fix processes accessing /dev/crypto stall whe...

[knakahara <knakahara%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/85b39a5fb8c9
branches:  trunk
changeset: 352498:85b39a5fb8c9
user:      knakahara <knakahara%NetBSD.org@localhost>
date:      Wed Apr 05 08:51:04 2017 +0000

description:
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.

diffstat:

 sys/opencrypto/crypto.c    |  26 +++++++++++++++-----------
 sys/opencrypto/cryptodev.c |  19 ++++++++-----------
 2 files changed, 23 insertions(+), 22 deletions(-)

diffs (106 lines):

diff -r 7722bee166d6 -r 85b39a5fb8c9 sys/opencrypto/crypto.c
--- a/sys/opencrypto/crypto.c   Wed Apr 05 04:04:54 2017 +0000
+++ b/sys/opencrypto/crypto.c   Wed Apr 05 08:51:04 2017 +0000
@@ -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 @@
                                (*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 @@
        } 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));
diff -r 7722bee166d6 -r 85b39a5fb8c9 sys/opencrypto/cryptodev.c
--- a/sys/opencrypto/cryptodev.c        Wed Apr 05 04:04:54 2017 +0000
+++ b/sys/opencrypto/cryptodev.c        Wed Apr 05 08:51:04 2017 +0000
@@ -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 @@
 
 
        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 @@
        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 */