diff --git a/include/spdk/thread.h b/include/spdk/thread.h index 678cc752e..27c575b31 100644 --- a/include/spdk/thread.h +++ b/include/spdk/thread.h @@ -267,13 +267,15 @@ void spdk_set_thread(struct spdk_thread *thread); * spdk_poller_register(), and spdk_get_io_channel() calls. May only be called * within an spdk poller or message. * - * All I/O channel references associated with the thread must be released using - * spdk_put_io_channel(), and all active pollers associated with the thread must - * be unregistered using spdk_poller_unregister(), prior to calling this function. + * All I/O channel references associated with the thread must be released + * using spdk_put_io_channel(), and all active pollers associated with the thread + * should be unregistered using spdk_poller_unregister(), prior to calling + * this function. This function will complete these processing. The completion can + * be queried by spdk_thread_is_exited(). * * \param thread The thread to destroy. * - * \return 0 on success, negated errno on failure. + * \return always 0. (return value was deprecated but keep it for ABI compatibility.) */ int spdk_thread_exit(struct spdk_thread *thread); diff --git a/lib/event/reactor.c b/lib/event/reactor.c index e9a443b99..913bc4b89 100644 --- a/lib/event/reactor.c +++ b/lib/event/reactor.c @@ -366,7 +366,6 @@ _spdk_reactor_run(void *arg) struct spdk_thread *thread; struct spdk_lw_thread *lw_thread, *tmp; char thread_name[32]; - int rc __attribute__((unused)); SPDK_NOTICELOG("Reactor started on core %u\n", reactor->lcore); @@ -386,18 +385,25 @@ _spdk_reactor_run(void *arg) } } - TAILQ_FOREACH_SAFE(lw_thread, &reactor->threads, link, tmp) { + TAILQ_FOREACH(lw_thread, &reactor->threads, link) { thread = spdk_thread_get_from_ctx(lw_thread); - TAILQ_REMOVE(&reactor->threads, lw_thread, link); - assert(reactor->thread_count > 0); - reactor->thread_count--; spdk_set_thread(thread); - rc = spdk_thread_exit(thread); - assert(rc == 0); - while (!spdk_thread_is_exited(thread)) { - spdk_thread_poll(thread, 0, 0); + spdk_thread_exit(thread); + } + + while (!TAILQ_EMPTY(&reactor->threads)) { + TAILQ_FOREACH_SAFE(lw_thread, &reactor->threads, link, tmp) { + thread = spdk_thread_get_from_ctx(lw_thread); + spdk_set_thread(thread); + if (spdk_thread_is_exited(thread)) { + TAILQ_REMOVE(&reactor->threads, lw_thread, link); + assert(reactor->thread_count > 0); + reactor->thread_count--; + spdk_thread_destroy(thread); + } else { + spdk_thread_poll(thread, 0, 0); + } } - spdk_thread_destroy(thread); } return 0; diff --git a/lib/thread/thread.c b/lib/thread/thread.c index 030893199..31f9c8645 100644 --- a/lib/thread/thread.c +++ b/lib/thread/thread.c @@ -316,7 +316,7 @@ spdk_set_thread(struct spdk_thread *thread) tls_thread = thread; } -static int +static void _spdk_thread_exit(struct spdk_thread *thread) { struct spdk_poller *poller; @@ -324,36 +324,37 @@ _spdk_thread_exit(struct spdk_thread *thread) TAILQ_FOREACH(poller, &thread->active_pollers, tailq) { if (poller->state != SPDK_POLLER_STATE_UNREGISTERED) { - SPDK_ERRLOG("thread %s still has active poller %s\n", - thread->name, poller->name); - return -EBUSY; + SPDK_INFOLOG(SPDK_LOG_THREAD, + "thread %s still has active poller %s\n", + thread->name, poller->name); + return; } } TAILQ_FOREACH(poller, &thread->timed_pollers, tailq) { if (poller->state != SPDK_POLLER_STATE_UNREGISTERED) { - SPDK_ERRLOG("thread %s still has active timed poller %s\n", - thread->name, poller->name); - return -EBUSY; + SPDK_INFOLOG(SPDK_LOG_THREAD, + "thread %s still has active timed poller %s\n", + thread->name, poller->name); + return; } } TAILQ_FOREACH(poller, &thread->paused_pollers, tailq) { - SPDK_ERRLOG("thread %s still has paused poller %s\n", - thread->name, poller->name); - return -EBUSY; + SPDK_INFOLOG(SPDK_LOG_THREAD, + "thread %s still has paused poller %s\n", + thread->name, poller->name); + return; } TAILQ_FOREACH(ch, &thread->io_channels, tailq) { - if (ch->ref != 0) { - SPDK_ERRLOG("thread %s still has active channel for io_device %s\n", - thread->name, ch->dev->name); - return -EBUSY; - } + SPDK_INFOLOG(SPDK_LOG_THREAD, + "thread %s still has channel for io_device %s\n", + thread->name, ch->dev->name); + return; } thread->state = SPDK_THREAD_STATE_EXITED; - return 0; } int @@ -370,7 +371,8 @@ spdk_thread_exit(struct spdk_thread *thread) return 0; } - return _spdk_thread_exit(thread); + thread->state = SPDK_THREAD_STATE_EXITING; + return 0; } bool @@ -661,6 +663,10 @@ spdk_thread_poll(struct spdk_thread *thread, uint32_t max_msgs, uint64_t now) rc = _spdk_thread_poll(thread, max_msgs, now); + if (spdk_unlikely(thread->state == SPDK_THREAD_STATE_EXITING)) { + _spdk_thread_exit(thread); + } + _spdk_thread_update_stats(thread, spdk_get_ticks(), now, rc); tls_thread = orig_thread; diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c index 7c89249e9..b717a16dc 100644 --- a/lib/vhost/vhost.c +++ b/lib/vhost/vhost.c @@ -590,10 +590,7 @@ vhost_parse_core_mask(const char *mask, struct spdk_cpuset *cpumask) static void vhost_dev_thread_exit(void *arg1) { - int rc __attribute__((unused)); - - rc = spdk_thread_exit(spdk_get_thread()); - assert(rc == 0); + spdk_thread_exit(spdk_get_thread()); } int diff --git a/test/common/lib/ut_multithread.c b/test/common/lib/ut_multithread.c index 1a059a13d..30b78f74d 100644 --- a/test/common/lib/ut_multithread.c +++ b/test/common/lib/ut_multithread.c @@ -102,20 +102,33 @@ allocate_threads(int num_threads) void free_threads(void) { - uint32_t i; + uint32_t i, num_threads; struct spdk_thread *thread; - int rc __attribute__((unused)); for (i = 0; i < g_ut_num_threads; i++) { set_thread(i); thread = g_ut_threads[i].thread; - rc = spdk_thread_exit(thread); - assert(rc == 0); - while (!spdk_thread_is_exited(thread)) { - spdk_thread_poll(thread, 0, 0); + spdk_thread_exit(thread); + } + + num_threads = g_ut_num_threads; + + while (num_threads != 0) { + for (i = 0; i < g_ut_num_threads; i++) { + set_thread(i); + thread = g_ut_threads[i].thread; + if (thread == NULL) { + continue; + } + + if (spdk_thread_is_exited(thread)) { + g_ut_threads[i].thread = NULL; + num_threads--; + spdk_thread_destroy(thread); + } else { + spdk_thread_poll(thread, 0, 0); + } } - spdk_thread_destroy(thread); - g_ut_threads[i].thread = NULL; } g_ut_num_threads = 0; diff --git a/test/unit/lib/thread/thread.c/thread_ut.c b/test/unit/lib/thread/thread.c/thread_ut.c index 32f76b591..b1b885f24 100644 --- a/test/unit/lib/thread/thread.c/thread_ut.c +++ b/test/unit/lib/thread/thread.c/thread_ut.c @@ -805,41 +805,45 @@ thread_exit(void) { struct spdk_thread *thread; struct spdk_io_channel *ch; - struct spdk_poller *poller; + struct spdk_poller *poller1, *poller2; void *ctx; - bool done1 = false, done2 = false, poller_run = false; + bool done1 = false, done2 = false, poller1_run = false, poller2_run = false; int rc __attribute__((unused)); - allocate_threads(6); + allocate_threads(3); - /* Test all pending messages are reaped for the thread marked as exited. */ + /* Test if all pending messages are reaped for the exiting thread, and the + * thread moves to the exited state. + */ set_thread(0); thread = spdk_get_thread(); /* Sending message to thread 0 will be accepted. */ - set_thread(1); rc = spdk_thread_send_msg(thread, send_msg_cb, &done1); CU_ASSERT(rc == 0); CU_ASSERT(!done1); - /* Mark thread 0 as exited. */ - set_thread(0); + /* Move thread 0 to the exiting state. */ spdk_thread_exit(thread); - /* Sending message to thread 0 will be rejected. */ - set_thread(1); - rc = spdk_thread_send_msg(thread, send_msg_cb, &done2); - CU_ASSERT(rc == -EIO); + CU_ASSERT(spdk_thread_is_exited(thread) == false); - /* Thread 0 will reap pending message. */ + /* Sending message to thread 0 will be still accepted. */ + rc = spdk_thread_send_msg(thread, send_msg_cb, &done2); + CU_ASSERT(rc == 0); + + /* Thread 0 will reap pending messages. */ poll_thread(0); CU_ASSERT(done1 == true); - CU_ASSERT(done2 == false); + CU_ASSERT(done2 == true); - /* Test releasing I/O channel is reaped even after the thread is marked - * as exited. + /* Thread 0 will move to the exited state. */ + CU_ASSERT(spdk_thread_is_exited(thread) == true); + + /* Test releasing I/O channel is reaped even after the thread moves to + * the exiting state */ - set_thread(2); + set_thread(1); spdk_io_device_register(&g_device1, create_cb_1, destroy_cb_1, sizeof(g_ctx1), NULL); @@ -857,76 +861,63 @@ thread_exit(void) thread = spdk_get_thread(); spdk_thread_exit(thread); - /* Thread will not be able to get I/O channel after it is marked as exited. */ - ch = spdk_get_io_channel(&g_device1); - CU_ASSERT(ch == NULL); - - poll_threads(); - CU_ASSERT(g_destroy_cb_calls == 1); - - spdk_io_device_unregister(&g_device1, NULL); - poll_threads(); - - /* Test 2nd spdk_thread_exit() call is ignored. */ - set_thread(3); - - thread = spdk_get_thread(); - - CU_ASSERT(spdk_thread_exit(thread) == 0); - CU_ASSERT(spdk_thread_exit(thread) == 0); - - /* Test if spdk_thread_exit() fails when there is any registered poller, - * and if no poller is executed after the thread is marked as exited. + /* Thread 1 will not move to the exited state yet because I/O channel release + * does not complete yet. */ - set_thread(4); - thread = spdk_get_thread(); - - poller = spdk_poller_register(poller_run_done, &poller_run, 0); - CU_ASSERT(poller != NULL); - - CU_ASSERT(spdk_thread_exit(thread) == -EBUSY); - - spdk_poller_pause(poller); - - CU_ASSERT(spdk_thread_exit(thread) == -EBUSY); - - poll_threads(); - - CU_ASSERT(spdk_thread_exit(thread) == -EBUSY); - - spdk_poller_unregister(&poller); - - CU_ASSERT(spdk_thread_exit(thread) == 0); - - poll_threads(); - - CU_ASSERT(poller_run == false); - - /* Test if spdk_thread_exit() fails when there is any active I/O channel. */ - set_thread(5); - thread = spdk_get_thread(); - - spdk_io_device_register(&g_device1, create_cb_1, destroy_cb_1, sizeof(g_ctx1), NULL); + CU_ASSERT(spdk_thread_is_exited(thread) == false); + /* Thread 1 will be able to get the another reference of I/O channel + * even after the thread moves to the exiting state. + */ g_create_cb_calls = 0; ch = spdk_get_io_channel(&g_device1); - CU_ASSERT(g_create_cb_calls == 1); - CU_ASSERT(ch != NULL); - CU_ASSERT(spdk_thread_exit(thread) == -EBUSY); + CU_ASSERT(g_create_cb_calls == 0); + SPDK_CU_ASSERT_FATAL(ch != NULL); + + ctx = spdk_io_channel_get_ctx(ch); + CU_ASSERT(*(uint64_t *)ctx == g_ctx1); - g_destroy_cb_calls = 0; spdk_put_io_channel(ch); - CU_ASSERT(g_destroy_cb_calls == 0); - - CU_ASSERT(spdk_thread_exit(thread) == 0); poll_threads(); CU_ASSERT(g_destroy_cb_calls == 1); - spdk_io_device_unregister(&g_device1, NULL); + /* Thread 1 will move to the exited state after I/O channel is released. + * are released. + */ + CU_ASSERT(spdk_thread_is_exited(thread) == true); - CU_ASSERT(TAILQ_EMPTY(&thread->io_channels)); + spdk_io_device_unregister(&g_device1, NULL); + poll_threads(); + + /* Test if unregistering poller is reaped for the exiting thread, and the + * thread moves to the exited thread. + */ + set_thread(2); + thread = spdk_get_thread(); + + poller1 = spdk_poller_register(poller_run_done, &poller1_run, 0); + CU_ASSERT(poller1 != NULL); + + spdk_poller_unregister(&poller1); + + spdk_thread_exit(thread); + + poller2 = spdk_poller_register(poller_run_done, &poller2_run, 0); + + poll_threads(); + + CU_ASSERT(poller1_run == false); + CU_ASSERT(poller2_run == true); + + CU_ASSERT(spdk_thread_is_exited(thread) == false); + + spdk_poller_unregister(&poller2); + + poll_threads(); + + CU_ASSERT(spdk_thread_is_exited(thread) == true); free_threads(); } @@ -1012,6 +1003,8 @@ thread_update_stats(void) spdk_poller_unregister(&poller); + MOCK_CLEAR(spdk_get_ticks); + free_threads(); }