Commit 4c4492dc authored by Dariusz Stojaczyk's avatar Dariusz Stojaczyk Committed by Daniel Verkamp
Browse files

vhost_scsi: removed internal eventq events ring 



Since we are calling eventq_enqueue only from inside of the controller
reactor, such ring is not needed. Eventq events will now be sent
immediately. This simplifies the whole process greatly.

The abstract vhost event layer is being replaced with much simpler and
less error-prone vhost external spdk_event API.

This reverts commit 542b5415 [1] together with vhost scsi device
hot-attach feature. Hot-attach shall be reintroduced in upcoming
patches.

[1] 542b5415 ("vhost_scsi: implemented abstract vhost event layer")

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


Tested-by: default avatarSPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: default avatarPawel Wodkowski <pawelx.wodkowski@intel.com>
Reviewed-by: default avatarDaniel Verkamp <daniel.verkamp@intel.com>
parent dcb7db96

lib/vhost/vhost_scsi.c

+68 −203
Original line number Diff line number Diff line
@@ -46,8 +46,6 @@ 
#include "spdk/vhost.h"

#include "vhost_internal.h"



#include "spdk_internal/assert.h"



/* Features supported by SPDK VHOST lib. */

#define SPDK_VHOST_SCSI_FEATURES	(SPDK_VHOST_FEATURES | \

					(1ULL << VIRTIO_SCSI_F_INOUT) | \
@@ -71,13 +69,11 @@ 
struct spdk_vhost_scsi_dev {

	struct spdk_vhost_dev vdev;

	struct spdk_scsi_dev *scsi_dev[SPDK_VHOST_SCSI_CTRLR_MAX_DEVS];

	bool detached_dev[SPDK_VHOST_SCSI_CTRLR_MAX_DEVS];

	bool removed_dev[SPDK_VHOST_SCSI_CTRLR_MAX_DEVS];



	struct spdk_ring *task_pool;

	struct spdk_poller *requestq_poller;

	struct spdk_poller *mgmt_poller;



	struct spdk_ring *vhost_events;

} __rte_cache_aligned;



struct spdk_vhost_scsi_task {
@@ -97,18 +93,6 @@ struct spdk_vhost_scsi_task { 
	struct rte_vhost_vring *vq;

};



enum spdk_vhost_scsi_event_type {

	SPDK_VHOST_SCSI_EVENT_HOTATTACH,

	SPDK_VHOST_SCSI_EVENT_HOTDETACH,

};



struct spdk_vhost_scsi_event {

	enum spdk_vhost_scsi_event_type type;

	unsigned dev_index;

	struct spdk_scsi_dev *dev;

	struct spdk_scsi_lun *lun;

};



static int new_device(struct spdk_vhost_dev *, void *);

static int destroy_device(struct spdk_vhost_dev *, void *);

static void spdk_vhost_scsi_config_json(struct spdk_vhost_dev *vdev, struct spdk_json_write_ctx *w);
@@ -155,150 +139,84 @@ spdk_vhost_get_tasks(struct spdk_vhost_scsi_dev *svdev, struct spdk_vhost_scsi_t 
}



static void

spdk_vhost_scsi_event_process(struct spdk_vhost_scsi_dev *svdev, struct spdk_vhost_scsi_event *ev,

			      struct virtio_scsi_event *desc_ev)

process_removed_devs(struct spdk_vhost_scsi_dev *svdev)

{

	int event_id, reason_id;

	int dev_id, lun_id;



	assert(ev->dev);

	assert(ev->dev_index < SPDK_VHOST_SCSI_CTRLR_MAX_DEVS);

	dev_id = ev->dev_index;



	switch (ev->type) {

	case SPDK_VHOST_SCSI_EVENT_HOTATTACH:

		event_id = VIRTIO_SCSI_T_TRANSPORT_RESET;

		reason_id = VIRTIO_SCSI_EVT_RESET_RESCAN;

	struct spdk_scsi_dev *dev;

	int i;



		spdk_scsi_dev_allocate_io_channels(ev->dev);

		svdev->scsi_dev[dev_id] = ev->dev;

		svdev->detached_dev[dev_id] = false;

		SPDK_NOTICELOG("%s: hot-attached device %d\n", svdev->vdev.name, dev_id);

		break;

	case SPDK_VHOST_SCSI_EVENT_HOTDETACH:

		event_id = VIRTIO_SCSI_T_TRANSPORT_RESET;

		reason_id = VIRTIO_SCSI_EVT_RESET_REMOVED;

	for (i = 0; i < SPDK_VHOST_SCSI_CTRLR_MAX_DEVS; ++i) {

		dev = svdev->scsi_dev[i];



		if (ev->lun == NULL) {

			svdev->detached_dev[dev_id] = true;

			SPDK_NOTICELOG("%s: marked 'Dev %d' for hot-detach\n", svdev->vdev.name, dev_id);

		} else {

			SPDK_NOTICELOG("%s: hotremoved LUN '%s'\n", svdev->vdev.name, spdk_scsi_lun_get_name(ev->lun));

		if (dev && svdev->removed_dev[i] && !spdk_scsi_dev_has_pending_tasks(dev)) {

			spdk_scsi_dev_free_io_channels(dev);

			spdk_scsi_dev_destruct(dev);

			svdev->scsi_dev[i] = NULL;

			SPDK_NOTICELOG("%s: hot-detached device 'Dev %u'.\n", svdev->vdev.name, i);

		}

		break;

	default:

		SPDK_UNREACHABLE();

	}



	/* some events may apply to the entire device via lun id set to 0 */

	lun_id = ev->lun == NULL ? 0 : spdk_scsi_lun_get_id(ev->lun);



	if (desc_ev) {

		desc_ev->event = event_id;

		desc_ev->lun[0] = 1;

		desc_ev->lun[1] = dev_id;

		desc_ev->lun[2] = lun_id >> 8; /* relies on linux kernel implementation */

		desc_ev->lun[3] = lun_id & 0xFF;

		memset(&desc_ev->lun[4], 0, 4);

		desc_ev->reason = reason_id;

	}

}



/**

 * Process vhost event, send virtio event

 */

static void

process_event(struct spdk_vhost_scsi_dev *svdev, struct spdk_vhost_scsi_event *ev)

eventq_enqueue(struct spdk_vhost_scsi_dev *svdev, const struct spdk_scsi_dev *dev,

	       const struct spdk_scsi_lun *lun, uint32_t event,

	       uint32_t reason)

{

	int dev_id, lun_id;

	struct rte_vhost_vring *vq;

	struct vring_desc *desc;

	struct virtio_scsi_event *desc_ev;

	uint32_t req_size;

	uint16_t req;

	struct rte_vhost_vring *vq = &svdev->vdev.virtqueue[VIRTIO_SCSI_EVENTQ];



	if (spdk_vhost_vq_avail_ring_get(vq, &req, 1) != 1) {

		SPDK_ERRLOG("%s: no avail virtio eventq ring entries. virtio event won't be sent.\n",

			    svdev->vdev.name);

		desc = NULL;

		desc_ev = NULL;

		req_size = 0;

		/* even though we can't send virtio event,

		 * the spdk vhost event should still be processed

		 */

	} else {

		desc =  spdk_vhost_vq_get_desc(vq, req);

		desc_ev = spdk_vhost_gpa_to_vva(&svdev->vdev, desc->addr);

		req_size = sizeof(*desc_ev);



		if (desc->len < sizeof(*desc_ev) || desc_ev == NULL) {

			SPDK_ERRLOG("%s: invalid eventq descriptor.\n", svdev->vdev.name);

			desc_ev = NULL;

			req_size = 0;

		}

	}



	spdk_vhost_scsi_event_process(svdev, ev, desc_ev);



	if (desc) {

		spdk_vhost_vq_used_ring_enqueue(&svdev->vdev, vq, req, req_size);

	}

	if (dev == NULL) {

		SPDK_ERRLOG("%s: eventq device cannot be NULL.\n", svdev->vdev.name);

		return;

	}



static void

process_eventq(struct spdk_vhost_scsi_dev *svdev)

{

	struct spdk_vhost_scsi_event *ev;



	while (spdk_ring_dequeue(svdev->vhost_events, (void **)&ev, 1) == 1) {

		process_event(svdev, ev);

		spdk_dma_free(ev);

	for (dev_id = 0; dev_id < SPDK_VHOST_SCSI_CTRLR_MAX_DEVS; dev_id++) {

		if (svdev->scsi_dev[dev_id] == dev) {

			break;

		}

	}



static void

process_removed_devs(struct spdk_vhost_scsi_dev *svdev)

{

	struct spdk_scsi_dev *dev;

	int i;



	for (i = 0; i < SPDK_VHOST_SCSI_CTRLR_MAX_DEVS; ++i) {

		dev = svdev->scsi_dev[i];



		if (dev && svdev->detached_dev[i] && !spdk_scsi_dev_has_pending_tasks(dev)) {

			spdk_scsi_dev_free_io_channels(dev);

			spdk_scsi_dev_destruct(dev);

			svdev->scsi_dev[i] = NULL;

			SPDK_NOTICELOG("%s: hot-detached 'Dev %d'.\n", svdev->vdev.name, i);

		}

	}

	if (dev_id == SPDK_VHOST_SCSI_CTRLR_MAX_DEVS) {

		SPDK_ERRLOG("Dev %s is not a part of vhost scsi controller '%s'.\n", spdk_scsi_dev_get_name(dev),

			    svdev->vdev.name);

		return;

	}



static void

enqueue_vhost_event(struct spdk_vhost_scsi_dev *svdev, enum spdk_vhost_scsi_event_type type,

		    int dev_index, struct spdk_scsi_dev *dev, struct spdk_scsi_lun *lun)

{

	struct spdk_vhost_scsi_event *ev;

	/* some events may apply to the entire device via lun id set to 0 */

	lun_id = lun == NULL ? 0 : spdk_scsi_lun_get_id(lun);



	if (dev == NULL) {

		SPDK_ERRLOG("%s: vhost event device cannot be NULL.\n", svdev->vdev.name);

		return;

	}

	vq = &svdev->vdev.virtqueue[VIRTIO_SCSI_EVENTQ];



	ev = spdk_dma_zmalloc(sizeof(*ev), SPDK_CACHE_LINE_SIZE, NULL);

	if (ev == NULL) {

		SPDK_ERRLOG("%s: failed to alloc vhost event.\n", svdev->vdev.name);

	if (spdk_vhost_vq_avail_ring_get(vq, &req, 1) != 1) {

		SPDK_ERRLOG("Controller %s: Failed to send virtio event (no avail ring entries?).\n",

			    svdev->vdev.name);

		return;

	}



	ev->type = type;

	ev->dev_index = dev_index;

	ev->dev = dev;

	ev->lun = lun;

	desc =  spdk_vhost_vq_get_desc(vq, req);

	desc_ev = spdk_vhost_gpa_to_vva(&svdev->vdev, desc->addr);



	if (spdk_ring_enqueue(svdev->vhost_events, (void **)&ev, 1) != 1) {

		SPDK_ERRLOG("%s: failed to enqueue vhost event (no room in ring?).\n", svdev->vdev.name);

		spdk_dma_free(ev);

	if (desc->len < sizeof(*desc_ev) || desc_ev == NULL) {

		SPDK_ERRLOG("Controller %s: Invalid eventq descriptor.\n", svdev->vdev.name);

		req_size = 0;

	} else {

		desc_ev->event = event;

		desc_ev->lun[0] = 1;

		desc_ev->lun[1] = dev_id;

		desc_ev->lun[2] = lun_id >> 8; /* relies on linux kernel implementation */

		desc_ev->lun[3] = lun_id & 0xFF;

		/* virtio doesn't specify any strict format for LUN id (bytes 2 and 3)

		 * current implementation relies on linux kernel sources

		 */

		memset(&desc_ev->lun[4], 0, 4);

		desc_ev->reason = reason;

		req_size = sizeof(*desc_ev);

	}



	spdk_vhost_vq_used_ring_enqueue(&svdev->vdev, vq, req, req_size);

}



static void
@@ -355,11 +273,6 @@ mgmt_task_submit(struct spdk_vhost_scsi_task *task, enum spdk_scsi_task_func fun 
static void

invalid_request(struct spdk_vhost_scsi_task *task)

{

	/* Flush eventq so that guest is instantly notified about any hotremoved luns.

	 * This might prevent him from sending more invalid requests and trying to reset

	 * the device.

	 */

	process_eventq(task->svdev);

	spdk_vhost_vq_used_ring_enqueue(&task->svdev->vdev, task->vq, task->req_idx, 0);

	spdk_vhost_scsi_task_put(task);
@@ -382,10 +295,10 @@ spdk_vhost_scsi_task_init_target(struct spdk_vhost_scsi_task *task, const __u8 * 
	dev = task->svdev->scsi_dev[lun[1]];

	task->scsi_dev = dev;

	if (dev == NULL) {

		/* If dev has been hot-detached, return 0 to allow sending additional

		 * scsi hotremove event via sense codes.

		/* If dev has been hotdetached, return 0 to allow sending

		 * additional hotremove event via sense codes.

		 */

		return task->svdev->detached_dev[lun[1]] ? 0 : -1;

		return task->svdev->removed_dev[lun[1]] ? 0 : -1;

	}



	task->scsi.target_port = spdk_scsi_dev_find_port_by_id(task->scsi_dev, 0);
@@ -673,7 +586,6 @@ vdev_mgmt_worker(void *arg) 
	struct spdk_vhost_scsi_dev *svdev = arg;



	process_removed_devs(svdev);

	process_eventq(svdev);

	process_controlq(svdev, &svdev->vdev.virtqueue[VIRTIO_SCSI_CONTROLQ]);

}
@@ -720,7 +632,6 @@ spdk_vhost_scsi_dev_construct(const char *name, const char *cpumask) 
				      &spdk_vhost_scsi_device_backend);



	if (rc) {

		spdk_ring_free(svdev->vhost_events);

		spdk_dma_free(svdev);

	}
@@ -750,7 +661,6 @@ spdk_vhost_scsi_dev_remove(struct spdk_vhost_dev *vdev) 
		return rc;

	}



	spdk_ring_free(svdev->vhost_events);

	spdk_dma_free(svdev);

	return 0;

}
@@ -770,8 +680,6 @@ static void 
spdk_vhost_scsi_lun_hotremove(const struct spdk_scsi_lun *lun, void *arg)

{

	struct spdk_vhost_scsi_dev *svdev = arg;

	const struct spdk_scsi_dev *scsi_dev;

	unsigned scsi_dev_num;



	assert(lun != NULL);

	assert(svdev != NULL);
@@ -780,25 +688,8 @@ spdk_vhost_scsi_lun_hotremove(const struct spdk_scsi_lun *lun, void *arg) 
		return;

	}



	scsi_dev = spdk_scsi_lun_get_dev(lun);



	for (scsi_dev_num = 0; scsi_dev_num < SPDK_VHOST_SCSI_CTRLR_MAX_DEVS; ++scsi_dev_num) {

		if (svdev->scsi_dev[scsi_dev_num] == scsi_dev) {

			break;

		}

	}



	if (scsi_dev_num == SPDK_VHOST_SCSI_CTRLR_MAX_DEVS) {

		SPDK_ERRLOG("%s: '%s' is not a part of this controller.\n", svdev->vdev.name,

			    spdk_scsi_dev_get_name(scsi_dev));

		return;

	}



	enqueue_vhost_event(svdev, SPDK_VHOST_SCSI_EVENT_HOTDETACH, scsi_dev_num,

			    (struct spdk_scsi_dev *) scsi_dev, (struct spdk_scsi_lun *) lun);



	SPDK_NOTICELOG("%s: queued LUN '%s' for hotremove\n", svdev->vdev.name,

		       spdk_scsi_dev_get_name(scsi_dev));

	eventq_enqueue(svdev, spdk_scsi_lun_get_dev(lun), lun, VIRTIO_SCSI_T_TRANSPORT_RESET,

		       VIRTIO_SCSI_EVT_RESET_REMOVED);

}



int
@@ -806,7 +697,6 @@ spdk_vhost_scsi_dev_add_dev(struct spdk_vhost_dev *vdev, unsigned scsi_dev_num, 
			    const char *lun_name)

{

	struct spdk_vhost_scsi_dev *svdev;

	struct spdk_scsi_dev *scsi_dev;

	char dev_name[SPDK_SCSI_DEV_MAX_NAME];

	int lun_id_list[1];

	char *lun_names_list[1];
@@ -835,9 +725,8 @@ spdk_vhost_scsi_dev_add_dev(struct spdk_vhost_dev *vdev, unsigned scsi_dev_num, 
		return -EINVAL;

	}



	if (vdev->lcore != -1 && !spdk_vhost_dev_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) {

		SPDK_ERRLOG("%s: 'Dev %u' is in use and hot-attach is not enabled for this controller\n",

			    vdev->name, scsi_dev_num);

	if (vdev->lcore != -1) {

		SPDK_ERRLOG("Controller %s is in use and hotplug is not supported\n", vdev->name);

		return -ENOTSUP;

	}
@@ -853,23 +742,16 @@ spdk_vhost_scsi_dev_add_dev(struct spdk_vhost_dev *vdev, unsigned scsi_dev_num, 
	lun_id_list[0] = 0;

	lun_names_list[0] = (char *)lun_name;



	scsi_dev = spdk_scsi_dev_construct(dev_name, lun_names_list, lun_id_list, 1,

	svdev->removed_dev[scsi_dev_num] = false;

	svdev->scsi_dev[scsi_dev_num] = spdk_scsi_dev_construct(dev_name, lun_names_list, lun_id_list, 1,

					SPDK_SPC_PROTOCOL_IDENTIFIER_SAS, spdk_vhost_scsi_lun_hotremove, svdev);

	if (scsi_dev == NULL) {



	if (svdev->scsi_dev[scsi_dev_num] == NULL) {

		SPDK_ERRLOG("Couldn't create spdk SCSI device '%s' using lun device '%s' in controller: %s\n",

			    dev_name, lun_name, vdev->name);

		return -EINVAL;

	}



	spdk_scsi_dev_add_port(scsi_dev, 0, "vhost");



	if (vdev->lcore == -1) {

		svdev->detached_dev[scsi_dev_num] = false;

		svdev->scsi_dev[scsi_dev_num] = scsi_dev;

	} else {

		enqueue_vhost_event(svdev, SPDK_VHOST_SCSI_EVENT_HOTATTACH, scsi_dev_num, scsi_dev, NULL);

	}



	spdk_scsi_dev_add_port(svdev->scsi_dev[scsi_dev_num], 0, "vhost");

	SPDK_NOTICELOG("Controller %s: defined device '%s' using lun '%s'\n",

		       vdev->name, dev_name, lun_name);

	return 0;
@@ -911,7 +793,8 @@ spdk_vhost_scsi_dev_remove_dev(struct spdk_vhost_dev *vdev, unsigned scsi_dev_nu 
		return -ENOTSUP;

	}



	enqueue_vhost_event(svdev, SPDK_VHOST_SCSI_EVENT_HOTDETACH, scsi_dev_num, scsi_dev, NULL);

	svdev->removed_dev[scsi_dev_num] = true;

	eventq_enqueue(svdev, scsi_dev, NULL, VIRTIO_SCSI_T_TRANSPORT_RESET, VIRTIO_SCSI_EVT_RESET_REMOVED);



	SPDK_NOTICELOG("%s: queued 'Dev %u' for hot-detach.\n", vdev->name, scsi_dev_num);

	return 0;
@@ -1055,7 +938,7 @@ new_device(struct spdk_vhost_dev *vdev, void *event_ctx) 
{

	struct spdk_vhost_scsi_dev *svdev;

	uint32_t i;

	int rc = 0;

	int rc;



	svdev = to_scsi_dev(vdev);

	if (svdev == NULL) {
@@ -1070,14 +953,6 @@ new_device(struct spdk_vhost_dev *vdev, void *event_ctx) 
		goto out;

	}



	svdev->vhost_events = spdk_ring_create(SPDK_RING_TYPE_MP_SC, 16,

					       spdk_env_get_socket_id(vdev->lcore));

	if (svdev->vhost_events == NULL) {

		SPDK_ERRLOG("%s: failed to alloc event pool.\n", vdev->name);

		rc = -1;

		goto out;

	}



	for (i = 0; i < SPDK_VHOST_SCSI_CTRLR_MAX_DEVS; i++) {

		if (svdev->scsi_dev[i] == NULL) {

			continue;
@@ -1107,7 +982,6 @@ destroy_device_poller_cb(void *arg) 
{

	struct spdk_vhost_dev_destroy_ctx *ctx = arg;

	struct spdk_vhost_scsi_dev *svdev = ctx->svdev;

	void *ev;

	uint32_t i;



	if (svdev->vdev.task_cnt > 0) {
@@ -1124,15 +998,6 @@ destroy_device_poller_cb(void *arg) 
	SPDK_NOTICELOG("Stopping poller for vhost controller %s\n", svdev->vdev.name);

	spdk_vhost_dev_mem_unregister(&svdev->vdev);



	/* Flush not sent events */

	while (spdk_ring_dequeue(svdev->vhost_events, &ev, 1) == 1) {

		/* process vhost event, but don't send virtio event */

		spdk_vhost_scsi_event_process(svdev, ev, NULL);

		spdk_dma_free(ev);

	}



	spdk_ring_free(svdev->vhost_events);



	free_task_pool(svdev);



	spdk_poller_unregister(&ctx->poller, NULL);

test/vhost/fiotest/autotest.sh

+0 −20
Original line number Diff line number Diff line
@@ -250,26 +250,6 @@ done 
$COMMON_DIR/vm_run.sh $x --work-dir=$TEST_DIR $used_vms

vm_wait_for_boot 600 $used_vms



if [[ $test_type == "spdk_vhost_scsi" ]]; then

	for vm_conf in ${vms[@]}; do

		IFS=',' read -ra conf <<< "$vm_conf"

		while IFS=':' read -ra disks; do

			for disk in "${disks[@]}"; do

				echo "INFO: Hotdetach test. Trying to remove existing device from a controller naa.$disk.${conf[0]}"

				$rpc_py remove_vhost_scsi_dev naa.$disk.${conf[0]} 0



				sleep 0.1



				echo "INFO: Hotattach test. Re-adding device 0 to naa.$disk.${conf[0]}"

				$rpc_py add_vhost_scsi_lun naa.$disk.${conf[0]} 0 $disk

			done

		done <<< "${conf[2]}"

		unset IFS;

	done

fi



sleep 0.1



echo "==============="

echo ""

echo "INFO: Testing..."