From a7a2e2721adf82351d2f4fa7c96aca9bf704dae5 Mon Sep 17 00:00:00 2001 From: Ben Walker Date: Wed, 5 Sep 2018 16:20:22 -0700 Subject: [PATCH] test/bdev: Add a unit test to expose a race condition There is a race condition with the following sequence: spdk_bdev_open() spdk_bdev_unregister() <-- starts deferred message spdk_bdev_close() deferred message runs, crashes Change-Id: I81fbced0849949cfb2dae5a7cc6f60c9685a8885 Signed-off-by: Ben Walker Reviewed-on: https://review.gerrithub.io/424739 Chandler-Test-Pool: SPDK Automated Test System Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Shuhei Matsumoto --- test/unit/lib/bdev/mt/bdev.c/bdev_ut.c | 60 ++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) 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 e7ee0f630..4d4b455e8 100644 --- a/test/unit/lib/bdev/mt/bdev.c/bdev_ut.c +++ b/test/unit/lib/bdev/mt/bdev.c/bdev_ut.c @@ -328,6 +328,65 @@ basic(void) CU_ASSERT(g_fini_start_called == true); } +static void +_bdev_removed(void *done) +{ + *(bool *)done = true; +} + +static void +_bdev_unregistered(void *done, int rc) +{ + CU_ASSERT(rc == 0); + *(bool *)done = true; +} + +static void +unregister_and_close(void) +{ + bool done, remove_notify; + struct spdk_bdev_desc *desc; + + setup_test(); + set_thread(0); + + /* setup_test() automatically opens the bdev, + * but this test needs to do that in a different + * way. */ + spdk_bdev_close(g_desc); + poll_threads(); + + remove_notify = false; + spdk_bdev_open(&g_bdev.bdev, true, _bdev_removed, &remove_notify, &desc); + CU_ASSERT(remove_notify == false); + CU_ASSERT(desc != NULL); + + /* There is an open descriptor on the device. Unregister it, + * which can't proceed until the descriptor is closed. */ + done = false; + spdk_bdev_unregister(&g_bdev.bdev, _bdev_unregistered, &done); + /* No polling has occurred, so neither of these should execute */ + CU_ASSERT(remove_notify == false); + CU_ASSERT(done == false); + + /* Prior to the unregister completing, close the descriptor */ + spdk_bdev_close(desc); + + /* Poll the threads to allow all events to be processed */ + poll_threads(); + + /* Remove notify should not have been called because the + * descriptor is already closed. */ + CU_ASSERT(remove_notify == false); + + /* The unregister should have completed */ + CU_ASSERT(done == true); + + spdk_bdev_finish(finish_cb, NULL); + poll_threads(); + free_threads(); +} + static void reset_done(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg) { @@ -1245,6 +1304,7 @@ main(int argc, char **argv) if ( CU_add_test(suite, "basic", basic) == NULL || + CU_add_test(suite, "unregister_and_close", unregister_and_close) == NULL || CU_add_test(suite, "basic_qos", basic_qos) == NULL || CU_add_test(suite, "put_channel_during_reset", put_channel_during_reset) == NULL || CU_add_test(suite, "aborted_reset", aborted_reset) == NULL ||