From 9072c4ad0d546a5bdf566eff7cd4d8352bdd885c Mon Sep 17 00:00:00 2001 From: GangCao Date: Thu, 14 Oct 2021 16:53:53 -0400 Subject: [PATCH] accel: create SW Engine Channel if HW Engine not supports Currently either HW Engine Channel or SW Engine Channel will be used. In the case that HW Engine Channel is used while does not support related operations like IOAT for CRC, it will shift back to the SW Engine's handle. So that this is an issue that it still refers to the HW Engine Channel while needs SW Eninge Channel to handle. This patch introduces the SW Eninge Channel and always initializes there in case that HW Engine does not support some operations. Related UT also added to simulate the case the IOAT does not support CRC and then SW Eninge needs to properly handle it. Change-Id: I4ecdcd09ab669a616b37c567b45b1e6499800ec9 Signed-off-by: GangCao Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9874 Tested-by: SPDK CI Jenkins Reviewed-by: Changpeng Liu Reviewed-by: Jim Harris Community-CI: Broadcom CI Community-CI: Mellanox Build Bot --- include/spdk_internal/accel_engine.h | 1 + lib/accel/accel_engine.c | 11 +++- test/unit/lib/accel/accel.c/accel_engine_ut.c | 50 ++++++++++++++++++- 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/include/spdk_internal/accel_engine.h b/include/spdk_internal/accel_engine.h index 5a72dacb3..c40b4b709 100644 --- a/include/spdk_internal/accel_engine.h +++ b/include/spdk_internal/accel_engine.h @@ -46,6 +46,7 @@ void spdk_accel_task_complete(struct spdk_accel_task *task, int status); struct accel_io_channel { struct spdk_accel_engine *engine; struct spdk_io_channel *engine_ch; + struct spdk_io_channel *sw_engine_ch; void *task_pool_base; TAILQ_HEAD(, spdk_accel_task) task_pool; void *batch_pool_base; diff --git a/lib/accel/accel_engine.c b/lib/accel/accel_engine.c index b5e9ecbd2..949208aab 100644 --- a/lib/accel/accel_engine.c +++ b/lib/accel/accel_engine.c @@ -190,7 +190,7 @@ _get_task(struct accel_io_channel *accel_ch, struct spdk_accel_batch *batch, inline static void _add_to_comp_list(struct accel_io_channel *accel_ch, struct spdk_accel_task *accel_task, int status) { - struct sw_accel_io_channel *sw_ch = spdk_io_channel_get_ctx(accel_ch->engine_ch); + struct sw_accel_io_channel *sw_ch = spdk_io_channel_get_ctx(accel_ch->sw_engine_ch); accel_task->status = status; TAILQ_INSERT_TAIL(&sw_ch->tasks_to_complete, accel_task, link); @@ -958,12 +958,16 @@ accel_engine_create_cb(void *io_device, void *ctx_buf) batch++; } + /* Set sw engine channel for operations where hw engine does not support. */ + accel_ch->sw_engine_ch = g_sw_accel_engine->get_io_channel(); + assert(accel_ch->sw_engine_ch != NULL); + if (g_hw_accel_engine != NULL) { accel_ch->engine_ch = g_hw_accel_engine->get_io_channel(); accel_ch->engine = g_hw_accel_engine; } else { /* No hw engine enabled, use sw. */ - accel_ch->engine_ch = g_sw_accel_engine->get_io_channel(); + accel_ch->engine_ch = accel_ch->sw_engine_ch; accel_ch->engine = g_sw_accel_engine; } assert(accel_ch->engine_ch != NULL); @@ -979,6 +983,9 @@ accel_engine_destroy_cb(void *io_device, void *ctx_buf) struct accel_io_channel *accel_ch = ctx_buf; free(accel_ch->batch_pool_base); + if (accel_ch->sw_engine_ch != accel_ch->engine_ch) { + spdk_put_io_channel(accel_ch->sw_engine_ch); + } spdk_put_io_channel(accel_ch->engine_ch); free(accel_ch->task_pool_base); } diff --git a/test/unit/lib/accel/accel.c/accel_engine_ut.c b/test/unit/lib/accel/accel.c/accel_engine_ut.c index c6ae43727..72d98b803 100644 --- a/test/unit/lib/accel/accel.c/accel_engine_ut.c +++ b/test/unit/lib/accel/accel.c/accel_engine_ut.c @@ -63,7 +63,8 @@ test_setup(void) return -1; } g_accel_ch->engine_ch = g_engine_ch; - g_sw_ch = (struct sw_accel_io_channel *)((char *)g_accel_ch->engine_ch + sizeof( + g_accel_ch->sw_engine_ch = g_engine_ch; + g_sw_ch = (struct sw_accel_io_channel *)((char *)g_accel_ch->sw_engine_ch + sizeof( struct spdk_io_channel)); TAILQ_INIT(&g_sw_ch->tasks_to_complete); return 0; @@ -649,6 +650,52 @@ test_spdk_accel_submit_crc32c(void) CU_ASSERT(expected_accel_task == &task); } +static void +test_spdk_accel_submit_crc32c_hw_engine_unsupported(void) +{ + const uint64_t nbytes = TEST_SUBMIT_SIZE; + uint32_t crc_dst; + uint8_t src[TEST_SUBMIT_SIZE]; + uint32_t seed = 1; + void *cb_arg = NULL; + int rc; + struct spdk_accel_task task; + struct spdk_accel_task *expected_accel_task = NULL; + + /* Fail with no tasks on _get_task() */ + rc = spdk_accel_submit_crc32c(g_ch, &crc_dst, src, seed, nbytes, dummy_submit_cb_fn, cb_arg); + CU_ASSERT(rc == -ENOMEM); + + TAILQ_INIT(&g_accel_ch->task_pool); + task.cb_fn = dummy_submit_cb_fn; + task.cb_arg = cb_arg; + task.accel_ch = g_accel_ch; + task.batch = NULL; + TAILQ_INSERT_TAIL(&g_accel_ch->task_pool, &task, link); + + g_accel_ch->engine = &g_accel_engine; + /* HW engine only supports COPY and does not support CRC */ + g_accel_ch->engine->capabilities = ACCEL_COPY; + g_accel_ch->engine->submit_tasks = dummy_submit_tasks; + + /* Summit to HW engine while eventually handled by SW engine. */ + rc = spdk_accel_submit_crc32c(g_ch, &crc_dst, src, seed, nbytes, dummy_submit_cb_fn, cb_arg); + CU_ASSERT(rc == 0); + CU_ASSERT(task.crc_dst == &crc_dst); + CU_ASSERT(task.src == src); + CU_ASSERT(task.v.iovcnt == 0); + CU_ASSERT(task.seed == seed); + CU_ASSERT(task.op_code == ACCEL_OPCODE_CRC32C); + CU_ASSERT(task.nbytes == nbytes); + /* Not set in HW engine callback while handled by SW engine instead. */ + CU_ASSERT(g_dummy_submit_called == false); + + /* SW engine does crc. */ + expected_accel_task = TAILQ_FIRST(&g_sw_ch->tasks_to_complete); + TAILQ_REMOVE(&g_sw_ch->tasks_to_complete, expected_accel_task, link); + CU_ASSERT(expected_accel_task == &task); +} + static void test_spdk_accel_submit_crc32cv(void) { @@ -818,6 +865,7 @@ int main(int argc, char **argv) CU_ADD_TEST(suite, test_spdk_accel_submit_compare); CU_ADD_TEST(suite, test_spdk_accel_submit_fill); CU_ADD_TEST(suite, test_spdk_accel_submit_crc32c); + CU_ADD_TEST(suite, test_spdk_accel_submit_crc32c_hw_engine_unsupported); CU_ADD_TEST(suite, test_spdk_accel_submit_crc32cv); CU_ADD_TEST(suite, test_spdk_accel_submit_copy_crc32c);