Commit f8d312c2 authored by Vasilii Ivanov's avatar Vasilii Ivanov Committed by Jim Harris
Browse files

lib/thread: check unregistered flag after failed create_cb for channel 



g_devlist_mutex is not hold during invokation of create_cb during
getting channel, because of it spdk_io_device_unregister can execute simoustanly.
However, device would not be destroyed as just created channel is holding refcnt.
If create_cb is successful - device will be destroyed lately after put_io_channel,
but if it fails - we do not check whether it is needed to destroy dev.
Add missed check and device destruction.

Change-Id: I5e81542f9aafefa5ee843d67435403d0f9aca235
Signed-off-by: default avatarVasilii Ivanov <iwanovvvasilij@gmail.com>
Reviewed-on: https://review.spdk.io/c/spdk/spdk/+/25601


Reviewed-by: default avatarJim Harris <jim.harris@nvidia.com>
Reviewed-by: default avatarShuhei Matsumoto <smatsumoto@nvidia.com>
Tested-by: default avatarSPDK Automated Test System <spdkbot@gmail.com>
Community-CI: Mellanox Build Bot
Reviewed-by: default avatarKonrad Sztyber <ksztyber@nvidia.com>
parent c3878216

lib/thread/thread.c

+8 −0
Original line number Diff line number Diff line
@@ -2349,6 +2349,7 @@ spdk_get_io_channel(void *io_device) 
	struct spdk_thread *thread;

	struct io_device *dev;

	int rc;

	bool do_remove_dev = false;



	pthread_mutex_lock(&g_devlist_mutex);

	dev = io_device_get(io_device);
@@ -2417,7 +2418,14 @@ spdk_get_io_channel(void *io_device) 
		free(ch);

		SPDK_ERRLOG("could not create io_channel for io_device %s (%p): %s (rc=%d)\n",

			    dev->name, io_device, spdk_strerror(-rc), rc);

		if (dev->unregistered && dev->refcnt == 0) {

			/* During invokation of create_cb dev was unregistered, but was not removed due to refcnt */

			do_remove_dev = true;

		}

		pthread_mutex_unlock(&g_devlist_mutex);

		if (do_remove_dev) {

			io_device_free(dev);

		}

		return NULL;

	}

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

+35 −0
Original line number Diff line number Diff line
@@ -2169,6 +2169,40 @@ poller_get_stats(void) 
	free_threads();

}



static bool g_unregistered;



static void

confirm_unregistered_cb(void *io_device)

{

	g_unregistered = true;

}



static int

failing_create_cb(void *io_device, void *ctx_buf)

{

	spdk_io_device_unregister(io_device, &confirm_unregistered_cb);

	return -1;

}



static void

channel_create_cb_failed(void)

{

	uint64_t device;

	struct spdk_io_channel *ch;



	allocate_threads(1);

	set_thread(0);



	g_unregistered = false;

	spdk_io_device_register(&device, failing_create_cb, destroy_cb, sizeof(uint64_t), NULL);

	ch = spdk_get_io_channel(&device);



	poll_threads();

	CU_ASSERT(ch == NULL);

	CU_ASSERT(RB_EMPTY(&g_io_devices));

	CU_ASSERT(g_unregistered);

	free_threads();

}



int

main(int argc, char **argv)
@@ -2205,6 +2239,7 @@ main(int argc, char **argv) 
	CU_ADD_TEST(suite, poller_get_state_str);

	CU_ADD_TEST(suite, poller_get_period_ticks);

	CU_ADD_TEST(suite, poller_get_stats);

	CU_ADD_TEST(suite, channel_create_cb_failed);



	num_failures = spdk_ut_run_tests(argc, argv, NULL);

	CU_cleanup_registry();