From b4ffeaec7ab887bfe0e0c5a4b69f27f27006a48a Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Fri, 8 Sep 2017 11:44:50 -0700 Subject: [PATCH] bdev: fail IO submitted while reset in progress This patch introduces per-channel flags to keep state of information needed in the primary I/O path. Setting/clearing of these flags should only done through an spdk_for_each_channel() call. Currently there is only a RESET_IN_PROGRESS flag defined but more may be added in the future. Signed-off-by: Jim Harris Change-Id: Ia81817e2dabc9997c12beebae72fb129cb5dcf9a Reviewed-on: https://review.gerrithub.io/377828 Tested-by: SPDK Automated Test System Reviewed-by: Dariusz Stojaczyk Reviewed-by: Daniel Verkamp --- lib/bdev/bdev.c | 17 ++++- test/unit/lib/bdev/mt/bdev.c/bdev_ut.c | 102 ++++++++++++++++++++++++- 2 files changed, 117 insertions(+), 2 deletions(-) diff --git a/lib/bdev/bdev.c b/lib/bdev/bdev.c index 6375b23079..c3694c9734 100644 --- a/lib/bdev/bdev.c +++ b/lib/bdev/bdev.c @@ -107,6 +107,8 @@ struct spdk_bdev_desc { TAILQ_ENTRY(spdk_bdev_desc) link; }; +#define BDEV_CH_RESET_IN_PROGRESS (1 << 0) + struct spdk_bdev_channel { struct spdk_bdev *bdev; @@ -126,6 +128,8 @@ struct spdk_bdev_channel { bdev_io_tailq_t queued_resets; + uint32_t flags; + #ifdef SPDK_CONFIG_VTUNE uint64_t start_tsc; uint64_t interval_tsc; @@ -613,7 +617,14 @@ spdk_bdev_io_submit(struct spdk_bdev_io *bdev_io) bdev_ch->io_outstanding++; bdev_io->in_submit_request = true; - bdev->fn_table->submit_request(ch, bdev_io); + if (spdk_likely(bdev_ch->flags == 0)) { + bdev->fn_table->submit_request(ch, bdev_io); + } else if (bdev_ch->flags & BDEV_CH_RESET_IN_PROGRESS) { + spdk_bdev_io_complete(bdev_io, SPDK_BDEV_IO_STATUS_FAILED); + } else { + SPDK_ERRLOG("unknown bdev_ch flag %x found\n", bdev_ch->flags); + spdk_bdev_io_complete(bdev_io, SPDK_BDEV_IO_STATUS_FAILED); + } bdev_io->in_submit_request = false; } @@ -671,6 +682,7 @@ spdk_bdev_channel_create(void *io_device, void *ctx_buf) memset(&ch->stat, 0, sizeof(ch->stat)); ch->io_outstanding = 0; TAILQ_INIT(&ch->queued_resets); + ch->flags = 0; #ifdef SPDK_CONFIG_VTUNE { @@ -1187,6 +1199,8 @@ _spdk_bdev_reset_abort_channel(void *io_device, struct spdk_io_channel *ch, channel = spdk_io_channel_get_ctx(ch); mgmt_channel = spdk_io_channel_get_ctx(channel->mgmt_channel); + channel->flags |= BDEV_CH_RESET_IN_PROGRESS; + _spdk_bdev_abort_buf_io(&mgmt_channel->need_buf_small, channel); _spdk_bdev_abort_buf_io(&mgmt_channel->need_buf_large, channel); } @@ -1228,6 +1242,7 @@ _spdk_bdev_complete_reset_channel(void *io_device, struct spdk_io_channel *_ch, { struct spdk_bdev_channel *ch = spdk_io_channel_get_ctx(_ch); + ch->flags &= ~BDEV_CH_RESET_IN_PROGRESS; if (!TAILQ_EMPTY(&ch->queued_resets)) { _spdk_bdev_channel_start_reset(ch); } diff --git a/test/unit/lib/bdev/mt/bdev.c/bdev_ut.c b/test/unit/lib/bdev/mt/bdev.c/bdev_ut.c index 69986a9daa..f424ddb62c 100644 --- a/test/unit/lib/bdev/mt/bdev.c/bdev_ut.c +++ b/test/unit/lib/bdev/mt/bdev.c/bdev_ut.c @@ -133,6 +133,8 @@ register_bdev(void) g_bdev.bdev.name = "bdev_ut"; g_bdev.bdev.fn_table = &fn_table; g_bdev.bdev.module = SPDK_GET_BDEV_MODULE(bdev_ut); + g_bdev.bdev.blocklen = 4096; + g_bdev.bdev.blockcnt = 1024; spdk_io_device_register(&g_bdev.io_target, stub_create_ch, stub_destroy_ch, sizeof(struct ut_bdev_channel)); @@ -289,6 +291,103 @@ aborted_reset(void) teardown_test(); } +static void +io_during_reset_done(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg) +{ + enum spdk_bdev_io_status *status = cb_arg; + + *status = success ? SPDK_BDEV_IO_STATUS_SUCCESS : SPDK_BDEV_IO_STATUS_FAILED; + spdk_bdev_free_io(bdev_io); +} + +static void +io_during_reset(void) +{ + struct spdk_io_channel *io_ch[2]; + struct spdk_bdev_channel *bdev_ch[2]; + enum spdk_bdev_io_status status0, status1, status_reset; + int rc; + + setup_test(); + + /* + * First test normal case - submit an I/O on each of two channels (with no resets) + * and verify they complete successfully. + */ + set_thread(0); + io_ch[0] = spdk_bdev_get_io_channel(g_desc); + bdev_ch[0] = spdk_io_channel_get_ctx(io_ch[0]); + CU_ASSERT(bdev_ch[0]->flags == 0); + status0 = SPDK_BDEV_IO_STATUS_PENDING; + rc = spdk_bdev_read_blocks(g_desc, io_ch[0], NULL, 0, 1, io_during_reset_done, &status0); + CU_ASSERT(rc == 0); + + set_thread(1); + io_ch[1] = spdk_bdev_get_io_channel(g_desc); + bdev_ch[1] = spdk_io_channel_get_ctx(io_ch[1]); + CU_ASSERT(bdev_ch[1]->flags == 0); + status1 = SPDK_BDEV_IO_STATUS_PENDING; + rc = spdk_bdev_read_blocks(g_desc, io_ch[1], NULL, 0, 1, io_during_reset_done, &status1); + CU_ASSERT(rc == 0); + + poll_threads(); + CU_ASSERT(status0 == SPDK_BDEV_IO_STATUS_PENDING); + CU_ASSERT(status1 == SPDK_BDEV_IO_STATUS_PENDING); + + set_thread(0); + stub_complete_io(); + CU_ASSERT(status0 == SPDK_BDEV_IO_STATUS_SUCCESS); + + set_thread(1); + stub_complete_io(); + CU_ASSERT(status1 == SPDK_BDEV_IO_STATUS_SUCCESS); + + /* + * Now submit a reset, and leave it pending while we submit I?O on two different + * channels. These I/O should be failed by the bdev layer since the reset is in + * progress. + */ + set_thread(0); + status_reset = SPDK_BDEV_IO_STATUS_PENDING; + rc = spdk_bdev_reset(g_desc, io_ch[0], io_during_reset_done, &status_reset); + CU_ASSERT(rc == 0); + + CU_ASSERT(bdev_ch[0]->flags == 0); + CU_ASSERT(bdev_ch[1]->flags == 0); + poll_threads(); + CU_ASSERT(bdev_ch[0]->flags == BDEV_CH_RESET_IN_PROGRESS); + CU_ASSERT(bdev_ch[1]->flags == BDEV_CH_RESET_IN_PROGRESS); + + set_thread(0); + status0 = SPDK_BDEV_IO_STATUS_PENDING; + rc = spdk_bdev_read_blocks(g_desc, io_ch[0], NULL, 0, 1, io_during_reset_done, &status0); + CU_ASSERT(rc == 0); + + set_thread(1); + status1 = SPDK_BDEV_IO_STATUS_PENDING; + rc = spdk_bdev_read_blocks(g_desc, io_ch[1], NULL, 0, 1, io_during_reset_done, &status1); + CU_ASSERT(rc == 0); + + /* + * A reset is in progress so these read I/O should complete with failure. Note that we + * need to poll_threads() since I/O completed inline have their completion deferred. + */ + poll_threads(); + CU_ASSERT(status_reset == SPDK_BDEV_IO_STATUS_PENDING); + CU_ASSERT(status0 == SPDK_BDEV_IO_STATUS_FAILED); + CU_ASSERT(status1 == SPDK_BDEV_IO_STATUS_FAILED); + + set_thread(0); + stub_complete_io(); + spdk_put_io_channel(io_ch[0]); + set_thread(1); + spdk_put_io_channel(io_ch[1]); + poll_threads(); + CU_ASSERT(status_reset == SPDK_BDEV_IO_STATUS_SUCCESS); + + teardown_test(); +} + int main(int argc, char **argv) { @@ -308,7 +407,8 @@ main(int argc, char **argv) if ( CU_add_test(suite, "basic", basic) == NULL || CU_add_test(suite, "put_channel_during_reset", put_channel_during_reset) == NULL || - CU_add_test(suite, "aborted_reset", aborted_reset) == NULL + CU_add_test(suite, "aborted_reset", aborted_reset) == NULL || + CU_add_test(suite, "io_during_reset", io_during_reset) == NULL ) { CU_cleanup_registry(); return CU_get_error();