From 7e0951d7f508e922db6cf4ad6b372914322ddf49 Mon Sep 17 00:00:00 2001 From: Dariusz Stojaczyk Date: Thu, 23 Nov 2017 20:34:44 +0100 Subject: [PATCH] bdev_virtio: ensure thread safety for virtio_dev unregister The io_device_unregister callback might be deferred and called on a different core, but the virtio device has to be released from the same thread that created it. Hence, once the unregister callback is called, it has to send yet another msg to the vdev-owning thread. Since all virtio devices are currently created on the same thread, no mutexes are needed. They will need to be introduced once we publish the API to connect to virtio controllers. This patch also defers bdev_virtio module- -finish until all virtio_devs are destroyed. Change-Id: Iaafceefa1e862b839b5db8c6c4975bf51441a083 Signed-off-by: Dariusz Stojaczyk Reviewed-on: https://review.gerrithub.io/388835 Tested-by: SPDK Automated Test System Reviewed-by: Ben Walker --- lib/bdev/virtio/bdev_virtio.c | 22 +++++++++++++++++++++- lib/bdev/virtio/rte_virtio/virtio.c | 26 ++++++++++++++------------ lib/bdev/virtio/rte_virtio/virtio.h | 12 ++++++++++++ 3 files changed, 47 insertions(+), 13 deletions(-) diff --git a/lib/bdev/virtio/bdev_virtio.c b/lib/bdev/virtio/bdev_virtio.c index 162352ff7..18438c9ae 100644 --- a/lib/bdev/virtio/bdev_virtio.c +++ b/lib/bdev/virtio/bdev_virtio.c @@ -135,6 +135,7 @@ SPDK_BDEV_MODULE_REGISTER(virtio_scsi, bdev_virtio_initialize, bdev_virtio_finis NULL, bdev_virtio_get_ctx_size, NULL) SPDK_BDEV_MODULE_ASYNC_INIT(virtio_scsi) +SPDK_BDEV_MODULE_ASYNC_FINI(virtio_scsi); static struct virtio_req * bdev_virtio_init_io_vreq(struct spdk_io_channel *ch, struct spdk_bdev_io *bdev_io) @@ -1206,6 +1207,14 @@ virtio_scsi_dev_unregister_cb(void *io_device) struct virtio_dev *vdev = io_device; struct virtqueue *vq; struct spdk_ring *send_ring; + struct spdk_thread *thread; + bool finish_module; + + thread = virtio_dev_queue_get_thread(vdev, VIRTIO_SCSI_CONTROLQ); + if (thread != spdk_get_thread()) { + spdk_thread_send_msg(thread, virtio_scsi_dev_unregister_cb, io_device); + return; + } if (virtio_dev_queue_is_acquired(vdev, VIRTIO_SCSI_CONTROLQ)) { vq = vdev->vqs[VIRTIO_SCSI_CONTROLQ]; @@ -1220,6 +1229,13 @@ virtio_scsi_dev_unregister_cb(void *io_device) virtio_dev_reset(vdev); virtio_dev_free(vdev); + + TAILQ_REMOVE(&g_virtio_driver.attached_ctrlrs, vdev, tailq); + finish_module = TAILQ_EMPTY(&g_virtio_driver.attached_ctrlrs); + + if (finish_module) { + spdk_bdev_module_finish_done(); + } } static void @@ -1227,8 +1243,12 @@ bdev_virtio_finish(void) { struct virtio_dev *vdev, *next; + if (TAILQ_EMPTY(&g_virtio_driver.attached_ctrlrs)) { + spdk_bdev_module_finish_done(); + return; + } + TAILQ_FOREACH_SAFE(vdev, &g_virtio_driver.attached_ctrlrs, tailq, next) { - TAILQ_REMOVE(&g_virtio_driver.attached_ctrlrs, vdev, tailq); spdk_io_device_unregister(vdev, virtio_scsi_dev_unregister_cb); } } diff --git a/lib/bdev/virtio/rte_virtio/virtio.c b/lib/bdev/virtio/rte_virtio/virtio.c index d749c8f37..bed1538bb 100644 --- a/lib/bdev/virtio/rte_virtio/virtio.c +++ b/lib/bdev/virtio/rte_virtio/virtio.c @@ -607,30 +607,32 @@ virtio_dev_find_and_acquire_queue(struct virtio_dev *vdev, uint16_t start_index) return i; } -bool -virtio_dev_queue_is_acquired(struct virtio_dev *vdev, uint16_t index) +struct spdk_thread * +virtio_dev_queue_get_thread(struct virtio_dev *vdev, uint16_t index) { - struct virtqueue *vq = NULL; - bool rc; + struct virtqueue *vq; + struct spdk_thread *thread = NULL; if (index >= vdev->max_queues) { SPDK_ERRLOG("given vq index %"PRIu16" exceeds max queue count %"PRIu16"\n", index, vdev->max_queues); - return false; + return NULL; } pthread_mutex_lock(&vdev->mutex); vq = vdev->vqs[index]; - if (vq == NULL) { - SPDK_ERRLOG("virtqueue at index %"PRIu16" is not initialized.\n", index); - pthread_mutex_unlock(&vdev->mutex); - return false; + if (vq != NULL) { + thread = vq->owner_thread; } - - rc = (vq->owner_thread != NULL); pthread_mutex_unlock(&vdev->mutex); - return rc; + return thread; +} + +bool +virtio_dev_queue_is_acquired(struct virtio_dev *vdev, uint16_t index) +{ + return virtio_dev_queue_get_thread(vdev, index) != NULL; } void diff --git a/lib/bdev/virtio/rte_virtio/virtio.h b/lib/bdev/virtio/rte_virtio/virtio.h index c416db0b5..ad7c5ea16 100644 --- a/lib/bdev/virtio/rte_virtio/virtio.h +++ b/lib/bdev/virtio/rte_virtio/virtio.h @@ -333,6 +333,18 @@ int virtio_dev_acquire_queue(struct virtio_dev *vdev, uint16_t index); */ int32_t virtio_dev_find_and_acquire_queue(struct virtio_dev *vdev, uint16_t start_index); +/** + * Get thread that acquired given virtqueue. + * + * This function is thread-safe. + * + * \param vdev vhost device + * \param index index of virtqueue + * \return thread that acquired given virtqueue. If the queue is unused + * or doesn't exist a NULL is returned. + */ +struct spdk_thread *virtio_dev_queue_get_thread(struct virtio_dev *vdev, uint16_t index); + /** * Check if virtqueue with given index is acquired. *