From ee8af4e9f3baa48b1edda539fe679a4bc08af69a Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Fri, 23 Feb 2018 14:26:17 -0700 Subject: [PATCH] blob: fix _spdk_bs_load_ctx_fail ordering Finish the sequence first, before calling _spdk_bs_free(). Otherwise synchronous bs_devs (like we use in the unit tests) cause the sequence memory to get freed via _spdk_bs_free() and then we try to finish the sequence. This eliminates the need for g_scheduler_delay and _bs_flush_scheduler() in the blob unit tests. But don't remove them - they will be useful in upcoming unit tests for queued persist operations. Signed-off-by: Jim Harris Change-Id: I09aac3ae4d3a56ff8e04a5b822fcd6746f13afc3 Reviewed-on: https://review.gerrithub.io/401267 Tested-by: SPDK Automated Test System Reviewed-by: Maciej Szwed Reviewed-by: Daniel Verkamp Reviewed-by: Changpeng Liu Reviewed-by: Shuhei Matsumoto --- lib/blob/blobstore.c | 2 +- test/unit/lib/blob/blob.c/blob_ut.c | 27 +++------------------------ 2 files changed, 4 insertions(+), 25 deletions(-) diff --git a/lib/blob/blobstore.c b/lib/blob/blobstore.c index c523c43e9..84a6b4a50 100644 --- a/lib/blob/blobstore.c +++ b/lib/blob/blobstore.c @@ -2070,6 +2070,7 @@ _spdk_bs_load_ctx_fail(spdk_bs_sequence_t *seq, struct spdk_bs_load_ctx *ctx, in assert(bserrno != 0); spdk_dma_free(ctx->super); + spdk_bs_sequence_finish(seq, bserrno); /* * Only free the blobstore when a load fails. If an unload fails (for some reason) * we want to keep the blobstore in case the caller wants to try again. @@ -2078,7 +2079,6 @@ _spdk_bs_load_ctx_fail(spdk_bs_sequence_t *seq, struct spdk_bs_load_ctx *ctx, in _spdk_bs_free(ctx->bs); } free(ctx); - spdk_bs_sequence_finish(seq, bserrno); } static void diff --git a/test/unit/lib/blob/blob.c/blob_ut.c b/test/unit/lib/blob/blob.c/blob_ut.c index 4918bae84..c415a5176 100644 --- a/test/unit/lib/blob/blob.c/blob_ut.c +++ b/test/unit/lib/blob/blob.c/blob_ut.c @@ -101,6 +101,7 @@ _bs_send_msg(spdk_thread_fn fn, void *ctx, void *thread_ctx) } } +#if 0 static void _bs_flush_scheduler(void) { @@ -112,6 +113,7 @@ _bs_flush_scheduler(void) free(ops); } } +#endif static void bs_op_complete(void *cb_arg, int bserrno) @@ -1354,8 +1356,6 @@ bs_load(void) size_t value_len; struct spdk_bs_opts opts; - g_scheduler_delay = true; - dev = init_dev(); spdk_bs_opts_init(&opts); strncpy(opts.bstype.bstype, "TESTTYPE", SPDK_BLOBSTORE_TYPE_LENGTH); @@ -1452,7 +1452,6 @@ bs_load(void) spdk_bs_unload(g_bs, bs_op_complete, NULL); CU_ASSERT(g_bserrno == 0); g_bs = NULL; - g_scheduler_delay = false; } static void @@ -1461,8 +1460,6 @@ bs_type(void) struct spdk_bs_dev *dev; struct spdk_bs_opts opts; - g_scheduler_delay = true; - dev = init_dev(); spdk_bs_opts_init(&opts); strncpy(opts.bstype.bstype, "TESTTYPE", SPDK_BLOBSTORE_TYPE_LENGTH); @@ -1521,7 +1518,6 @@ bs_type(void) spdk_bs_unload(g_bs, bs_op_complete, NULL); CU_ASSERT(g_bserrno == 0); g_bs = NULL; - g_scheduler_delay = false; } static void @@ -1532,8 +1528,6 @@ bs_super_block(void) struct spdk_bs_opts opts; struct spdk_bs_super_block_ver1 super_block_v1; - g_scheduler_delay = true; - dev = init_dev(); spdk_bs_opts_init(&opts); strncpy(opts.bstype.bstype, "TESTTYPE", SPDK_BLOBSTORE_TYPE_LENGTH); @@ -1584,7 +1578,6 @@ bs_super_block(void) spdk_bs_unload(g_bs, bs_op_complete, NULL); CU_ASSERT(g_bserrno == 0); g_bs = NULL; - g_scheduler_delay = false; } /* @@ -1867,11 +1860,6 @@ bs_destroy(void) struct spdk_bs_dev *dev; struct spdk_bs_opts opts; - g_scheduler_delay = true; - - _bs_flush_scheduler(); - CU_ASSERT(TAILQ_EMPTY(&g_scheduled_ops)); - /* Initialize a new blob store */ dev = init_dev(); spdk_bs_opts_init(&opts); @@ -1882,9 +1870,6 @@ bs_destroy(void) /* Destroy the blob store */ g_bserrno = -1; spdk_bs_destroy(g_bs, bs_op_complete, NULL); - /* Callback is called after device is destroyed in next scheduler run. */ - _bs_flush_scheduler(); - CU_ASSERT(TAILQ_EMPTY(&g_scheduled_ops)); CU_ASSERT(g_bserrno == 0); /* Loading an non-existent blob store should fail. */ @@ -1894,7 +1879,6 @@ bs_destroy(void) spdk_bs_load(dev, &opts, bs_op_with_handle_complete, NULL); CU_ASSERT(g_bserrno != 0); - g_scheduler_delay = false; } /* Try to hit all of the corner cases associated with serializing @@ -2071,15 +2055,10 @@ super_block_crc(void) super_block->crc = 0; dev = init_dev(); - g_scheduler_delay = true; /* Load an existing blob store */ + g_bserrno = 0; spdk_bs_load(dev, &opts, bs_op_with_handle_complete, NULL); - CU_ASSERT(g_bserrno == -EILSEQ); - _bs_flush_scheduler(); - CU_ASSERT(TAILQ_EMPTY(&g_scheduled_ops)); - - g_scheduler_delay = false; } /* For blob dirty shutdown test case we do the following sub-test cases: