Source-Changes-HG archive

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

[src/trunk]: src/sys/opencrypto opencrypto: Nix CRYPTO_F_USER, CRYPTO_F_CBIMM...



details:   https://anonhg.NetBSD.org/src/rev/30586b901acf
branches:  trunk
changeset: 366240:30586b901acf
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Thu May 19 20:51:46 2022 +0000

description:
opencrypto: Nix CRYPTO_F_USER, CRYPTO_F_CBIMM, CRYPTO_F_CBIFSYNC.

CRYPTO_F_USER is no longer needed.  It was introduced in 2008 by
darran@ in crypto.c 1.30, cryptodev.c 1.45 in an attempt to avoid
double-free between the issuing thread and asynchronous callback.
But the `fix' didn't work.  In 2017, knakahara@ fixed it properly in
cryptodev.c 1.87 by distinguishing `the crypto operation has
completed' (CRYPTO_F_DONE) from `the callback is done touching the
crp object' (CRYPTO_F_DQRETQ, now renamed to CRYPTODEV_F_RET).

CRYPTO_F_CBIMM formerly served to invoke the callback synchronously
from the driver's interrupt completion routine, to reduce contention
on what was once a single cryptoret thread.  Now, there is a per-CPU
queue and softint for much cheaper processing, so there is less
motivation for this in the first place.  So let's remove the
complicated logic.  This means the callbacks never run in hard
interrupt context, which means we don't need to worry about recursion
into crypto_dispatch in hard interrupt context.

diffstat:

 sys/opencrypto/crypto.c    |  119 +++++++++++---------------------------------
 sys/opencrypto/cryptodev.c |   16 +----
 sys/opencrypto/cryptodev.h |    8 +-
 3 files changed, 39 insertions(+), 104 deletions(-)

diffs (238 lines):

diff -r 86883fdff0fb -r 30586b901acf sys/opencrypto/crypto.c
--- a/sys/opencrypto/crypto.c   Thu May 19 20:09:46 2022 +0000
+++ b/sys/opencrypto/crypto.c   Thu May 19 20:51:46 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: crypto.c,v 1.117 2022/05/17 10:32:58 riastradh Exp $ */
+/*     $NetBSD: crypto.c,v 1.118 2022/05/19 20:51:46 riastradh 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.117 2022/05/17 10:32:58 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.118 2022/05/19 20:51:46 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/reboot.h>
@@ -1707,6 +1707,9 @@
 void
 crypto_done(struct cryptop *crp)
 {
+       int wasempty;
+       struct crypto_crp_ret_qs *qs;
+       struct crypto_crp_ret_q *crp_ret_q;
 
        KASSERT(crp != NULL);
 
@@ -1720,70 +1723,19 @@
 
        crp->crp_flags |= CRYPTO_F_DONE;
 
-       /*
-        * Normal case; queue the callback for the thread.
-        *
-        * The return queue is manipulated by the swi thread
-        * and, potentially, by crypto device drivers calling
-        * back to mark operations completed.  Thus we need
-        * to mask both while manipulating the return queue.
-        */
-       if (crp->crp_flags & CRYPTO_F_CBIMM) {
-               /*
-               * Do the callback directly.  This is ok when the
-               * callback routine does very little (e.g. the
-               * /dev/crypto callback method just does a wakeup).
-               */
-#ifdef CRYPTO_TIMING
-               if (crypto_timing) {
-                       /*
-                       * NB: We must copy the timestamp before
-                       * doing the callback as the cryptop is
-                       * likely to be reclaimed.
-                       */
-                       struct timespec t = crp->crp_tstamp;
-                       crypto_tstat(&cryptostats.cs_cb, &t);
-                       crp->crp_callback(crp);
-                       crypto_tstat(&cryptostats.cs_finis, &t);
-               } else
-#endif
-               crp->crp_callback(crp);
-       } else {
-#if 0
-               if (crp->crp_flags & CRYPTO_F_USER) {
-                       /*
-                        * 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 softint_schedule(crypto_ret_si).
-                        */
-                       DPRINTF("lid[%u]: crp %p CRYPTO_F_USER\n",
-                               CRYPTO_SESID2LID(crp->crp_sid), crp);
-               } else
-#endif
-               {
-                       int wasempty;
-                       struct crypto_crp_ret_qs *qs;
-                       struct crypto_crp_ret_q *crp_ret_q;
-
-                       qs = crypto_get_crp_ret_qs(crp->reqcpu);
-                       crp_ret_q = &qs->crp_ret_q;
-                       wasempty = TAILQ_EMPTY(crp_ret_q);
-                       DPRINTF("lid[%u]: queueing %p\n",
-                               CRYPTO_SESID2LID(crp->crp_sid), crp);
-                       crp->crp_flags |= CRYPTO_F_ONRETQ;
-                       TAILQ_INSERT_TAIL(crp_ret_q, crp, crp_next);
-                       qs->crp_ret_q_len++;
-                       if (wasempty && !qs->crp_ret_q_exit_flag) {
-                               DPRINTF("lid[%u]: waking cryptoret,"
-                                       "crp %p hit empty queue\n.",
-                                       CRYPTO_SESID2LID(crp->crp_sid), crp);
-                               softint_schedule_cpu(crypto_ret_si, crp->reqcpu);
-                       }
-                       crypto_put_crp_ret_qs(crp->reqcpu);
-               }
+       qs = crypto_get_crp_ret_qs(crp->reqcpu);
+       crp_ret_q = &qs->crp_ret_q;
+       wasempty = TAILQ_EMPTY(crp_ret_q);
+       DPRINTF("lid[%u]: queueing %p\n", CRYPTO_SESID2LID(crp->crp_sid), crp);
+       crp->crp_flags |= CRYPTO_F_ONRETQ;
+       TAILQ_INSERT_TAIL(crp_ret_q, crp, crp_next);
+       qs->crp_ret_q_len++;
+       if (wasempty && !qs->crp_ret_q_exit_flag) {
+               DPRINTF("lid[%u]: waking cryptoret, crp %p hit empty queue\n.",
+                   CRYPTO_SESID2LID(crp->crp_sid), crp);
+               softint_schedule_cpu(crypto_ret_si, crp->reqcpu);
        }
+       crypto_put_crp_ret_qs(crp->reqcpu);
 }
 
 /*
@@ -1792,38 +1744,27 @@
 void
 crypto_kdone(struct cryptkop *krp)
 {
+       int wasempty;
+       struct crypto_crp_ret_qs *qs;
+       struct crypto_crp_ret_kq *crp_ret_kq;
 
        KASSERT(krp != NULL);
 
        if (krp->krp_status != 0)
                cryptostats.cs_kerrs++;
-               
+
        krp->krp_flags |= CRYPTO_F_DONE;
 
-       /*
-        * The return queue is manipulated by the swi thread
-        * and, potentially, by crypto device drivers calling
-        * back to mark operations completed.  Thus we need
-        * to mask both while manipulating the return queue.
-        */
-       if (krp->krp_flags & CRYPTO_F_CBIMM) {
-               krp->krp_callback(krp);
-       } else {
-               int wasempty;
-               struct crypto_crp_ret_qs *qs;
-               struct crypto_crp_ret_kq *crp_ret_kq;
+       qs = crypto_get_crp_ret_qs(krp->reqcpu);
+       crp_ret_kq = &qs->crp_ret_kq;
 
-               qs = crypto_get_crp_ret_qs(krp->reqcpu);
-               crp_ret_kq = &qs->crp_ret_kq;
-
-               wasempty = TAILQ_EMPTY(crp_ret_kq);
-               krp->krp_flags |= CRYPTO_F_ONRETQ;
-               TAILQ_INSERT_TAIL(crp_ret_kq, krp, krp_next);
-               qs->crp_ret_kq_len++;
-               if (wasempty && !qs->crp_ret_q_exit_flag)
-                       softint_schedule_cpu(crypto_ret_si, krp->reqcpu);
-               crypto_put_crp_ret_qs(krp->reqcpu);
-       }
+       wasempty = TAILQ_EMPTY(crp_ret_kq);
+       krp->krp_flags |= CRYPTO_F_ONRETQ;
+       TAILQ_INSERT_TAIL(crp_ret_kq, krp, krp_next);
+       qs->crp_ret_kq_len++;
+       if (wasempty && !qs->crp_ret_q_exit_flag)
+               softint_schedule_cpu(crypto_ret_si, krp->reqcpu);
+       crypto_put_crp_ret_qs(krp->reqcpu);
 }
 
 int
diff -r 86883fdff0fb -r 30586b901acf sys/opencrypto/cryptodev.c
--- a/sys/opencrypto/cryptodev.c        Thu May 19 20:09:46 2022 +0000
+++ b/sys/opencrypto/cryptodev.c        Thu May 19 20:51:46 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: cryptodev.c,v 1.112 2022/05/18 20:03:58 riastradh Exp $ */
+/*     $NetBSD: cryptodev.c,v 1.113 2022/05/19 20:51:46 riastradh 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.112 2022/05/18 20:03:58 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.113 2022/05/19 20:51:46 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -604,13 +604,7 @@
 
 
        crp->crp_ilen = cop->len;
-       /*
-        * 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;
+       crp->crp_flags = CRYPTO_F_IOV | (cop->flags & COP_F_BATCH) | flags;
        crp->crp_buf = (void *)&cse->uio;
        crp->crp_callback = cryptodev_cb;
        crp->crp_sid = cse->sid;
@@ -1268,7 +1262,7 @@
                }
 
                crp->crp_ilen = cnop[req].len;
-               crp->crp_flags = CRYPTO_F_IOV | CRYPTO_F_CBIMM |
+               crp->crp_flags = CRYPTO_F_IOV |
                    (cnop[req].flags & COP_F_BATCH) | flags;
                crp->crp_buf = (void *)&crp->uio;
                crp->crp_callback = cryptodev_mcb;
@@ -1451,7 +1445,7 @@
                (void)memcpy(krp->crk_param, kop[req].crk_param,
                    sizeof(kop[req].crk_param));
 
-               krp->krp_flags = CRYPTO_F_CBIMM;
+               krp->krp_flags = 0;
 
                for (i = 0; i < CRK_MAXPARAM; i++)
                        krp->krp_param[i].crp_nbits =
diff -r 86883fdff0fb -r 30586b901acf sys/opencrypto/cryptodev.h
--- a/sys/opencrypto/cryptodev.h        Thu May 19 20:09:46 2022 +0000
+++ b/sys/opencrypto/cryptodev.h        Thu May 19 20:51:46 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: cryptodev.h,v 1.42 2021/08/14 20:43:05 andvar Exp $ */
+/*     $NetBSD: cryptodev.h,v 1.43 2022/05/19 20:51:46 riastradh Exp $ */
 /*     $FreeBSD: src/sys/opencrypto/cryptodev.h,v 1.2.2.6 2003/07/02 17:04:50 sam Exp $        */
 /*     $OpenBSD: cryptodev.h,v 1.33 2002/07/17 23:52:39 art Exp $      */
 
@@ -469,11 +469,11 @@
 #define CRYPTO_F_IOV           0x0002  /* Input/output are uio */
 #define CRYPTO_F_REL           0x0004  /* Must return data in same place */
 #define        CRYPTO_F_BATCH          0x0008  /* Batch op if possible possible */
-#define        CRYPTO_F_CBIMM          0x0010  /* Do callback immediately */
+#define        CRYPTO_F_UNUSED0        0x0010  /* was CRYPTO_F_CBIMM */
 #define        CRYPTO_F_DONE           0x0020  /* Operation completed */
-#define        CRYPTO_F_CBIFSYNC       0x0040  /* Do CBIMM if op is synchronous */
+#define        CRYPTO_F_UNUSED1        0x0040  /* was CRYPTO_F_CBIFSYNC */
 #define        CRYPTO_F_ONRETQ         0x0080  /* Request is on return queue */
-#define        CRYPTO_F_USER           0x0100  /* Request is in user context */
+#define        CRYPTO_F_UNUSED2        0x0100  /* was CRYPTO_F_USER */
 #define        CRYPTO_F_MORE           0x0200  /* more data to follow */
 
        int             crp_devflags;   /* other than cryptodev.c must not use. */



Home | Main Index | Thread Index | Old Index