Commit 648d6cd5 authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Tomasz Zawadzki
Browse files

lib/thread: Fail spdk_thread_exit() if thread has any registered poller 



This is a preparation to support voluntary thread termination by
calling spdk_thread_exit().

Change spdk_thread_exit() to return -EBUSY if the thread has any
registered poller. We enforce all pollers including paused poller
are unresitered before the thread is marked as exited.

By this change, a bug was found in reactor_perf test tool. Fix it
by adding spdk_poller_unregister() and add the g_ prefix to avoid
future potential errors.

Signed-off-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: If7f40357c9a6f4101b3998ea0da3cc46cc435031
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/487


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@mellanox.com>
parent 70ec7287

include/spdk/thread.h

+2 −1
Original line number Diff line number Diff line
@@ -225,7 +225,8 @@ void spdk_set_thread(struct spdk_thread *thread); 
 * 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() prior to calling this function.

 * spdk_put_io_channel(), and all active pollers associated with the thread must

 * be unregistered using spdk_poller_unregister(), prior to calling this function.

 *

 * \param thread The thread to destroy.

 *

lib/thread/thread.c

+24 −0
Original line number Diff line number Diff line
@@ -348,6 +348,8 @@ spdk_set_thread(struct spdk_thread *thread) 
int

spdk_thread_exit(struct spdk_thread *thread)

{

	struct spdk_poller *poller;



	SPDK_DEBUGLOG(SPDK_LOG_THREAD, "Exit thread %s\n", thread->name);



	assert(tls_thread == thread);
@@ -358,6 +360,28 @@ spdk_thread_exit(struct spdk_thread *thread) 
		return -EINVAL;

	}



	TAILQ_FOREACH(poller, &thread->active_pollers, tailq) {

		if (poller->state != SPDK_POLLER_STATE_UNREGISTERED) {

			SPDK_ERRLOG("thread %s still has active poller %p\n",

				    thread->name, poller);

			return -EBUSY;

		}

	}



	TAILQ_FOREACH(poller, &thread->timed_pollers, tailq) {

		if (poller->state != SPDK_POLLER_STATE_UNREGISTERED) {

			SPDK_ERRLOG("thread %s still has active timed poller %p\n",

				    thread->name, poller);

			return -EBUSY;

		}

	}



	TAILQ_FOREACH(poller, &thread->paused_pollers, tailq) {

		SPDK_ERRLOG("thread %s still has paused poller %p\n",

			    thread->name, poller);

		return -EBUSY;

	}



	thread->exit = true;

	return 0;

}

test/event/reactor_perf/reactor_perf.c

+5 −4
Original line number Diff line number Diff line
@@ -40,13 +40,14 @@ 


static int g_time_in_sec;

static int g_queue_depth;

static struct spdk_poller *test_end_poller;

static struct spdk_poller *g_test_end_poller;

static uint64_t g_call_count = 0;



static int

__test_end(void *arg)

{

	printf("test_end\n");

	spdk_poller_unregister(&g_test_end_poller);

	spdk_app_stop(0);

	return -1;

}
@@ -71,7 +72,7 @@ test_start(void *arg1) 
	printf("test_start\n");



	/* Register a poller that will stop the test after the time has elapsed. */

	test_end_poller = spdk_poller_register(__test_end, NULL,

	g_test_end_poller = spdk_poller_register(__test_end, NULL,

			    g_time_in_sec * 1000000ULL);



	for (i = 0; i < g_queue_depth; i++) {
@@ -84,7 +85,7 @@ test_cleanup(void) 
{

	printf("test_abort\n");



	spdk_poller_unregister(&test_end_poller);

	spdk_poller_unregister(&g_test_end_poller);

	spdk_app_stop(0);

}

test/unit/lib/thread/thread.c/thread_ut.c

+30 −2
Original line number Diff line number Diff line
@@ -750,11 +750,12 @@ thread_exit(void) 
{

	struct spdk_thread *thread;

	struct spdk_io_channel *ch;

	struct spdk_poller *poller;

	void *ctx;

	bool done1 = false, done2 = false;

	bool done1 = false, done2 = false, poller_run = false;

	int rc __attribute__((unused));



	allocate_threads(4);

	allocate_threads(5);



	/* Test all pending messages are reaped for the thread marked as exited. */

	set_thread(0);
@@ -819,6 +820,33 @@ thread_exit(void) 
	CU_ASSERT(spdk_thread_exit(thread) == 0);

	CU_ASSERT(spdk_thread_exit(thread) == -EINVAL);



	/* Test if spdk_thread_exit() fails when there is any not-unregistered poller,

	 * and if no poller is executed after the thread is marked as exited.

	 */

	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);



	free_threads();

}