ivampiresp/Spdk
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

io_channel: Fix race between put channel and get channel

Browse Source 
Previously, there was a period of time where the user
had put the last reference to a channel, but when
calling to get a new channel would end up with the
previously created channel and channel destruction
would never execute.

This causes a number of unexpected issues to pop up,
as often the creation and destruction of a channel
is a key event.

Change-Id: Ie68135f6efdf45093a5a227f8c51cd11b971fcff
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/407602
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
This commit is contained in:
Ben Walker 2018-04-13 14:46:05 -07:00 committed by Daniel Verkamp
parent ade146b966
commit 979d9997c9
1 changed files with 28 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

65
lib/util/io_channel.c
View File

@ -56,6 +56,8 @@ struct io_device {
uint32_t for_each_count; uint32_t for_each_count;
TAILQ_ENTRY(io_device) tailq; TAILQ_ENTRY(io_device) tailq;
uint32_t refcnt;
bool unregistered; bool unregistered;
}; };
@ -314,6 +316,7 @@ spdk_io_device_register(void *io_device, spdk_io_channel_create_cb create_cb,
dev->ctx_size = ctx_size; dev->ctx_size = ctx_size;
dev->for_each_count = 0; dev->for_each_count = 0;
dev->unregistered = false; dev->unregistered = false;
dev->refcnt = 0;
pthread_mutex_lock(&g_devlist_mutex); pthread_mutex_lock(&g_devlist_mutex);
TAILQ_FOREACH(tmp, &g_io_devices, tailq) { TAILQ_FOREACH(tmp, &g_io_devices, tailq) {
@ -338,34 +341,20 @@ _finish_unregister(void *arg)
} }
static void static void
_spdk_io_device_attempt_free(struct io_device *dev, struct spdk_io_channel *ch) _spdk_io_device_attempt_free(struct io_device *dev)
{ {
struct spdk_thread *thread;
pthread_mutex_lock(&g_devlist_mutex); pthread_mutex_lock(&g_devlist_mutex);
if (ch != NULL) {
TAILQ_REMOVE(&ch->thread->io_channels, ch, tailq);
}
if (!dev->unregistered) { if (!dev->unregistered) {
pthread_mutex_unlock(&g_devlist_mutex); pthread_mutex_unlock(&g_devlist_mutex);
return; return;
} }
TAILQ_FOREACH(thread, &g_threads, tailq) { if (dev->refcnt > 0) {
/* ch parameter is no longer needed, so use that variable here for iterating. */ pthread_mutex_unlock(&g_devlist_mutex);
TAILQ_FOREACH(ch, &thread->io_channels, tailq) { return;
if (ch->dev == dev) {
/* A channel that references this I/O
* device still exists. Defer deletion
* until it is removed.
*/
pthread_mutex_unlock(&g_devlist_mutex);
return;
}
}
} }
pthread_mutex_unlock(&g_devlist_mutex); pthread_mutex_unlock(&g_devlist_mutex);
if (dev->unregister_cb == NULL) { if (dev->unregister_cb == NULL) {
@ -405,7 +394,7 @@ spdk_io_device_unregister(void *io_device, spdk_io_device_unregister_cb unregist
TAILQ_REMOVE(&g_io_devices, dev, tailq); TAILQ_REMOVE(&g_io_devices, dev, tailq);
pthread_mutex_unlock(&g_devlist_mutex); pthread_mutex_unlock(&g_devlist_mutex);
dev->unregister_thread = spdk_get_thread(); dev->unregister_thread = spdk_get_thread();
_spdk_io_device_attempt_free(dev, NULL); _spdk_io_device_attempt_free(dev);
} }
struct spdk_io_channel * struct spdk_io_channel *
@ -460,12 +449,15 @@ spdk_get_io_channel(void *io_device)
ch->ref = 1; ch->ref = 1;
TAILQ_INSERT_TAIL(&thread->io_channels, ch, tailq); TAILQ_INSERT_TAIL(&thread->io_channels, ch, tailq);
dev->refcnt++;
pthread_mutex_unlock(&g_devlist_mutex); pthread_mutex_unlock(&g_devlist_mutex);
rc = dev->create_cb(io_device, (uint8_t *)ch + sizeof(*ch)); rc = dev->create_cb(io_device, (uint8_t *)ch + sizeof(*ch));
if (rc == -1) { if (rc == -1) {
pthread_mutex_lock(&g_devlist_mutex); pthread_mutex_lock(&g_devlist_mutex);
TAILQ_REMOVE(&ch->thread->io_channels, ch, tailq); TAILQ_REMOVE(&ch->thread->io_channels, ch, tailq);
dev->refcnt--;
free(ch); free(ch);
pthread_mutex_unlock(&g_devlist_mutex); pthread_mutex_unlock(&g_devlist_mutex);
return NULL; return NULL;
@ -480,32 +472,31 @@ _spdk_put_io_channel(void *arg)
struct spdk_io_channel *ch = arg; struct spdk_io_channel *ch = arg;
assert(ch->thread == spdk_get_thread()); assert(ch->thread == spdk_get_thread());
assert(ch->ref == 0);
if (ch->ref == 0) {
SPDK_ERRLOG("ref already zero\n");
return;
}
ch->ref--;
if (ch->ref > 0) {
return;
}
ch->destroy_cb(ch->dev->io_device, spdk_io_channel_get_ctx(ch)); ch->destroy_cb(ch->dev->io_device, spdk_io_channel_get_ctx(ch));
/* pthread_mutex_lock(&g_devlist_mutex);
* _spdk_io_device_attempt_free() will remove the ch from the thread after it ch->dev->refcnt--;
* acquires the g_devlist_mutex lock. pthread_mutex_unlock(&g_devlist_mutex);
*/
_spdk_io_device_attempt_free(ch->dev, ch); _spdk_io_device_attempt_free(ch->dev);
free(ch); free(ch);
} }
void void
spdk_put_io_channel(struct spdk_io_channel *ch) spdk_put_io_channel(struct spdk_io_channel *ch)
{ {
spdk_thread_send_msg(ch->thread, _spdk_put_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);
}
} }
struct spdk_io_channel * struct spdk_io_channel *