From d536b4086d659f000514219f614113f8dd2b9604 Mon Sep 17 00:00:00 2001 From: Alex Hornung Date: Wed, 13 Jul 2011 21:41:43 +0100 Subject: [PATCH] dm_target_crypt - use per-instance mpipes, KTR * Add KTR support (KTR_DMCRYPT) to monitor the read and write strategy paths. * Make the mpipes for reads and writes per-instance instead of for all targets. This solves the issue where two instances are interdependent and one starves the other, which will cause a deadlock. This way each instance is guaranteed to have some buffers. * Change the maximum size of the mpipes from 0.5% of physical memory to 0.2%, as this limit is now per-instance and an average of 2 or 3 crypt targets sounds reasonable. --- sys/conf/options | 1 + sys/config/LINT | 1 + sys/config/LINT64 | 1 + sys/dev/disk/dm/targets/crypt/Makefile | 2 +- .../disk/dm/targets/crypt/dm_target_crypt.c | 100 ++++++++++++++---- 5 files changed, 82 insertions(+), 23 deletions(-) diff --git a/sys/conf/options b/sys/conf/options index c64e46f9af..00023ee966 100644 --- a/sys/conf/options +++ b/sys/conf/options @@ -599,6 +599,7 @@ KTR_IF_START opt_ktr.h KTR_HAMMER opt_ktr.h KTR_ETHERNET opt_ktr.h KTR_CTXSW opt_ktr.h +KTR_DMCRYPT opt_ktr.h # options for the Atheros driver ATH_DEBUG opt_ath.h diff --git a/sys/config/LINT b/sys/config/LINT index 08ec535a8c..91ffb33c61 100644 --- a/sys/config/LINT +++ b/sys/config/LINT @@ -2842,6 +2842,7 @@ options KTR #options KTR_TESTLOG #options KTR_TOKENS #options KTR_USB_MEMORY +#options KTR_DMCRYPT options KTR_ALL options KTR_ENTRIES=1024 options KTR_VERBOSE=1 diff --git a/sys/config/LINT64 b/sys/config/LINT64 index 24dc2283ca..1f24cd09b3 100644 --- a/sys/config/LINT64 +++ b/sys/config/LINT64 @@ -2554,6 +2554,7 @@ options KTR #options KTR_TESTLOG #options KTR_TOKENS #options KTR_USB_MEMORY +#options KTR_DMCRYPT options KTR_ALL options KTR_ENTRIES=1024 options KTR_VERBOSE=1 diff --git a/sys/dev/disk/dm/targets/crypt/Makefile b/sys/dev/disk/dm/targets/crypt/Makefile index 41b60f12fa..a8a241bdf8 100644 --- a/sys/dev/disk/dm/targets/crypt/Makefile +++ b/sys/dev/disk/dm/targets/crypt/Makefile @@ -1,6 +1,6 @@ KMOD= dm_target_crypt -SRCS+= bus_if.h device_if.h +SRCS+= bus_if.h device_if.h opt_ktr.h SRCS+= dm_target_crypt.c .include diff --git a/sys/dev/disk/dm/targets/crypt/dm_target_crypt.c b/sys/dev/disk/dm/targets/crypt/dm_target_crypt.c index e8e5f5d379..4035d2fe16 100644 --- a/sys/dev/disk/dm/targets/crypt/dm_target_crypt.c +++ b/sys/dev/disk/dm/targets/crypt/dm_target_crypt.c @@ -54,9 +54,40 @@ #include #include +#include + #include MALLOC_DEFINE(M_DMCRYPT, "dm_crypt", "Device Mapper Target Crypt"); +KTR_INFO_MASTER(dmcrypt); + +#if !defined(KTR_DMCRYPT) +#define KTR_DMCRYPT KTR_ALL +#endif + +KTR_INFO(KTR_DMCRYPT, dmcrypt, crypto_dispatch, 0, + "crypto_dispatch(%p)", sizeof(void *)); +KTR_INFO(KTR_DMCRYPT, dmcrypt, crypt_strategy, 0, + "crypt_strategy(b_cmd = %d, bp = %p)", sizeof(int) + sizeof(void *)); +KTR_INFO(KTR_DMCRYPT, dmcrypt, crypto_write_start, 1, + "crypto_write_start(crp = %p, bp = %p, sector = %d/%d)", + sizeof(void *) + sizeof(void *) + sizeof(int) + sizeof(int)); +KTR_INFO(KTR_DMCRYPT, dmcrypt, crypto_cb_write_done, 1, + "crypto_cb_write_done(crp = %p, bp = %p, n = %d)", + sizeof(void *) + sizeof(void *) + sizeof(int)); +KTR_INFO(KTR_DMCRYPT, dmcrypt, bio_write_done, 1, + "bio_write_done(bp = %p)", sizeof(void *)); +KTR_INFO(KTR_DMCRYPT, dmcrypt, crypto_write_retry, 1, + "crypto_write_retry(crp = %p)", sizeof(void *)); +KTR_INFO(KTR_DMCRYPT, dmcrypt, bio_read_done, 2, + "bio_read_done(bp = %p)", sizeof(void *)); +KTR_INFO(KTR_DMCRYPT, dmcrypt, crypto_read_start, 2, + "crypto_read_start(crp = %p, bp = %p, sector = %d/%d)", + sizeof(void *) + sizeof(void *) + sizeof(int) + sizeof(int)); +KTR_INFO(KTR_DMCRYPT, dmcrypt, crypto_cb_read_done, 2, + "crypto_cb_read_done(crp = %p, bp = %p, n = %d)", + sizeof(void *) + sizeof(void *) + sizeof(int)); + struct target_crypt_config; typedef void dispatch_t(void *); @@ -98,9 +129,13 @@ typedef struct target_crypt_config { struct iv_generator *ivgen; void *ivgen_priv; + + struct malloc_pipe read_mpipe; + struct malloc_pipe write_mpipe; } dm_target_crypt_config_t; struct dmtc_helper { + dm_target_crypt_config_t *priv; caddr_t free_addr; caddr_t orig_buf; caddr_t data_buf; @@ -158,32 +193,29 @@ struct objcache_malloc_args essiv_ivgen_malloc_args = { 2*sizeof(void *) + (sizeof(struct cryptodesc) + sizeof(struct cryptop)), M_DMCRYPT }; -static struct malloc_pipe dmtc_read_mpipe; -static struct malloc_pipe dmtc_write_mpipe; - static void -dmtc_init_mpipe(void) +dmtc_init_mpipe(struct target_crypt_config *priv) { int nmax; - nmax = (physmem*5/1000*PAGE_SIZE)/(DMTC_BUF_SIZE_WRITE + DMTC_BUF_SIZE_READ) + 1; + nmax = (physmem*2/1000*PAGE_SIZE)/(DMTC_BUF_SIZE_WRITE + DMTC_BUF_SIZE_READ) + 1; if (nmax < 2) nmax = 2; kprintf("dm_target_crypt: Setting min/max mpipe buffers: %d/%d\n", 2, nmax); - mpipe_init(&dmtc_write_mpipe, M_DMCRYPT, DMTC_BUF_SIZE_WRITE, + mpipe_init(&priv->write_mpipe, M_DMCRYPT, DMTC_BUF_SIZE_WRITE, 2, nmax, MPF_NOZERO | MPF_CALLBACK, NULL, NULL, NULL); - mpipe_init(&dmtc_read_mpipe, M_DMCRYPT, DMTC_BUF_SIZE_READ, + mpipe_init(&priv->read_mpipe, M_DMCRYPT, DMTC_BUF_SIZE_READ, 2, nmax, MPF_NOZERO | MPF_CALLBACK, NULL, NULL, NULL); } static void -dmtc_destroy_mpipe(void) +dmtc_destroy_mpipe(struct target_crypt_config *priv) { - mpipe_done(&dmtc_write_mpipe); - mpipe_done(&dmtc_read_mpipe); + mpipe_done(&priv->write_mpipe); + mpipe_done(&priv->read_mpipe); } /* @@ -713,6 +745,9 @@ dm_target_crypt_init(dm_dev_t * dmv, void **target_config, char *params) } priv->status_str = status_str; + /* Initialize mpipes */ + dmtc_init_mpipe(priv); + return 0; notsup: @@ -771,6 +806,9 @@ dm_target_crypt_destroy(dm_table_entry_t * table_en) priv->ivgen->dtor(priv, priv->ivgen_priv); } + /* Destroy mpipes */ + dmtc_destroy_mpipe(priv); + dmtc_crypto_clear(priv, sizeof(dm_target_crypt_config_t)); kfree(priv, M_DMCRYPT); @@ -824,9 +862,10 @@ dmtc_crypto_dispatch(void *arg) crp = (struct cryptop *)arg; KKASSERT(crp != NULL); + KTR_LOG(dmcrypt_crypto_dispatch, crp); crypto_dispatch(crp); } - + /* * Start IO operation, called from dmstrategy routine. */ @@ -851,6 +890,8 @@ dm_target_crypt_strategy(dm_table_entry_t *table_en, struct buf *bp) } } + KTR_LOG(dmcrypt_crypt_strategy, bp->b_cmd, bp); + switch (bp->b_cmd) { case BUF_CMD_READ: bio = push_bio(&bp->b_bio1); @@ -884,6 +925,8 @@ dmtc_bio_read_done(struct bio *bio) dm_target_crypt_config_t *priv; + KTR_LOG(dmcrypt_bio_read_done, bio->bio_buf); + /* * If a read error occurs we shortcut the operation, otherwise * go on to stage 2. @@ -939,7 +982,7 @@ dmtc_crypto_read_start(dm_target_crypt_config_t *priv, struct bio *bio) * read already completed and threw crypted data into the buffer * cache buffer. Disable for now. */ - space = mpipe_alloc_callback(&dmtc_read_mpipe, + space = mpipe_alloc_callback(&priv->read_mpipe, dmtc_crypto_read_retry, priv, bio); if (space == NULL) return; @@ -949,6 +992,7 @@ dmtc_crypto_read_start(dm_target_crypt_config_t *priv, struct bio *bio) space += sizeof(struct dmtc_helper); dmtc->orig_buf = NULL; dmtc->data_buf = bio->bio_buf->b_data; + dmtc->priv = priv; bio->bio_caller_info2.ptr = dmtc; bio->bio_buf->b_error = 0; @@ -996,6 +1040,9 @@ dmtc_crypto_read_start(dm_target_crypt_config_t *priv, struct bio *bio) crd->crd_flags &= ~CRD_F_ENCRYPT; + KTR_LOG(dmcrypt_crypto_read_start, crp, bio->bio_buf, i, + sectors); + /* * Note: last argument is used to generate salt(?) and is * a 64 bit value, but the original code passed an @@ -1042,6 +1089,8 @@ dmtc_crypto_cb_read_done(struct cryptop *crp) kprintf("dmtc_crypto_cb_read_done %p, n = %d\n", bio, n); #endif + KTR_LOG(dmcrypt_crypto_cb_read_done, crp, bio->bio_buf, n); + if (n == 1) { /* * For the B_HASBOGUS case we didn't decrypt in place, @@ -1059,7 +1108,7 @@ dmtc_crypto_cb_read_done(struct cryptop *crp) bio->bio_buf->b_bcount); } #endif - mpipe_free(&dmtc_read_mpipe, dmtc->free_addr); + mpipe_free(&dmtc->priv->read_mpipe, dmtc->free_addr); obio = pop_bio(bio); biodone(obio); } @@ -1077,6 +1126,8 @@ dmtc_crypto_write_retry(void *arg1, void *arg2) dm_target_crypt_config_t *priv = arg1; struct bio *bio = arg2; + KTR_LOG(dmcrypt_crypto_write_retry, bio->bio_buf); + dmtc_crypto_write_start(priv, bio); } @@ -1105,7 +1156,7 @@ dmtc_crypto_write_start(dm_target_crypt_config_t *priv, struct bio *bio) /* * For writes and reads with bogus page don't decrypt in place. */ - space = mpipe_alloc_callback(&dmtc_write_mpipe, + space = mpipe_alloc_callback(&priv->write_mpipe, dmtc_crypto_write_retry, priv, bio); if (space == NULL) return; @@ -1120,6 +1171,7 @@ dmtc_crypto_write_start(dm_target_crypt_config_t *priv, struct bio *bio) dmtc->orig_buf = bio->bio_buf->b_data; dmtc->data_buf = space + sz; + dmtc->priv = priv; /* * Load crypto descriptors (crp/crd loop) @@ -1171,6 +1223,10 @@ dmtc_crypto_write_start(dm_target_crypt_config_t *priv, struct bio *bio) * int. Changing it now will break pre-existing * crypt volumes. */ + + KTR_LOG(dmcrypt_crypto_write_start, crp, bio->bio_buf, + i, sectors); + priv->ivgen->gen_iv(priv, crd->crd_iv, sizeof(crd->crd_iv), isector + i, crp); } @@ -1211,13 +1267,15 @@ dmtc_crypto_cb_write_done(struct cryptop *crp) kprintf("dmtc_crypto_cb_write_done %p, n = %d\n", bio, n); #endif + KTR_LOG(dmcrypt_crypto_cb_write_done, crp, bio->bio_buf, n); + if (n == 1) { dmtc = bio->bio_caller_info2.ptr; priv = (dm_target_crypt_config_t *)bio->bio_caller_info1.ptr; if (bio->bio_buf->b_error) { bio->bio_buf->b_flags |= B_ERROR; - mpipe_free(&dmtc_write_mpipe, dmtc->free_addr); + mpipe_free(&dmtc->priv->write_mpipe, dmtc->free_addr); obio = pop_bio(bio); biodone(obio); } else { @@ -1241,7 +1299,10 @@ dmtc_bio_write_done(struct bio *bio) dmtc = bio->bio_caller_info2.ptr; bio->bio_buf->b_data = dmtc->orig_buf; - mpipe_free(&dmtc_write_mpipe, dmtc->free_addr); + mpipe_free(&dmtc->priv->write_mpipe, dmtc->free_addr); + + KTR_LOG(dmcrypt_bio_write_done, bio->bio_buf); + obio = pop_bio(bio); biodone(obio); } @@ -1438,19 +1499,14 @@ dmtc_mod_handler(module_t mod, int type, void *unused) dmt->upcall = &dm_target_crypt_upcall; dmt->dump = &dm_target_crypt_dump; - dmtc_init_mpipe(); - err = dm_target_insert(dmt); - if (err) - dmtc_destroy_mpipe(); - else + if (!err) kprintf("dm_target_crypt: Successfully initialized\n"); break; case MOD_UNLOAD: err = dm_target_rem("crypt"); if (err == 0) { - dmtc_destroy_mpipe(); kprintf("dm_target_crypt: unloaded\n"); } break; -- 2.41.0