From 9da1c7384d8b540a85c489429e7592dbf805d7c6 Mon Sep 17 00:00:00 2001 From: Changpeng Liu Date: Wed, 15 May 2019 20:49:57 -0400 Subject: [PATCH] bdev: don't call spdk_bdev_free_io() for the error case Existing code in spdk_bdev_write_zeroes_blocks() will call spdk_bdev_free_io() for the error case, which will cause assertion because the bdev_io isn't submitted to the backend yet, so we will check the condtion first to avoid the error case. Change-Id: If27d78217f709a3315e74c00869d345abd6b9a69 Signed-off-by: Changpeng Liu Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/453491 Tested-by: SPDK CI Jenkins Reviewed-by: Shuhei Matsumoto Reviewed-by: Jim Harris --- lib/bdev/bdev.c | 22 ++++++++------ test/unit/lib/bdev/bdev.c/bdev_ut.c | 47 ++++++++++++++++++++++++++--- 2 files changed, 55 insertions(+), 14 deletions(-) diff --git a/lib/bdev/bdev.c b/lib/bdev/bdev.c index 4a78763b6..8472ee389 100644 --- a/lib/bdev/bdev.c +++ b/lib/bdev/bdev.c @@ -2908,6 +2908,11 @@ spdk_bdev_write_zeroes_blocks(struct spdk_bdev_desc *desc, struct spdk_io_channe return -EINVAL; } + if (!_spdk_bdev_io_type_supported(bdev, SPDK_BDEV_IO_TYPE_WRITE_ZEROES) && + !_spdk_bdev_io_type_supported(bdev, SPDK_BDEV_IO_TYPE_WRITE)) { + return -ENOTSUP; + } + bdev_io = spdk_bdev_get_io(channel); if (!bdev_io) { @@ -2924,16 +2929,15 @@ spdk_bdev_write_zeroes_blocks(struct spdk_bdev_desc *desc, struct spdk_io_channe if (_spdk_bdev_io_type_supported(bdev, SPDK_BDEV_IO_TYPE_WRITE_ZEROES)) { spdk_bdev_io_submit(bdev_io); return 0; - } else if (_spdk_bdev_io_type_supported(bdev, SPDK_BDEV_IO_TYPE_WRITE)) { - assert(spdk_bdev_get_block_size(bdev) <= ZERO_BUFFER_SIZE); - bdev_io->u.bdev.split_remaining_num_blocks = num_blocks; - bdev_io->u.bdev.split_current_offset_blocks = offset_blocks; - _spdk_bdev_write_zero_buffer_next(bdev_io); - return 0; - } else { - spdk_bdev_free_io(bdev_io); - return -ENOTSUP; } + + assert(_spdk_bdev_io_type_supported(bdev, SPDK_BDEV_IO_TYPE_WRITE)); + assert(spdk_bdev_get_block_size(bdev) <= ZERO_BUFFER_SIZE); + bdev_io->u.bdev.split_remaining_num_blocks = num_blocks; + bdev_io->u.bdev.split_current_offset_blocks = offset_blocks; + _spdk_bdev_write_zero_buffer_next(bdev_io); + + return 0; } int diff --git a/test/unit/lib/bdev/bdev.c/bdev_ut.c b/test/unit/lib/bdev/bdev.c/bdev_ut.c index 00eee80c2..a0cdc95e2 100644 --- a/test/unit/lib/bdev/bdev.c/bdev_ut.c +++ b/test/unit/lib/bdev/bdev.c/bdev_ut.c @@ -229,11 +229,8 @@ bdev_ut_get_io_channel(void *ctx) return spdk_get_io_channel(&g_bdev_ut_io_device); } -static bool -stub_io_type_supported(void *_bdev, enum spdk_bdev_io_type io_type) -{ - return true; -} +DEFINE_STUB(stub_io_type_supported, static bool, (void *_bdev, enum spdk_bdev_io_type io_type), + true); static struct spdk_bdev_fn_table fn_table = { .destruct = stub_destruct, @@ -763,6 +760,45 @@ io_wait_cb(void *arg) entry->submitted = true; } +static void +bdev_io_types_test(void) +{ + struct spdk_bdev *bdev; + struct spdk_bdev_desc *desc = NULL; + struct spdk_io_channel *io_ch; + struct spdk_bdev_opts bdev_opts = { + .bdev_io_pool_size = 4, + .bdev_io_cache_size = 2, + }; + int rc; + + rc = spdk_bdev_set_opts(&bdev_opts); + CU_ASSERT(rc == 0); + spdk_bdev_initialize(bdev_init_cb, NULL); + poll_threads(); + + bdev = allocate_bdev("bdev0"); + + rc = spdk_bdev_open(bdev, true, NULL, NULL, &desc); + CU_ASSERT(rc == 0); + poll_threads(); + SPDK_CU_ASSERT_FATAL(desc != NULL); + io_ch = spdk_bdev_get_io_channel(desc); + CU_ASSERT(io_ch != NULL); + + /* WRITE and WRITE ZEROES are not supported */ + MOCK_SET(stub_io_type_supported, false); + rc = spdk_bdev_write_zeroes_blocks(desc, io_ch, 0, 128, io_done, NULL); + CU_ASSERT(rc == -ENOTSUP); + MOCK_SET(stub_io_type_supported, true); + + spdk_put_io_channel(io_ch); + spdk_bdev_close(desc); + free_bdev(bdev); + spdk_bdev_finish(bdev_fini_cb, NULL); + poll_threads(); +} + static void bdev_io_wait_test(void) { @@ -1656,6 +1692,7 @@ main(int argc, char **argv) CU_add_test(suite, "open_write", open_write_test) == NULL || CU_add_test(suite, "alias_add_del", alias_add_del_test) == NULL || CU_add_test(suite, "get_device_stat", get_device_stat_test) == NULL || + CU_add_test(suite, "bdev_io_types", bdev_io_types_test) == NULL || CU_add_test(suite, "bdev_io_wait", bdev_io_wait_test) == NULL || CU_add_test(suite, "bdev_io_spans_boundary", bdev_io_spans_boundary_test) == NULL || CU_add_test(suite, "bdev_io_split", bdev_io_split) == NULL ||