Commit 64d76e50 authored by Darek Stojaczyk's avatar Darek Stojaczyk Committed by Jim Harris
Browse files

vhost: call session_event_done() always under the global vhost lock



In the next patch we will put much more responsibility
on spdk_vhost_session_event_done(), so here we make
sure it's always called under the global vhost mutex.

Specifically, spdk_vhost_session_event_done() will set
vsession->lcore, which any other thread might try to
concurrently access via spdk_vhost_dev_foreach_session().

Change-Id: I7a5fde4be4e8bdfdbbb24ac955af964f516bdb68
Signed-off-by: default avatarDarek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/448227


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
parent bbfbadf5
Loading
Loading
Loading
Loading
+9 −0
Original line number Diff line number Diff line
@@ -886,9 +886,18 @@ spdk_vhost_event_cb(void *arg1, void *arg2)
{
	struct spdk_vhost_session_fn_ctx *ctx = arg1;
	struct spdk_vhost_session *vsession;
	struct spdk_event *ev;

	if (pthread_mutex_trylock(&g_spdk_vhost_mutex) != 0) {
		ev = spdk_event_allocate(spdk_env_get_current_core(),
					 spdk_vhost_event_cb, arg1, arg2);
		spdk_event_call(ev);
		return;
	}

	vsession = spdk_vhost_session_find_by_id(ctx->vdev, ctx->vsession_id);
	ctx->cb_fn(ctx->vdev, vsession, NULL);
	pthread_mutex_unlock(&g_spdk_vhost_mutex);
}

static void spdk_vhost_external_event_foreach_continue(struct spdk_vhost_dev *vdev,
+5 −0
Original line number Diff line number Diff line
@@ -754,6 +754,10 @@ destroy_session_poller_cb(void *arg)
		return -1;
	}

	if (spdk_vhost_trylock() != 0) {
		return -1;
	}

	for (i = 0; i < vsession->max_queues; i++) {
		vsession->virtqueue[i].next_event_time = 0;
		spdk_vhost_vq_used_signal(vsession, &vsession->virtqueue[i]);
@@ -770,6 +774,7 @@ destroy_session_poller_cb(void *arg)
	spdk_poller_unregister(&bvsession->stop_poller);
	spdk_vhost_session_event_done(vsession, 0);

	spdk_vhost_unlock();
	return -1;
}

+5 −3
Original line number Diff line number Diff line
@@ -321,9 +321,9 @@ void spdk_vhost_dev_foreach_session(struct spdk_vhost_dev *dev,
 * Call the provided function on the session's lcore and block until
 * spdk_vhost_session_event_done() is called.
 *
 * As an optimization, this function will unlock the vhost mutex
 * while it's waiting, which makes it prone to data races.
 * Practically, it is only useful for session start/stop and still
 * This must be called under the global vhost mutex, which this function
 * will unlock for the time it's waiting. This makes it prone to data races,
 * so practically it is only useful for session start/stop and still
 * has to be used with extra caution.
 *
 * \param vsession vhost session
@@ -340,6 +340,8 @@ int spdk_vhost_session_send_event(struct spdk_vhost_session *vsession,
/**
 * Finish a blocking spdk_vhost_session_send_event() call.
 *
 * Must be called under the global vhost mutex.
 *
 * \param vsession vhost session
 * \param response return code
 */
+5 −0
Original line number Diff line number Diff line
@@ -1164,6 +1164,10 @@ destroy_device_poller_cb(void *arg)

	/* FIXME wait for pending I/Os to complete */

	if (spdk_vhost_trylock() != 0) {
		return -1;
	}

	for (i = 0; i < nvme->num_ns; i++) {
		ns_dev = &nvme->ns[i];
		if (ns_dev->bdev_io_channel) {
@@ -1184,6 +1188,7 @@ destroy_device_poller_cb(void *arg)
	spdk_poller_unregister(&nvme->stop_poller);
	spdk_vhost_session_event_done(nvme->vsession, 0);

	spdk_vhost_unlock();
	return -1;
}

+4 −0
Original line number Diff line number Diff line
@@ -1365,6 +1365,9 @@ destroy_session_poller_cb(void *arg)
		return -1;
	}

	if (spdk_vhost_trylock() != 0) {
		return -1;
	}

	for (i = 0; i < vsession->max_queues; i++) {
		spdk_vhost_vq_used_signal(vsession, &vsession->virtqueue[i]);
@@ -1386,6 +1389,7 @@ destroy_session_poller_cb(void *arg)
	spdk_poller_unregister(&svsession->stop_poller);
	spdk_vhost_session_event_done(vsession, 0);

	spdk_vhost_unlock();
	return -1;
}