io_channel: do not unlock/relock after removing io_ch from thread

We must keep the lock held after removing the io_ch from the thread
before iterating the list of threads to check for other channels
referencing this thread.  Without it, it is possible for two different
threads to think they are the last thread holding a reference to
an unregistered device, and both will call the unregister_cb and free
the dev memory.

Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: Ifcc18752937731722fefc6e2161ee41443c4fb86

Reviewed-on: https://review.gerrithub.io/404410
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
This commit is contained in:
Jim Harris 2018-03-19 11:53:16 -07:00 committed by Changpeng Liu
parent 09fb50530e
commit 1b41df0558

View File

@ -328,19 +328,23 @@ spdk_io_device_register(void *io_device, spdk_io_channel_create_cb create_cb,
}
static void
_spdk_io_device_attempt_free(struct io_device *dev)
_spdk_io_device_attempt_free(struct io_device *dev, struct spdk_io_channel *ch)
{
struct spdk_thread *thread;
struct spdk_io_channel *ch;
pthread_mutex_lock(&g_devlist_mutex);
if (ch != NULL) {
TAILQ_REMOVE(&ch->thread->io_channels, ch, tailq);
}
if (!dev->unregistered) {
pthread_mutex_unlock(&g_devlist_mutex);
return;
}
TAILQ_FOREACH(thread, &g_threads, tailq) {
/* ch parameter is no longer needed, so use that variable here for iterating. */
TAILQ_FOREACH(ch, &thread->io_channels, tailq) {
if (ch->dev == dev) {
/* A channel that references this I/O
@ -389,7 +393,7 @@ spdk_io_device_unregister(void *io_device, spdk_io_device_unregister_cb unregist
dev->unregistered = true;
TAILQ_REMOVE(&g_io_devices, dev, tailq);
pthread_mutex_unlock(&g_devlist_mutex);
_spdk_io_device_attempt_free(dev);
_spdk_io_device_attempt_free(dev, NULL);
}
struct spdk_io_channel *
@ -478,11 +482,11 @@ _spdk_put_io_channel(void *arg)
ch->destroy_cb(ch->dev->io_device, spdk_io_channel_get_ctx(ch));
pthread_mutex_lock(&g_devlist_mutex);
TAILQ_REMOVE(&ch->thread->io_channels, ch, tailq);
pthread_mutex_unlock(&g_devlist_mutex);
_spdk_io_device_attempt_free(ch->dev);
/*
* _spdk_io_device_attempt_free() will remove the ch from the thread after it
* acquires the g_devlist_mutex lock.
*/
_spdk_io_device_attempt_free(ch->dev, ch);
free(ch);
}