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

vhost: introduce vsession->started

Browse Source 
We used to rely on lcore >= 0 for sessions that are
started (have their pollers running) and in order to
prevent data races, that lcore field had to be set from
the same thread that runs the pollers, directly after
registering/unregistering them. The lcore was always
set to spdk_env_get_current_core(), but we won't be able
to use an equivalent get_current_poll_group() function
after we switch to poll groups. We will have a poll group
object only inside spdk_vhost_session_send_event() that's
called from the DPDK rte_vhost thread.

In order to change the lcore field (or a poll group one)
from spdk_vhost_session_send_event(), we'll need a separate
field to maintain the started/stopped status that's only
going to be modified from the session's thread.

Change-Id: Idb09cae3c4715eebb20282aad203987b26be707b
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/452394
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
This commit is contained in:
Darek Stojaczyk 2019-04-29 09:26:10 +02:00 committed by Jim Harris
parent 838949ea97
commit 376d893a20
5 changed files with 16 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
lib/vhost/rte_vhost_compat.c
View File

@ -66,7 +66,7 @@ spdk_extern_vhost_pre_msg_handler(int vid, void *_msg)
switch (msg->request) { switch (msg->request) {
case VHOST_USER_GET_VRING_BASE: case VHOST_USER_GET_VRING_BASE:
if (vsession->forced_polling && vsession->lcore != -1) { if (vsession->forced_polling && vsession->started) {
/* Our queue is stopped for whatever reason, but we may still /* Our queue is stopped for whatever reason, but we may still
* need to poll it after it's initialized again. * need to poll it after it's initialized again.
*/ */
@ -77,7 +77,7 @@ spdk_extern_vhost_pre_msg_handler(int vid, void *_msg)
case VHOST_USER_SET_VRING_ADDR: case VHOST_USER_SET_VRING_ADDR:
case VHOST_USER_SET_VRING_NUM: case VHOST_USER_SET_VRING_NUM:
case VHOST_USER_SET_VRING_KICK: case VHOST_USER_SET_VRING_KICK:
if (vsession->forced_polling && vsession->lcore != -1) { if (vsession->forced_polling && vsession->started) {
/* Additional queues are being initialized, so we either processed /* Additional queues are being initialized, so we either processed
* enough I/Os and are switching from SeaBIOS to the OS now, or * enough I/Os and are switching from SeaBIOS to the OS now, or
* we were never in SeaBIOS in the first place. Either way, we * we were never in SeaBIOS in the first place. Either way, we
@ -105,7 +105,7 @@ spdk_extern_vhost_pre_msg_handler(int vid, void *_msg)
* We will start the device again from the post-processing * We will start the device again from the post-processing
* message handler. * message handler.
*/ */
if (vsession->lcore != -1) { if (vsession->started) {
g_spdk_vhost_ops.destroy_device(vid); g_spdk_vhost_ops.destroy_device(vid);
vsession->needs_restart = true; vsession->needs_restart = true;
} }
@ -187,7 +187,7 @@ spdk_extern_vhost_post_msg_handler(int vid, void *_msg)
/* vhost-user spec tells us to start polling a queue after receiving /* vhost-user spec tells us to start polling a queue after receiving
* its SET_VRING_KICK message. Let's do it! * its SET_VRING_KICK message. Let's do it!
*/ */
if (vsession->forced_polling && vsession->lcore == -1) { if (vsession->forced_polling && !vsession->started) {
g_spdk_vhost_ops.new_device(vid); g_spdk_vhost_ops.new_device(vid);
} }
break; break;

13
lib/vhost/vhost.c
View File

@ -899,6 +899,7 @@ spdk_vhost_session_start_done(struct spdk_vhost_session *vsession, int response)
{ {
if (response == 0) { if (response == 0) {
vsession->lcore = spdk_env_get_current_core(); vsession->lcore = spdk_env_get_current_core();
vsession->started = true;
assert(vsession->vdev->active_session_num < UINT32_MAX); assert(vsession->vdev->active_session_num < UINT32_MAX);
vsession->vdev->active_session_num++; vsession->vdev->active_session_num++;
} }
@ -910,6 +911,7 @@ spdk_vhost_session_stop_done(struct spdk_vhost_session *vsession, int response)
{ {
if (response == 0) { if (response == 0) {
vsession->lcore = -1; vsession->lcore = -1;
vsession->started = false;
assert(vsession->vdev->active_session_num > 0); assert(vsession->vdev->active_session_num > 0);
vsession->vdev->active_session_num--; vsession->vdev->active_session_num--;
} }
@ -963,7 +965,7 @@ spdk_vhost_event_async_foreach_fn(void *arg1, void *arg2)
goto out_unlock_continue; goto out_unlock_continue;
} }
if (vsession->lcore >= 0 && if (vsession->started &&
(uint32_t)vsession->lcore != spdk_env_get_current_core()) { (uint32_t)vsession->lcore != spdk_env_get_current_core()) {
/* if session has been relocated to other core, it is no longer thread-safe /* if session has been relocated to other core, it is no longer thread-safe
* to access its contents here. Even though we're running under the global * to access its contents here. Even though we're running under the global
@ -1099,7 +1101,7 @@ stop_device(int vid)
return; return;
} }
if (vsession->lcore == -1) { if (!vsession->started) {
/* already stopped, nothing to do */ /* already stopped, nothing to do */
pthread_mutex_unlock(&g_spdk_vhost_mutex); pthread_mutex_unlock(&g_spdk_vhost_mutex);
return; return;
@ -1126,7 +1128,7 @@ start_device(int vid)
} }
vdev = vsession->vdev; vdev = vsession->vdev;
if (vsession->lcore != -1) { if (vsession->started) {
/* already started, nothing to do */ /* already started, nothing to do */
rc = 0; rc = 0;
goto out; goto out;
@ -1345,6 +1347,7 @@ new_connection(int vid)
vsession->id = vdev->vsessions_num++; vsession->id = vdev->vsessions_num++;
vsession->vid = vid; vsession->vid = vid;
vsession->lcore = -1; vsession->lcore = -1;
vsession->started = false;
vsession->initialized = false; vsession->initialized = false;
vsession->next_stats_check_time = 0; vsession->next_stats_check_time = 0;
vsession->stats_check_interval = SPDK_VHOST_STATS_CHECK_INTERVAL_MS * vsession->stats_check_interval = SPDK_VHOST_STATS_CHECK_INTERVAL_MS *
@ -1369,7 +1372,7 @@ destroy_connection(int vid)
return; return;
} }
if (vsession->lcore != -1) { if (vsession->started) {
_stop_session(vsession); _stop_session(vsession);
} }
@ -1389,7 +1392,7 @@ spdk_vhost_external_event_foreach_continue(struct spdk_vhost_dev *vdev,
goto out_finish_foreach; goto out_finish_foreach;
} }
while (vsession->lcore == -1) { while (!vsession->started) {
if (vsession->initialized) { if (vsession->initialized) {
rc = fn(vdev, vsession, arg); rc = fn(vdev, vsession, arg);
if (rc < 0) { if (rc < 0) {

1
lib/vhost/vhost_internal.h
View File

@ -127,6 +127,7 @@ struct spdk_vhost_session {
int32_t lcore; int32_t lcore;
bool initialized; bool initialized;
bool started;
bool needs_restart; bool needs_restart;
bool forced_polling; bool forced_polling;

4
lib/vhost/vhost_scsi.c
View File

@ -947,7 +947,7 @@ spdk_vhost_scsi_session_add_tgt(struct spdk_vhost_dev *vdev,
svsession = (struct spdk_vhost_scsi_session *)vsession; svsession = (struct spdk_vhost_scsi_session *)vsession;
session_sdev = &svsession->scsi_dev_state[scsi_tgt_num]; session_sdev = &svsession->scsi_dev_state[scsi_tgt_num];
if (vsession->lcore == -1 || session_sdev->dev != NULL) { if (!vsession->started || session_sdev->dev != NULL) {
/* Nothing to do. */ /* Nothing to do. */
return 0; return 0;
} }
@ -1092,7 +1092,7 @@ spdk_vhost_scsi_session_remove_tgt(struct spdk_vhost_dev *vdev,
svsession = (struct spdk_vhost_scsi_session *)vsession; svsession = (struct spdk_vhost_scsi_session *)vsession;
state = &svsession->scsi_dev_state[scsi_tgt_num]; state = &svsession->scsi_dev_state[scsi_tgt_num];
if (vsession->lcore == -1 || state->dev == NULL) { if (!vsession->started || state->dev == NULL) {
/* Nothing to do */ /* Nothing to do */
return 0; return 0;
} }

1
test/unit/lib/vhost/vhost.c/vhost_ut.c
View File

@ -164,6 +164,7 @@ start_vdev(struct spdk_vhost_dev *vdev)
CU_ASSERT(rc == 0); CU_ASSERT(rc == 0);
SPDK_CU_ASSERT_FATAL(vsession != NULL); SPDK_CU_ASSERT_FATAL(vsession != NULL);
vsession->lcore = 0; vsession->lcore = 0;
vsession->started = true;
vsession->vid = 0; vsession->vid = 0;
vsession->mem = mem; vsession->mem = mem;
TAILQ_INSERT_TAIL(&vdev->vsessions, vsession, tailq); TAILQ_INSERT_TAIL(&vdev->vsessions, vsession, tailq);