Commit f67dddc6 authored by Dariusz Stojaczyk's avatar Dariusz Stojaczyk Committed by Jim Harris
Browse files

bdev_virtio: fix memleak on init failure 



This patch also introduces
vq->poller_ctx. It will be later
reused for per-virtqueue task pools
to allow multiple threads using
the same virtqueue.

Since at the time of scanning there
is no I/O traffic, this field is
now being used to keep scan base
pointer. It has to be freed if an
initialization error occurs.

Change-Id: Ia54ee6c8402d38218dc811b4994761105d17269a
Signed-off-by: default avatarDariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/382199


Tested-by: default avatarSPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarDaniel Verkamp <daniel.verkamp@intel.com>
parent 13017493

lib/bdev/virtio/bdev_virtio.c

+23 −5
Original line number Diff line number Diff line
@@ -687,11 +687,7 @@ bdev_virtio_process_config(void) 
		rc = vtpci_enumerate_pci();

	}



	return rc;

out:

	if (vdev) {

		virtio_dev_free(vdev);

	}

	return rc;

}
@@ -700,7 +696,7 @@ static int 
bdev_virtio_initialize(void)

{

	struct virtio_scsi_scan_base *base;

	struct virtio_dev *vdev = NULL;

	struct virtio_dev *vdev, *next_vdev;

	struct virtqueue *vq;

	int rc = 0;
@@ -714,6 +710,7 @@ bdev_virtio_initialize(void) 
		return 0;

	}



	/* Initialize all created devices and scan available targets */

	TAILQ_FOREACH(vdev, &g_virtio_driver.init_ctrlrs, tailq) {

		base = spdk_dma_zmalloc(sizeof(*base), 64, NULL);

		if (base == NULL) {
@@ -724,11 +721,13 @@ bdev_virtio_initialize(void) 


		rc = virtio_dev_init(vdev, VIRTIO_SCSI_DEV_SUPPORTED_FEATURES);

		if (rc != 0) {

			spdk_dma_free(base);

			goto out;

		}



		rc = virtio_dev_start(vdev);

		if (rc != 0) {

			spdk_dma_free(base);

			goto out;

		}
@@ -738,19 +737,38 @@ bdev_virtio_initialize(void) 
		rc = virtio_dev_acquire_queue(vdev, VIRTIO_SCSI_REQUESTQ);

		if (rc != 0) {

			SPDK_ERRLOG("Couldn't acquire requestq for the target scan.\n");

			spdk_dma_free(base);

			goto out;

		}



		vq = vdev->vqs[VIRTIO_SCSI_REQUESTQ];

		base->vq = vq;

		vq->poller_ctx = base;

		spdk_bdev_poller_start(&vq->poller, bdev_scan_poll, base,

				       vq->owner_lcore, 0);



		scan_target(base);

	}



	return 0;



out:

	/* Remove any created devices */

	TAILQ_FOREACH_SAFE(vdev, &g_virtio_driver.init_ctrlrs, tailq, next_vdev) {

		TAILQ_REMOVE(&g_virtio_driver.init_ctrlrs, vdev, tailq);

		if (virtio_dev_queue_is_acquired(vdev, VIRTIO_SCSI_REQUESTQ)) {

			vq = vdev->vqs[VIRTIO_SCSI_REQUESTQ];

			spdk_bdev_poller_stop(&vq->poller);

			spdk_dma_free(vq->poller_ctx);

			vq->poller_ctx = NULL;

			virtio_dev_release_queue(vdev, VIRTIO_SCSI_REQUESTQ);

		}

		/* since scan pollers couldn't do a single tick yet.

		 * it's safe just to free the vdev now.

		 */

		virtio_dev_free(vdev);

	}



	spdk_bdev_module_init_done(SPDK_GET_BDEV_MODULE(virtio_scsi));

	return rc;

}

lib/bdev/virtio/rte_virtio/virtio_dev.c

+26 −0
Original line number Diff line number Diff line
@@ -578,6 +578,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 virtqueue *vq = NULL;

	bool rc;



	if (index >= vdev->max_queues) {

		SPDK_ERRLOG("given vq index %"PRIu16" exceeds max queue count %"PRIu16"\n",

			    index, vdev->max_queues);

		return false;

	}



	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;

	}



	rc = (vq->owner_lcore != SPDK_VIRTIO_QUEUE_LCORE_ID_UNUSED);

	pthread_mutex_unlock(&vdev->mutex);



	return rc;

}



void

virtio_dev_release_queue(struct virtio_dev *vdev, uint16_t index)

{

lib/bdev/virtio/rte_virtio/virtio_dev.h

+14 −0
Original line number Diff line number Diff line
@@ -142,6 +142,9 @@ struct virtqueue { 
	/** Response poller. */

	struct spdk_bdev_poller	*poller;



	/** Context for response poller. */

	void *poller_ctx;



	struct vq_desc_extra vq_descx[0];

};
@@ -252,6 +255,17 @@ 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);



/**

 * Check if virtqueue with given index is acquired.

 *

 * This function is thread-safe.

 *

 * \param vdev vhost device

 * \param index index of virtqueue

 * \return virtqueue acquire status. in case of invalid index *false* is returned.

 */

bool virtio_dev_queue_is_acquired(struct virtio_dev *vdev, uint16_t index);



/**

 * Release previously acquired queue.

 *