diff --git a/module/bdev/crypto/vbdev_crypto.c b/module/bdev/crypto/vbdev_crypto.c index 44db9af2a..9b0ae402c 100644 --- a/module/bdev/crypto/vbdev_crypto.c +++ b/module/bdev/crypto/vbdev_crypto.c @@ -44,7 +44,6 @@ struct crypto_io_channel { }; enum crypto_io_resubmit_state { - CRYPTO_IO_NEW, /* Resubmit IO from the scratch */ CRYPTO_IO_DECRYPT_DONE, /* Appended decrypt, need to read */ CRYPTO_IO_ENCRYPT_DONE, /* Need to write */ }; @@ -164,7 +163,7 @@ crypto_encrypt(struct crypto_io_channel *crypto_ch, struct spdk_bdev_io *bdev_io crypto_io->aux_domain, crypto_io->aux_domain_ctx); if (rc == -ENOMEM) { SPDK_DEBUGLOG(vbdev_crypto, "No memory, queue the IO.\n"); - vbdev_crypto_queue_io(bdev_io, CRYPTO_IO_NEW); + spdk_bdev_io_complete(bdev_io, SPDK_BDEV_IO_STATUS_NOMEM); } else { SPDK_ERRLOG("Failed to submit bdev_io!\n"); crypto_io_fail(crypto_io); @@ -193,14 +192,8 @@ vbdev_crypto_resubmit_io(void *arg) { struct spdk_bdev_io *bdev_io = (struct spdk_bdev_io *)arg; struct crypto_bdev_io *crypto_io = (struct crypto_bdev_io *)bdev_io->driver_ctx; - struct spdk_io_channel *ch; switch (crypto_io->resubmit_state) { - case CRYPTO_IO_NEW: - assert(crypto_io->crypto_ch); - ch = spdk_io_channel_from_ctx(crypto_io->crypto_ch); - vbdev_crypto_submit_request(ch, bdev_io); - break; case CRYPTO_IO_ENCRYPT_DONE: crypto_write(crypto_io->crypto_ch, bdev_io); break; @@ -223,9 +216,6 @@ vbdev_crypto_queue_io(struct spdk_bdev_io *bdev_io, enum crypto_io_resubmit_stat crypto_io->bdev_io_wait.cb_arg = bdev_io; crypto_io->resubmit_state = state; - /* TODO: We shouldn't use spdk_bdev_queue_io_wait() for queueing IOs due to receiving ENOMEM - * from anything other than one of the bdev functions (e.g. accel). We should have a - * different mechanism for handling such requests. */ rc = spdk_bdev_queue_io_wait(bdev_io->bdev, crypto_io->crypto_ch->base_ch, &crypto_io->bdev_io_wait); if (rc != 0) { @@ -294,7 +284,7 @@ crypto_read_get_buf_cb(struct spdk_io_channel *ch, struct spdk_bdev_io *bdev_io, if (rc != 0) { if (rc == -ENOMEM) { SPDK_DEBUGLOG(vbdev_crypto, "No memory, queue the IO.\n"); - vbdev_crypto_queue_io(bdev_io, CRYPTO_IO_NEW); + spdk_bdev_io_complete(bdev_io, SPDK_BDEV_IO_STATUS_NOMEM); } else { SPDK_ERRLOG("Failed to submit bdev_io!\n"); crypto_io_fail(crypto_io); @@ -371,7 +361,7 @@ vbdev_crypto_submit_request(struct spdk_io_channel *ch, struct spdk_bdev_io *bde if (rc != 0) { if (rc == -ENOMEM) { SPDK_DEBUGLOG(vbdev_crypto, "No memory, queue the IO.\n"); - vbdev_crypto_queue_io(bdev_io, CRYPTO_IO_NEW); + spdk_bdev_io_complete(bdev_io, SPDK_BDEV_IO_STATUS_NOMEM); } else { SPDK_ERRLOG("Failed to submit bdev_io!\n"); crypto_io_fail(crypto_io); diff --git a/test/bdev/chaining.sh b/test/bdev/chaining.sh index 990faf1cf..6c38df0c3 100755 --- a/test/bdev/chaining.sh +++ b/test/bdev/chaining.sh @@ -49,17 +49,21 @@ update_stats() { stats[copy_executed]=$(get_stat executed copy) } -cleanup() { +tgtcleanup() { [[ -v input ]] && rm -f "$input" || : [[ -v output ]] && rm -f "$output" || : nvmftestfini } +bperfcleanup() { + [[ -v bperfpid ]] && killprocess $bperfpid +} + nvmftestinit nvmfappstart -m 0x2 input=$(mktemp) output=$(mktemp) -trap 'cleanup; exit 1' SIGINT SIGTERM EXIT +trap 'tgtcleanup; exit 1' SIGINT SIGTERM EXIT rpc_cmd <<- CONFIG bdev_malloc_create 32 4096 -b malloc0 @@ -115,4 +119,27 @@ spdk_dd --of "$output" --ib Nvme0n1 --bs 4096 --count 16 cmp "$input" "$output" trap - SIGINT SIGTERM EXIT -cleanup +tgtcleanup + +# Verify bdev crypto ENOMEM handling by setting low accel task count and sending IO with high qd +trap 'bperfcleanup; exit 1' SIGINT SIGTERM EXIT + +"$rootdir/build/examples/bdevperf" -t 5 -w verify -o 4096 -q 256 --wait-for-rpc -z & +bperfpid=$! + +waitforlisten $bperfpid +rpc_cmd <<- CONFIG + accel_set_options --task-count 16 + framework_start_init + bdev_malloc_create 32 4096 -b malloc0 + accel_crypto_key_create -c AES_XTS -k "${key0[0]}" -e "${key0[1]}" -n key0 + accel_crypto_key_create -c AES_XTS -k "${key1[0]}" -e "${key1[1]}" -n key1 + bdev_crypto_create malloc0 crypto0 -n key0 + bdev_crypto_create crypto0 crypto1 -n key1 +CONFIG + +"$rootdir/examples/bdev/bdevperf/bdevperf.py" perform_tests + +trap - SIGINT SIGTERM EXIT +killprocess $bperfpid +wait $bperfpid diff --git a/test/unit/lib/bdev/crypto.c/crypto_ut.c b/test/unit/lib/bdev/crypto.c/crypto_ut.c index 1991c9115..9bf8ef699 100644 --- a/test/unit/lib/bdev/crypto.c/crypto_ut.c +++ b/test/unit/lib/bdev/crypto.c/crypto_ut.c @@ -254,13 +254,10 @@ test_error_paths(void) /* test error returned by accel fw */ MOCK_SET(spdk_accel_append_encrypt, -ENOMEM); + g_completion_called = false; vbdev_crypto_submit_request(g_io_ch, g_bdev_io); - CU_ASSERT(g_bdev_io->internal.status == SPDK_BDEV_IO_STATUS_SUCCESS); - CU_ASSERT(g_io_ctx->bdev_io_wait.bdev == &g_crypto_bdev.crypto_bdev); - CU_ASSERT(g_io_ctx->bdev_io_wait.cb_fn == vbdev_crypto_resubmit_io); - CU_ASSERT(g_io_ctx->bdev_io_wait.cb_arg == g_bdev_io); - CU_ASSERT(g_io_ctx->resubmit_state == CRYPTO_IO_NEW); - memset(&g_io_ctx->bdev_io_wait, 0, sizeof(g_io_ctx->bdev_io_wait)); + CU_ASSERT(g_bdev_io->internal.status == SPDK_BDEV_IO_STATUS_NOMEM); + CU_ASSERT(g_completion_called); MOCK_SET(spdk_accel_append_encrypt, -EINVAL); vbdev_crypto_submit_request(g_io_ch, g_bdev_io); @@ -328,15 +325,12 @@ test_error_paths(void) /* test error returned by accel fw */ g_bdev_io->internal.status = SPDK_BDEV_IO_STATUS_SUCCESS; MOCK_SET(spdk_accel_append_decrypt, -ENOMEM); + g_completion_called = false; vbdev_crypto_submit_request(g_io_ch, g_bdev_io); - poll_threads(); - CU_ASSERT(g_bdev_io->internal.status == SPDK_BDEV_IO_STATUS_SUCCESS); - CU_ASSERT(g_io_ctx->bdev_io_wait.bdev == &g_crypto_bdev.crypto_bdev); - CU_ASSERT(g_io_ctx->bdev_io_wait.cb_fn == vbdev_crypto_resubmit_io); - CU_ASSERT(g_io_ctx->bdev_io_wait.cb_arg == g_bdev_io); - CU_ASSERT(g_io_ctx->resubmit_state == CRYPTO_IO_NEW); - memset(&g_io_ctx->bdev_io_wait, 0, sizeof(g_io_ctx->bdev_io_wait)); + CU_ASSERT(g_bdev_io->internal.status == SPDK_BDEV_IO_STATUS_NOMEM); + CU_ASSERT(g_completion_called); MOCK_SET(spdk_accel_append_decrypt, 0); + g_completion_called = false; } static void