Commit 9ddf6438 authored by Jim Harris's avatar Jim Harris Committed by Changpeng Liu
Browse files

thread: don't immediately remove channel from list when put 



When spdk_put_io_channel is called, if its the last reference,
we defer actual destruction of the channel, so that code
in the same context which may be referring to the channel
doesn't crash.

But it is possible that an io_channel for that same io_device
could be requested before the deferred message is processed.
This would result in a second io_channel being created for
that device on the same thread.

To avoid this case, don't immediately remove the channel from
the list when the last reference is put.  When the deferred
message is processed, if additional references were allocated
in the meantime, don't destroy the channel.

Signed-off-by: default avatarJim Harris <james.r.harris@intel.com>
Signed-off-by: default avatarZiye Yang <ziye.yang@intel.com>
Change-Id: Idb8d4705fda0eb9c338e4960430e04edbe537e05
Reviewed-on: https://review.gerrithub.io/418878


Tested-by: default avatarSPDK Automated Test System <sys_sgsw@intel.com>
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: default avatarGangCao <gang.cao@intel.com>
Reviewed-by: default avatarPawel Wodkowski <pawelx.wodkowski@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
parent 706c57bf

lib/bdev/bdev.c

+8 −3
Original line number Diff line number Diff line
@@ -1100,10 +1100,11 @@ spdk_bdev_io_submit(struct spdk_bdev_io *bdev_io) 
	struct spdk_bdev *bdev = bdev_io->bdev;

	struct spdk_thread *thread = spdk_io_channel_get_thread(bdev_io->internal.ch->channel);



	assert(thread != NULL);

	assert(bdev_io->internal.status == SPDK_BDEV_IO_STATUS_PENDING);



	if (bdev_io->internal.ch->flags & BDEV_CH_QOS_ENABLED) {

		if (thread == bdev->internal.qos->thread) {

		if ((thread == bdev->internal.qos->thread) || !bdev->internal.qos->thread) {

			_spdk_bdev_io_submit(bdev_io);

		} else {

			bdev_io->internal.io_submit_ch = bdev_io->internal.ch;
@@ -1487,8 +1488,12 @@ spdk_bdev_qos_destroy(struct spdk_bdev *bdev) 


	bdev->internal.qos = new_qos;



	if (old_qos->thread == NULL) {

		free(old_qos);

	} else {

		spdk_thread_send_msg(old_qos->thread, spdk_bdev_qos_channel_destroy,

				     old_qos);

	}



	/* It is safe to continue with destroying the bdev even though the QoS channel hasn't

	 * been destroyed yet. The destruction path will end up waiting for the final

lib/thread/thread.c

+14 −6
Original line number Diff line number Diff line
@@ -490,8 +490,21 @@ _spdk_put_io_channel(void *arg) 
	bool do_remove_dev = true;



	assert(ch->thread == spdk_get_thread());

	assert(ch->ref == 0);



	if (ch->ref > 0) {

		/*

		 * Another reference to the associated io_device was requested

		 *  after this message was sent but before it had a chance to

		 *  execute.

		 */

		return;

	}



	pthread_mutex_lock(&g_devlist_mutex);

	TAILQ_REMOVE(&ch->thread->io_channels, ch, tailq);

	pthread_mutex_unlock(&g_devlist_mutex);



	/* Don't hold the devlist mutex while the destroy_cb is called. */

	ch->destroy_cb(ch->dev->io_device, spdk_io_channel_get_ctx(ch));



	pthread_mutex_lock(&g_devlist_mutex);
@@ -519,11 +532,6 @@ spdk_put_io_channel(struct spdk_io_channel *ch) 
	ch->ref--;



	if (ch->ref == 0) {

		/* If this was the last reference, remove the channel from the list */

		pthread_mutex_lock(&g_devlist_mutex);

		TAILQ_REMOVE(&ch->thread->io_channels, ch, tailq);

		pthread_mutex_unlock(&g_devlist_mutex);



		spdk_thread_send_msg(ch->thread, _spdk_put_io_channel, ch);

	}

}