From 59832146393fb3c0b311f99b6586c9c404a0ef51 Mon Sep 17 00:00:00 2001 From: k-nakahara Date: Fri, 2 Jun 2017 20:10:49 +0900 Subject: [PATCH 2/3] remove migrate process, but panic because call localcount_acquire after localcount_drain --- sys/opencrypto/crypto.c | 134 ++++++++++++++---------------------------------- 1 file changed, 38 insertions(+), 96 deletions(-) diff --git a/sys/opencrypto/crypto.c b/sys/opencrypto/crypto.c index dde7707a07d..0a6842b1e56 100644 --- a/sys/opencrypto/crypto.c +++ b/sys/opencrypto/crypto.c @@ -527,8 +527,7 @@ crypto_newsession(u_int64_t *sid, struct cryptoini *cri, int hard) * If it's not initialized or has remaining sessions * referencing it, skip. */ - if (cap->cc_newsession == NULL || - (cap->cc_flags & CRYPTOCAP_F_CLEANUP)) { + if (cap->cc_newsession == NULL) { crypto_driver_unref(cap); continue; } @@ -616,13 +615,6 @@ crypto_freesession(u_int64_t sid) /* release reference which acquired by crypto_newsession() */ crypto_driver_unref(cap); - /* - * If this was the last session of a driver marked as invalid, - * make the entry available for reuse. - */ - if ((cap->cc_flags & CRYPTOCAP_F_CLEANUP) && cap->cc_sessions == 0) - memset(cap, 0, sizeof(struct cryptocap)); - done: mutex_exit(&crypto_drv_mtx); return err; @@ -646,9 +638,7 @@ crypto_get_driverid(u_int32_t flags) cap = crypto_checkdriver(i); if (cap == NULL) continue; - if (cap->cc_process == NULL && - (cap->cc_flags & CRYPTOCAP_F_CLEANUP) == 0 && - cap->cc_sessions == 0) + if (cap->cc_process == NULL && cap->cc_sessions == 0) break; } @@ -876,7 +866,6 @@ static int crypto_unregister_locked(u_int32_t driverid, int alg, bool all) { int i; - u_int32_t ses; struct cryptocap *cap; bool lastalg = true; @@ -917,16 +906,9 @@ crypto_unregister_locked(u_int32_t driverid, int alg, bool all) mutex_exit(&cap->cc_lock); localcount_fini(&cap->cc_ref); mutex_destroy(&cap->cc_lock); - - ses = cap->cc_sessions; + KASSERT(cap->cc_sessions == 0); memset(cap, 0, sizeof(struct cryptocap)); - if (ses != 0) { - /* - * If there are pending sessions, just mark as invalid. - */ - cap->cc_flags |= CRYPTOCAP_F_CLEANUP; - cap->cc_sessions = ses; - } + mutex_enter(&crypto_drv_mtx); /* XXX */ } @@ -1045,30 +1027,16 @@ crypto_dispatch(struct cryptop *crp) return 0; } - mutex_spin_enter(&crypto_q_mtx); - cap = crypto_checkdriver_ref(CRYPTO_SESID2HID(crp->crp_sid)); - /* - * TODO: - * If we can ensure the driver has been valid until the driver is - * done crypto_unregister(), this migrate operation is not required. - */ - if (cap == NULL) { - /* - * The driver must be detached, so this request will migrate - * to other drivers in cryptointr() later. - */ - TAILQ_INSERT_TAIL(&crp_q, crp, crp_next); - mutex_spin_exit(&crypto_q_mtx); - - return 0; - } + KASSERTMSG(cap != NULL, "crypto_driverid[%lu] is free unexpectedly.", + CRYPTO_SESID2HID(crp->crp_sid)); /* * TODO: * cap->cc_qblocked should be protected by a spin lock other than * crypto_q_mtx. */ + mutex_spin_enter(&crypto_q_mtx); if (cap->cc_qblocked != 0) { /* * The driver is blocked, just queue the op until @@ -1135,9 +1103,9 @@ crypto_kdispatch(struct cryptkop *krp) cap = crypto_checkdriver_ref(krp->krp_hid); /* - * TODO: - * If we can ensure the driver has been valid until the driver is - * done crypto_unregister(), this migrate operation is not required. + * XXX + * The "krp_hid" of asymetric crypto request can be uninitialized when + * called by cryptodev.c. So, this return value can be NULL... */ if (cap == NULL) { TAILQ_INSERT_TAIL(&crp_kq, krp, krp_next); @@ -1302,39 +1270,14 @@ crypto_invoke(struct cryptop *crp, int hint) /* divert reference got in crypto_dispatch() */ cap = crypto_checkdriver(CRYPTO_SESID2HID(crp->crp_sid)); - if (cap != NULL && (cap->cc_flags & CRYPTOCAP_F_CLEANUP) == 0) { - int (*process)(void *, struct cryptop *, int); - void *arg; + KASSERTMSG(cap != NULL, "crypto_driverid[%lu] is free unexpectedly.", + CRYPTO_SESID2HID(crp->crp_sid)); - process = cap->cc_process; - arg = cap->cc_arg; - - /* - * Invoke the driver to process the request. - */ - DPRINTF("calling process for %p\n", crp); - return (*process)(arg, crp, hint); - } else { - struct cryptodesc *crd; - u_int64_t nid = 0; - - /* - * Driver has unregistered; migrate the session and return - * an error to the caller so they'll resubmit the op. - */ - crypto_freesession(crp->crp_sid); - - for (crd = crp->crp_desc; crd->crd_next; crd = crd->crd_next) - crd->CRD_INI.cri_next = &(crd->crd_next->CRD_INI); - - if (crypto_newsession(&nid, &(crp->crp_desc->CRD_INI), 0) == 0) - crp->crp_sid = nid; - - crp->crp_etype = EAGAIN; - - crypto_done(crp); - return 0; - } + /* + * Invoke the driver to process the request. + */ + DPRINTF("calling process for %p\n", crp); + return (*cap->cc_process)(cap->cc_arg, crp, hint); } /* @@ -1624,12 +1567,9 @@ cryptointr(void) TAILQ_FOREACH_SAFE(crp, &crp_q, crp_next, cnext) { u_int32_t hid = CRYPTO_SESID2HID(crp->crp_sid); cap = crypto_checkdriver(hid); - /* divert ref got in crypto_dispatch() */ - if (cap == NULL || cap->cc_process == NULL) { - /* Op needs to be migrated, process it. */ - submit = crp; - break; - } + KASSERTMSG(cap != NULL, + "crypto_driverid[%u] is free unexpectedly.", + hid); /* * skip blocked crp regardless of CRYPTO_F_BATCH @@ -1682,14 +1622,12 @@ cryptointr(void) /* validate sid again */ /* divert ref got in crypto_dispatch() */ cap = crypto_checkdriver(CRYPTO_SESID2HID(submit->crp_sid)); - if (cap == NULL) { - /* migrate again, sigh... */ - TAILQ_INSERT_TAIL(&crp_q, submit, crp_next); - } else { - cap->cc_qblocked = 1; - TAILQ_INSERT_HEAD(&crp_q, submit, crp_next); - cryptostats.cs_blocks++; - } + KASSERTMSG(cap != NULL, + "crypto_driverid[%lu] is free unexpectedly.", + CRYPTO_SESID2HID(crp->crp_sid)); + cap->cc_qblocked = 1; + TAILQ_INSERT_HEAD(&crp_q, submit, crp_next); + cryptostats.cs_blocks++; } } @@ -1697,6 +1635,12 @@ cryptointr(void) TAILQ_FOREACH_SAFE(krp, &crp_kq, krp_next, knext) { /* divert ref got in crypto_kdispatch() */ cap = crypto_checkdriver(krp->krp_hid); + /* + * XXX + * The "krp_hid" of asymetric crypto request can be + * uninitialized when called by cryptodev.c. So, this + * return value can be NULL... + */ if (cap == NULL || cap->cc_kprocess == NULL) { /* Op needs to be migrated, process it. */ break; @@ -1723,14 +1667,12 @@ cryptointr(void) /* validate sid again */ /* divert reference got in crypto_dispatch() */ cap = crypto_checkdriver(krp->krp_hid); - if (cap == NULL) { - /* migrate again, sigh... */ - TAILQ_INSERT_TAIL(&crp_kq, krp, krp_next); - } else { - cap->cc_kqblocked = 1; - TAILQ_INSERT_HEAD(&crp_kq, krp, krp_next); - cryptostats.cs_kblocks++; - } + KASSERTMSG(cap != NULL, + "crypto_driverid[%lu] is free unexpectedly.", + CRYPTO_SESID2HID(crp->crp_sid)); + cap->cc_kqblocked = 1; + TAILQ_INSERT_HEAD(&crp_kq, krp, krp_next); + cryptostats.cs_kblocks++; } } } while (submit != NULL || krp != NULL); -- 2.11.0