Commit 93cfe874 authored by Jim Harris's avatar Jim Harris
Browse files

lib/util: handle channel being removed during spdk_for_each_channel 



A channel may have been deleted between when a message was sent to
a thread and when it gets executed on that thread.  So we must look
for the channel when the messages gets executed - if it's not found,
just continue to the next thread(s).

Signed-off-by: default avatarJim Harris <james.r.harris@intel.com>
Change-Id: Ib7e596f11f287c6be521ba729d33378f0a825c7e

Reviewed-on: https://review.gerrithub.io/378002


Reviewed-by: default avatarDariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Tested-by: default avatarSPDK Automated Test System <sys_sgsw@intel.com>
parent 32a87ed9

lib/util/io_channel.c

+15 −5
Original line number Diff line number Diff line
@@ -404,7 +404,6 @@ struct call_channel { 
	void *ctx;



	struct spdk_thread *cur_thread;

	struct spdk_io_channel *cur_ch;



	struct spdk_thread *orig_thread;

	spdk_channel_for_each_cpl cpl;
@@ -429,9 +428,22 @@ _call_channel(void *ctx) 
	struct spdk_io_channel *ch;



	thread = ch_ctx->cur_thread;

	ch = ch_ctx->cur_ch;

	pthread_mutex_lock(&g_devlist_mutex);

	TAILQ_FOREACH(ch, &thread->io_channels, tailq) {

		if (ch->dev->io_device == ch_ctx->io_device) {

			break;

		}

	}

	pthread_mutex_unlock(&g_devlist_mutex);



	/*

	 * It is possible that the channel was deleted before this

	 *  message had a chance to execute.  If so, skip calling

	 *  the fn() on this thread.

	 */

	if (ch != NULL) {

		ch_ctx->fn(ch_ctx->io_device, ch, ch_ctx->ctx);

	}



	pthread_mutex_lock(&g_devlist_mutex);

	thread = TAILQ_NEXT(thread, tailq);
@@ -439,7 +451,6 @@ _call_channel(void *ctx) 
		TAILQ_FOREACH(ch, &thread->io_channels, tailq) {

			if (ch->dev->io_device == ch_ctx->io_device) {

				ch_ctx->cur_thread = thread;

				ch_ctx->cur_ch = ch;

				pthread_mutex_unlock(&g_devlist_mutex);

				spdk_thread_send_msg(thread, _call_channel, ch_ctx);

				return;
@@ -479,7 +490,6 @@ spdk_for_each_channel(void *io_device, spdk_channel_msg fn, void *ctx, 
		TAILQ_FOREACH(ch, &thread->io_channels, tailq) {

			if (ch->dev->io_device == io_device) {

				ch_ctx->cur_thread = thread;

				ch_ctx->cur_ch = ch;

				pthread_mutex_unlock(&g_devlist_mutex);

				spdk_thread_send_msg(thread, _call_channel, ch_ctx);

				return;

test/unit/lib/util/io_channel.c/io_channel_ut.c

+74 −0
Original line number Diff line number Diff line
@@ -97,6 +97,79 @@ thread_send_msg(void) 
	free_threads();

}



static int

channel_create(void *io_device, void *ctx_buf)

{

	return 0;

}



static void

channel_destroy(void *io_device, void *ctx_buf)

{

}



static void

channel_msg(void *io_device, struct spdk_io_channel *ch, void *ctx)

{

	int *count = spdk_io_channel_get_ctx(ch);



	(*count)++;

}



static void

channel_cpl(void *io_device, void *ctx)

{

}



static void

for_each_channel_remove(void)

{

	struct spdk_io_channel *ch0, *ch1, *ch2;

	int io_target;

	int count = 0;



	allocate_threads(3);

	spdk_io_device_register(&io_target, channel_create, channel_destroy, sizeof(int));

	set_thread(0);

	ch0 = spdk_get_io_channel(&io_target);

	set_thread(1);

	ch1 = spdk_get_io_channel(&io_target);

	set_thread(2);

	ch2 = spdk_get_io_channel(&io_target);



	/*

	 * Test that io_channel handles the case where we start to iterate through

	 *  the channels, and during the iteration, one of the channels is deleted.

	 * This is done in some different and sometimes non-intuitive orders, because

	 *  some operations are deferred and won't execute until their threads are

	 *  polled.

	 *

	 * Case #1: Put the I/O channel before spdk_for_each_channel.

	 */

	set_thread(0);

	spdk_put_io_channel(ch0);

	spdk_for_each_channel(&io_target, channel_msg, &count, channel_cpl);

	poll_threads();



	/*

	 * Case #2: Put the I/O channel after spdk_for_each_channel, but before

	 *  thread 0 is polled.

	 */

	ch0 = spdk_get_io_channel(&io_target);

	spdk_for_each_channel(&io_target, channel_msg, &count, channel_cpl);

	spdk_put_io_channel(ch0);

	poll_threads();



	set_thread(1);

	spdk_put_io_channel(ch1);

	set_thread(2);

	spdk_put_io_channel(ch2);

	spdk_io_device_unregister(&io_target, NULL);

	poll_threads();



	free_threads();

}



static void

thread_name(void)

{
@@ -244,6 +317,7 @@ main(int argc, char **argv) 
	if (

		CU_add_test(suite, "thread_alloc", thread_alloc) == NULL ||

		CU_add_test(suite, "thread_send_msg", thread_send_msg) == NULL ||

		CU_add_test(suite, "for_each_channel_remove", for_each_channel_remove) == NULL ||

		CU_add_test(suite, "thread_name", thread_name) == NULL ||

		CU_add_test(suite, "channel", channel) == NULL

	) {