Module Name: src
Committed By: riastradh
Date: Thu May 19 20:51:46 UTC 2022
Modified Files:
src/sys/opencrypto: crypto.c cryptodev.c cryptodev.h
Log Message:
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.
To generate a diff of this commit:
cvs rdiff -u -r1.117 -r1.118 src/sys/opencrypto/crypto.c
cvs rdiff -u -r1.112 -r1.113 src/sys/opencrypto/cryptodev.c
cvs rdiff -u -r1.42 -r1.43 src/sys/opencrypto/cryptodev.h
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.117 src/sys/opencrypto/crypto.c:1.118
--- src/sys/opencrypto/crypto.c:1.117 Tue May 17 10:32:58 2022
+++ src/sys/opencrypto/crypto.c Thu May 19 20:51:46 2022
@@ -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 @@ crypto_kgetreq(int num __unused, int prf
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 @@ crypto_done(struct cryptop *crp)
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 @@ crypto_done(struct cryptop *crp)
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;
+ krp->krp_flags |= CRYPTO_F_DONE;
- 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
Index: src/sys/opencrypto/cryptodev.c
diff -u src/sys/opencrypto/cryptodev.c:1.112 src/sys/opencrypto/cryptodev.c:1.113
--- src/sys/opencrypto/cryptodev.c:1.112 Wed May 18 20:03:58 2022
+++ src/sys/opencrypto/cryptodev.c Thu May 19 20:51:46 2022
@@ -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 @@ cryptodev_op(struct csession *cse, struc
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 @@ cryptodev_mop(struct fcrypt *fcr,
}
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 @@ cryptodev_mkey(struct fcrypt *fcr, struc
(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 =
Index: src/sys/opencrypto/cryptodev.h
diff -u src/sys/opencrypto/cryptodev.h:1.42 src/sys/opencrypto/cryptodev.h:1.43
--- src/sys/opencrypto/cryptodev.h:1.42 Sat Aug 14 20:43:05 2021
+++ src/sys/opencrypto/cryptodev.h Thu May 19 20:51:46 2022
@@ -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 @@ struct cryptop {
#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. */
