Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/opencrypto restructure locks(1/2): make relation between...



details:   https://anonhg.NetBSD.org/src/rev/38fdb147a400
branches:  trunk
changeset: 354163:38fdb147a400
user:      knakahara <knakahara%NetBSD.org@localhost>
date:      Tue Jun 06 01:45:57 2017 +0000

description:
restructure locks(1/2): make relation between lock and data explicit.

    + crypto_drv_mtx protects
      -  whole crypto_drivers
    + crypto_drivers[i].cc_lock (new) protects
      - crypto_drivers[i] itself
      - member of crypto_drivers[i]
    + crypto_q_mtx protects
      - crp_q
      - crp_kq
    + crypto_ret_q_mtx protects
      - crp_ret_q
      - crp_ret_kq
      - crypto_exit_flag

I will add locking note later.

diffstat:

 sys/opencrypto/crypto.c    |  252 +++++++++++++++++++++++++++++++-------------
 sys/opencrypto/cryptodev.h |    4 +-
 2 files changed, 177 insertions(+), 79 deletions(-)

diffs (truncated from 651 to 300 lines):

diff -r 7779cd2284f1 -r 38fdb147a400 sys/opencrypto/crypto.c
--- a/sys/opencrypto/crypto.c   Tue Jun 06 00:28:05 2017 +0000
+++ b/sys/opencrypto/crypto.c   Tue Jun 06 01:45:57 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: crypto.c,v 1.80 2017/06/05 09:09:13 knakahara Exp $ */
+/*     $NetBSD: crypto.c,v 1.81 2017/06/06 01:45:57 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.80 2017/06/05 09:09:13 knakahara Exp $");
+__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.81 2017/06/06 01:45:57 knakahara Exp $");
 
 #include <sys/param.h>
 #include <sys/reboot.h>
@@ -374,8 +374,11 @@
 static int crypto_invoke(struct cryptop *crp, int hint);
 static int crypto_kinvoke(struct cryptkop *krp, int hint);
 
-static struct cryptocap *crypto_checkdriver(u_int32_t);
+static struct cryptocap *crypto_checkdriver_lock(u_int32_t);
 static struct cryptocap *crypto_checkdriver_uninit(u_int32_t);
+static void crypto_driver_lock(struct cryptocap *);
+static void crypto_driver_unlock(struct cryptocap *);
+static void crypto_driver_clear(struct cryptocap *);
 
 static struct cryptostats cryptostats;
 #ifdef CRYPTO_TIMING
@@ -438,26 +441,33 @@
        if (exit_kthread) {
                struct cryptocap *cap = NULL;
 
-               mutex_spin_enter(&crypto_ret_q_mtx);
-
                /* if we have any in-progress requests, don't unload */
+               mutex_spin_enter(&crypto_q_mtx);
                if (!TAILQ_EMPTY(&crp_q) || !TAILQ_EMPTY(&crp_kq)) {
-                       mutex_spin_exit(&crypto_ret_q_mtx);
+                       mutex_spin_exit(&crypto_q_mtx);
                        return EBUSY;
                }
+               mutex_spin_exit(&crypto_q_mtx);
+               /* FIXME:
+                * prohibit enqueue to crp_q and crp_kq after here.
+                */
 
+               mutex_enter(&crypto_drv_mtx);
                for (i = 0; i < crypto_drivers_num; i++) {
                        cap = crypto_checkdriver_uninit(i);
                        if (cap == NULL)
                                continue;
-                       if (cap->cc_sessions != 0)
-                               break;
+                       if (cap->cc_sessions != 0) {
+                               mutex_exit(&crypto_drv_mtx);
+                               return EBUSY;
+                       }
                }
-               if (cap != NULL) {
-                       mutex_spin_exit(&crypto_ret_q_mtx);
-                       return EBUSY;
-               }
+               mutex_exit(&crypto_drv_mtx);
+               /* FIXME:
+                * prohibit touch crypto_drivers[] and each element after here.
+                */
 
+               mutex_spin_enter(&crypto_ret_q_mtx);
                /* kick the cryptoret thread and wait for it to exit */
                crypto_exit_flag = 1;
                cv_signal(&cryptoret_cv);
@@ -516,20 +526,28 @@
                if (cap == NULL)
                        continue;
 
+               crypto_driver_lock(cap);
+
                /*
                 * If it's not initialized or has remaining sessions
                 * referencing it, skip.
                 */
                if (cap->cc_newsession == NULL ||
-                   (cap->cc_flags & CRYPTOCAP_F_CLEANUP))
+                   (cap->cc_flags & CRYPTOCAP_F_CLEANUP)) {
+                       crypto_driver_unlock(cap);
                        continue;
+               }
 
                /* Hardware required -- ignore software drivers. */
-               if (hard > 0 && (cap->cc_flags & CRYPTOCAP_F_SOFTWARE))
+               if (hard > 0 && (cap->cc_flags & CRYPTOCAP_F_SOFTWARE)) {
+                       crypto_driver_unlock(cap);
                        continue;
+               }
                /* Software required -- ignore hardware drivers. */
-               if (hard < 0 && (cap->cc_flags & CRYPTOCAP_F_SOFTWARE) == 0)
+               if (hard < 0 && (cap->cc_flags & CRYPTOCAP_F_SOFTWARE) == 0) {
+                       crypto_driver_unlock(cap);
                        continue;
+               }
 
                /* See if all the algorithms are supported. */
                for (cr = cri; cr; cr = cr->cri_next)
@@ -560,9 +578,12 @@
                                DPRINTF("crypto_drivers[%d].cc_newsession() failed. error=%d\n",
                                        hid, err);
                        }
+                       crypto_driver_unlock(cap);
                        goto done;
                        /*break;*/
                }
+
+               crypto_driver_unlock(cap);
        }
 done:
        mutex_exit(&crypto_drv_mtx);
@@ -579,14 +600,10 @@
        struct cryptocap *cap;
        int err = 0;
 
-       mutex_enter(&crypto_drv_mtx);
-
        /* Determine two IDs. */
-       cap = crypto_checkdriver(CRYPTO_SESID2HID(sid));
-       if (cap == NULL) {
-               err = ENOENT;
-               goto done;
-       }
+       cap = crypto_checkdriver_lock(CRYPTO_SESID2HID(sid));
+       if (cap == NULL)
+               return ENOENT;
 
        if (cap->cc_sessions)
                (cap->cc_sessions)--;
@@ -602,10 +619,9 @@
         * make the entry available for reuse.
         */
        if ((cap->cc_flags & CRYPTOCAP_F_CLEANUP) && cap->cc_sessions == 0)
-               memset(cap, 0, sizeof(struct cryptocap));
+               crypto_driver_clear(cap);
 
-done:
-       mutex_exit(&crypto_drv_mtx);
+       crypto_driver_unlock(cap);
        return err;
 }
 
@@ -665,6 +681,7 @@
        /* NB: state is zero'd on free */
        cap->cc_sessions = 1;   /* Mark */
        cap->cc_flags = flags;
+       mutex_init(&cap->cc_lock, MUTEX_DEFAULT, IPL_NET);
 
        if (bootverbose)
                printf("crypto: assign driver %u, flags %u\n", i, flags);
@@ -675,12 +692,18 @@
 }
 
 static struct cryptocap *
-crypto_checkdriver(u_int32_t hid)
+crypto_checkdriver_lock(u_int32_t hid)
 {
+       struct cryptocap *cap;
 
        KASSERT(crypto_drivers != NULL);
 
-       return (hid >= crypto_drivers_num ? NULL : &crypto_drivers[hid]);
+       if (hid >= crypto_drivers_num)
+               return NULL;
+
+       cap = &crypto_drivers[hid];
+       mutex_enter(&cap->cc_lock);
+       return cap;
 }
 
 /*
@@ -693,12 +716,56 @@
 crypto_checkdriver_uninit(u_int32_t hid)
 {
 
+       KASSERT(mutex_owned(&crypto_drv_mtx));
+
        if (crypto_drivers == NULL)
                return NULL;
 
        return (hid >= crypto_drivers_num ? NULL : &crypto_drivers[hid]);
 }
 
+static inline void
+crypto_driver_lock(struct cryptocap *cap)
+{
+
+       KASSERT(cap != NULL);
+
+       mutex_enter(&cap->cc_lock);
+}
+
+static inline void
+crypto_driver_unlock(struct cryptocap *cap)
+{
+
+       KASSERT(cap != NULL);
+
+       mutex_exit(&cap->cc_lock);
+}
+
+static void
+crypto_driver_clear(struct cryptocap *cap)
+{
+
+       if (cap == NULL)
+               return;
+
+       KASSERT(mutex_owned(&cap->cc_lock));
+
+       cap->cc_sessions = 0;
+       memset(&cap->cc_max_op_len, 0, sizeof(cap->cc_max_op_len));
+       memset(&cap->cc_alg, 0, sizeof(cap->cc_alg));
+       memset(&cap->cc_kalg, 0, sizeof(cap->cc_kalg));
+       cap->cc_flags = 0;
+       cap->cc_qblocked = 0;
+       cap->cc_kqblocked = 0;
+
+       cap->cc_arg = NULL;
+       cap->cc_newsession = NULL;
+       cap->cc_process = NULL;
+       cap->cc_freesession = NULL;
+       cap->cc_kprocess = NULL;
+}
+
 /*
  * Register support for a key-related algorithm.  This routine
  * is called once for each algorithm supported a driver.
@@ -713,7 +780,7 @@
 
        mutex_enter(&crypto_drv_mtx);
 
-       cap = crypto_checkdriver(driverid);
+       cap = crypto_checkdriver_lock(driverid);
        if (cap != NULL &&
            (CRK_ALGORITM_MIN <= kalg && kalg <= CRK_ALGORITHM_MAX)) {
                /*
@@ -759,12 +826,12 @@
        struct cryptocap *cap;
        int err;
 
-       mutex_enter(&crypto_drv_mtx);
+       cap = crypto_checkdriver_lock(driverid);
+       if (cap == NULL)
+               return EINVAL;
 
-       cap = crypto_checkdriver(driverid);
        /* NB: algorithms are in the range [1..max] */
-       if (cap != NULL &&
-           (CRYPTO_ALGORITHM_MIN <= alg && alg <= CRYPTO_ALGORITHM_MAX)) {
+       if (CRYPTO_ALGORITHM_MIN <= alg && alg <= CRYPTO_ALGORITHM_MAX) {
                /*
                 * XXX Do some performance testing to determine placing.
                 * XXX We probably need an auxiliary data structure that
@@ -794,25 +861,25 @@
        } else
                err = EINVAL;
 
-       mutex_exit(&crypto_drv_mtx);
+       crypto_driver_unlock(cap);
+
        return err;
 }
 
 static int
-crypto_unregister_locked(u_int32_t driverid, int alg, bool all)
+crypto_unregister_locked(struct cryptocap *cap, int alg, bool all)
 {
        int i;
        u_int32_t ses;
-       struct cryptocap *cap;
        bool lastalg = true;
 
-       KASSERT(mutex_owned(&crypto_drv_mtx));
+       KASSERT(cap != NULL);
+       KASSERT(mutex_owned(&cap->cc_lock));
 
        if (alg < CRYPTO_ALGORITHM_MIN || CRYPTO_ALGORITHM_MAX < alg)
                return EINVAL;
 
-       cap = crypto_checkdriver(driverid);
-       if (cap == NULL || (!all && cap->cc_alg[alg] == 0))
+       if (!all && cap->cc_alg[alg] == 0)
                return EINVAL;
 
        cap->cc_alg[alg] = 0;
@@ -831,7 +898,7 @@
        }
        if (lastalg) {
                ses = cap->cc_sessions;
-               memset(cap, 0, sizeof(struct cryptocap));
+               crypto_driver_clear(cap);



Home | Main Index | Thread Index | Old Index