From a19781ba616dcc3b9d53e8cbcfcb883a066e4699 Mon Sep 17 00:00:00 2001 From: paul luse Date: Tue, 23 Mar 2021 12:43:31 -0400 Subject: [PATCH] lib/accel: rework UT for code reuse Moved frequenty used stack vars to globals and added setup and teardown functions. Should be useful in upcoming patches as well wrt code savings. Signed-off-by: paul luse Change-Id: I468bec8856c354fcc954628e4e733594a6580104 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/7013 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Aleksey Marchuk Reviewed-by: Jim Harris Reviewed-by: Monica Kenguva Reviewed-by: Ziye Yang --- test/unit/lib/accel/accel.c/accel_engine_ut.c | 143 +++++++++--------- 1 file changed, 75 insertions(+), 68 deletions(-) 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 b137f5ab7..4b93fdf3c 100644 --- a/test/unit/lib/accel/accel.c/accel_engine_ut.c +++ b/test/unit/lib/accel/accel.c/accel_engine_ut.c @@ -41,30 +41,53 @@ DEFINE_STUB(spdk_json_write_array_begin, int, (struct spdk_json_write_ctx *w), 0); DEFINE_STUB(spdk_json_write_array_end, int, (struct spdk_json_write_ctx *w), 0); +/* global vars and setup/cleanup functions used for all test functions */ +struct spdk_accel_engine g_accel_engine = {}; +struct spdk_io_channel *g_ch = NULL; +struct accel_io_channel *g_accel_ch = NULL; + +static int +test_setup(void) +{ + g_ch = calloc(1, sizeof(struct spdk_io_channel) + sizeof(struct accel_io_channel)); + if (g_ch == NULL) { + /* for some reason the assert fatal macro doesn't work in the setup function. */ + CU_ASSERT(false); + return -1; + } + g_accel_ch = (struct accel_io_channel *)((char *)g_ch + sizeof(struct spdk_io_channel)); + + return 0; +} + +static int +test_cleanup(void) +{ + free(g_ch); + + return 0; +} + static void test_spdk_accel_hw_engine_register(void) { - struct spdk_accel_engine accel_engine; - /* Run once with no engine assigned, assign it. */ g_hw_accel_engine = NULL; - spdk_accel_hw_engine_register(&accel_engine); - CU_ASSERT(g_hw_accel_engine == &accel_engine); + spdk_accel_hw_engine_register(&g_accel_engine); + CU_ASSERT(g_hw_accel_engine == &g_accel_engine); /* Run with one assigned, should not change. */ - spdk_accel_hw_engine_register(&accel_engine); - CU_ASSERT(g_hw_accel_engine == &accel_engine); + spdk_accel_hw_engine_register(&g_accel_engine); + CU_ASSERT(g_hw_accel_engine == &g_accel_engine); } static int test_accel_sw_register(void) { - struct spdk_accel_engine accel_engine; - /* Run once with no engine assigned, assign it. */ g_sw_accel_engine = NULL; - accel_sw_register(&accel_engine); - CU_ASSERT(g_sw_accel_engine == &accel_engine); + accel_sw_register(&g_accel_engine); + CU_ASSERT(g_sw_accel_engine == &g_accel_engine); return 0; } @@ -72,10 +95,8 @@ test_accel_sw_register(void) static void test_accel_sw_unregister(void) { - struct spdk_accel_engine accel_engine; - /* Run once engine assigned, make sure it gets unassigned. */ - g_sw_accel_engine = &accel_engine; + g_sw_accel_engine = &g_accel_engine; accel_sw_unregister(); CU_ASSERT(g_sw_accel_engine == NULL); } @@ -83,15 +104,13 @@ test_accel_sw_unregister(void) static void test_is_supported(void) { - struct spdk_accel_engine engine; - - engine.capabilities = ACCEL_COPY | ACCEL_DUALCAST | ACCEL_CRC32C; - CU_ASSERT(_is_supported(&engine, ACCEL_COPY) == true); - CU_ASSERT(_is_supported(&engine, ACCEL_FILL) == false); - CU_ASSERT(_is_supported(&engine, ACCEL_DUALCAST) == true); - CU_ASSERT(_is_supported(&engine, ACCEL_COMPARE) == false); - CU_ASSERT(_is_supported(&engine, ACCEL_CRC32C) == true); - CU_ASSERT(_is_supported(&engine, ACCEL_DIF) == false); + g_accel_engine.capabilities = ACCEL_COPY | ACCEL_DUALCAST | ACCEL_CRC32C; + CU_ASSERT(_is_supported(&g_accel_engine, ACCEL_COPY) == true); + CU_ASSERT(_is_supported(&g_accel_engine, ACCEL_FILL) == false); + CU_ASSERT(_is_supported(&g_accel_engine, ACCEL_DUALCAST) == true); + CU_ASSERT(_is_supported(&g_accel_engine, ACCEL_COMPARE) == false); + CU_ASSERT(_is_supported(&g_accel_engine, ACCEL_CRC32C) == true); + CU_ASSERT(_is_supported(&g_accel_engine, ACCEL_DIF) == false); } #define DUMMY_ARG 0xDEADBEEF @@ -116,7 +135,6 @@ dummy_batch_cb_fn(void *cb_arg, int status) static void test_spdk_accel_task_complete(void) { - struct accel_io_channel accel_ch = {}; struct spdk_accel_task accel_task = {}; struct spdk_accel_task *expected_accel_task = NULL; struct spdk_accel_batch batch = {}; @@ -124,21 +142,21 @@ test_spdk_accel_task_complete(void) uint32_t cb_arg = DUMMY_ARG; int status = 0; - accel_task.accel_ch = &accel_ch; + accel_task.accel_ch = g_accel_ch; accel_task.cb_fn = dummy_cb_fn; accel_task.cb_arg = &cb_arg; - TAILQ_INIT(&accel_ch.task_pool); + TAILQ_INIT(&g_accel_ch->task_pool); /* W/o batch, confirm cb is called and task added to list. */ spdk_accel_task_complete(&accel_task, status); CU_ASSERT(g_dummy_cb_called == true); - expected_accel_task = TAILQ_FIRST(&accel_ch.task_pool); - TAILQ_REMOVE(&accel_ch.task_pool, expected_accel_task, link); + expected_accel_task = TAILQ_FIRST(&g_accel_ch->task_pool); + TAILQ_REMOVE(&g_accel_ch->task_pool, expected_accel_task, link); CU_ASSERT(expected_accel_task == &accel_task); - TAILQ_INIT(&accel_ch.task_pool); - TAILQ_INIT(&accel_ch.batches); - TAILQ_INIT(&accel_ch.batch_pool); + TAILQ_INIT(&g_accel_ch->task_pool); + TAILQ_INIT(&g_accel_ch->batches); + TAILQ_INIT(&g_accel_ch->batch_pool); batch.count = 2; batch.cb_fn = dummy_batch_cb_fn; batch.cb_arg = &cb_arg; @@ -150,68 +168,58 @@ test_spdk_accel_task_complete(void) CU_ASSERT(batch.count == 1); CU_ASSERT(false == g_dummy_batch_cb_called); - expected_accel_task = TAILQ_FIRST(&accel_ch.task_pool); - TAILQ_REMOVE(&accel_ch.task_pool, expected_accel_task, link); + expected_accel_task = TAILQ_FIRST(&g_accel_ch->task_pool); + TAILQ_REMOVE(&g_accel_ch->task_pool, expected_accel_task, link); CU_ASSERT(expected_accel_task == &accel_task); - CU_ASSERT(true == TAILQ_EMPTY(&accel_ch.batch_pool)); - CU_ASSERT(true == TAILQ_EMPTY(&accel_ch.batches)); + CU_ASSERT(true == TAILQ_EMPTY(&g_accel_ch->batch_pool)); + CU_ASSERT(true == TAILQ_EMPTY(&g_accel_ch->batches)); - TAILQ_INIT(&accel_ch.task_pool); - TAILQ_INSERT_TAIL(&accel_ch.batches, &batch, link); + TAILQ_INIT(&g_accel_ch->task_pool); + TAILQ_INSERT_TAIL(&g_accel_ch->batches, &batch, link); /* Call it again and the batch should complete and lists updated accordingly. */ spdk_accel_task_complete(&accel_task, status); CU_ASSERT(batch.count == 0); CU_ASSERT(true == g_dummy_batch_cb_called); - CU_ASSERT(true == TAILQ_EMPTY(&accel_ch.batches)); + CU_ASSERT(true == TAILQ_EMPTY(&g_accel_ch->batches)); - expected_accel_task = TAILQ_FIRST(&accel_ch.task_pool); - TAILQ_REMOVE(&accel_ch.task_pool, expected_accel_task, link); + expected_accel_task = TAILQ_FIRST(&g_accel_ch->task_pool); + TAILQ_REMOVE(&g_accel_ch->task_pool, expected_accel_task, link); CU_ASSERT(expected_accel_task == &accel_task); - expected_batch = TAILQ_FIRST(&accel_ch.batch_pool); - TAILQ_REMOVE(&accel_ch.batch_pool, expected_batch, link); + expected_batch = TAILQ_FIRST(&g_accel_ch->batch_pool); + TAILQ_REMOVE(&g_accel_ch->batch_pool, expected_batch, link); CU_ASSERT(expected_batch == &batch); } static void test_spdk_accel_get_capabilities(void) { - struct spdk_io_channel *ch; - struct accel_io_channel *accel_ch; - struct spdk_accel_engine engine; uint64_t cap, expected_cap; - ch = calloc(1, sizeof(struct spdk_io_channel) + sizeof(struct accel_io_channel)); - SPDK_CU_ASSERT_FATAL(ch != NULL); - /* Setup a few capabilites and make sure they are reported as expected. */ - accel_ch = (struct accel_io_channel *)spdk_io_channel_get_ctx(ch); - accel_ch->engine = &engine; + g_accel_ch->engine = &g_accel_engine; expected_cap = ACCEL_COPY | ACCEL_DUALCAST | ACCEL_CRC32C; - accel_ch->engine->capabilities = expected_cap; + g_accel_ch->engine->capabilities = expected_cap; - cap = spdk_accel_get_capabilities(ch); + cap = spdk_accel_get_capabilities(g_ch); CU_ASSERT(cap == expected_cap); - - free(ch); } static void test_is_batch_valid(void) { struct spdk_accel_batch batch = {}; - struct accel_io_channel accel_ch = {}; bool rc; /* This batch doesn't go with this channel. */ batch.accel_ch = (struct accel_io_channel *)0xDEADBEEF; - rc = _is_batch_valid(&batch, &accel_ch); + rc = _is_batch_valid(&batch, g_accel_ch); CU_ASSERT(rc == false); /* This one does. */ - batch.accel_ch = &accel_ch; - rc = _is_batch_valid(&batch, &accel_ch); + batch.accel_ch = g_accel_ch; + rc = _is_batch_valid(&batch, g_accel_ch); CU_ASSERT(rc == true); } @@ -219,38 +227,37 @@ static void test_get_task(void) { struct spdk_accel_batch batch = {}; - struct accel_io_channel accel_ch = {}; struct spdk_accel_task *task; struct spdk_accel_task _task; void *cb_arg = NULL; /* NULL batch should return NULL task. */ - task = _get_task(&accel_ch, NULL, dummy_cb_fn, cb_arg); + task = _get_task(g_accel_ch, NULL, dummy_cb_fn, cb_arg); CU_ASSERT(task == NULL); /* valid batch with bogus accel_ch should return NULL task. */ - task = _get_task(&accel_ch, &batch, dummy_cb_fn, cb_arg); + task = _get_task(g_accel_ch, &batch, dummy_cb_fn, cb_arg); CU_ASSERT(task == NULL); - TAILQ_INIT(&accel_ch.task_pool); - batch.accel_ch = &accel_ch; + TAILQ_INIT(&g_accel_ch->task_pool); + batch.accel_ch = g_accel_ch; /* no tasks left, return NULL. */ - task = _get_task(&accel_ch, &batch, dummy_cb_fn, cb_arg); + task = _get_task(g_accel_ch, &batch, dummy_cb_fn, cb_arg); CU_ASSERT(task == NULL); _task.cb_fn = dummy_cb_fn; _task.cb_arg = cb_arg; - _task.accel_ch = &accel_ch; + _task.accel_ch = g_accel_ch; _task.batch = &batch; - TAILQ_INSERT_TAIL(&accel_ch.task_pool, &_task, link); + TAILQ_INSERT_TAIL(&g_accel_ch->task_pool, &_task, link); /* Get a valid task. */ - task = _get_task(&accel_ch, &batch, dummy_cb_fn, cb_arg); + task = _get_task(g_accel_ch, &batch, dummy_cb_fn, cb_arg); CU_ASSERT(task == &_task); CU_ASSERT(_task.cb_fn == dummy_cb_fn); CU_ASSERT(_task.cb_arg == cb_arg); - CU_ASSERT(_task.accel_ch == &accel_ch); + CU_ASSERT(_task.accel_ch == g_accel_ch); CU_ASSERT(_task.batch->count == 1); } @@ -262,7 +269,7 @@ int main(int argc, char **argv) CU_set_error_action(CUEA_ABORT); CU_initialize_registry(); - suite = CU_add_suite("accel", NULL, NULL); + suite = CU_add_suite("accel", test_setup, test_cleanup); CU_ADD_TEST(suite, test_spdk_accel_hw_engine_register); CU_ADD_TEST(suite, test_accel_sw_register);