From 3fd405c54af2df6b7345808b6047ea4a8c47e952 Mon Sep 17 00:00:00 2001 From: Darek Stojaczyk Date: Tue, 8 Jan 2019 01:10:53 +0100 Subject: [PATCH] vhost/scsi: remove hotremoved scsi targets on device stop In cases where initiator closes the connection as soon as it receives a hotremove event, there is a possibility of SPDK vhost stopping the session before finishing up the asynchronous target hotremoval. The target would be either hotremoved once the session is started again (and it registers its management poller again) or it could cause a potential memory leak if that session is destroyed. Even though the SCSI target itself is always freed, the hotremoval completion callback is only called from the management poller. At least in our RPC case, not calling that callback results in leaking the context structure and some json data. We fix the above by calling all hotremove callbacks just before stopping the device. Change-Id: Ibfd773e1ab82b63643c57d7a9d37304e3007e38b Signed-off-by: Darek Stojaczyk Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/439445 Tested-by: SPDK CI Jenkins Reviewed-by: Pawel Wodkowski Reviewed-by: Changpeng Liu Reviewed-by: Jim Harris --- lib/vhost/vhost_scsi.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/lib/vhost/vhost_scsi.c b/lib/vhost/vhost_scsi.c index 8557c34b1..42761d95b 100644 --- a/lib/vhost/vhost_scsi.c +++ b/lib/vhost/vhost_scsi.c @@ -1356,6 +1356,7 @@ destroy_session_poller_cb(void *arg) { struct spdk_vhost_scsi_session *svsession = arg; struct spdk_vhost_session *vsession = &svsession->vsession; + struct spdk_scsi_dev_vhost_state *state; uint32_t i; if (vsession->task_cnt > 0) { @@ -1371,11 +1372,21 @@ destroy_session_poller_cb(void *arg) } for (i = 0; i < SPDK_VHOST_SCSI_CTRLR_MAX_DEVS; i++) { - if (svsession->scsi_dev_state[i].dev == NULL) { + state = &svsession->scsi_dev_state[i]; + if (state->dev == NULL) { continue; } - spdk_scsi_dev_free_io_channels(svsession->scsi_dev_state[i].dev); + spdk_scsi_dev_free_io_channels(state->dev); + + if (state->status == VHOST_SCSI_DEV_REMOVING) { + state->dev = NULL; + state->status = VHOST_SCSI_DEV_REMOVED; + /* try to detach it globally */ + spdk_vhost_dev_foreach_session(vsession->vdev, + spdk_vhost_scsi_session_process_removed, + (void *)(uintptr_t)i); + } } SPDK_INFOLOG(SPDK_LOG_VHOST, "Stopping poller for vhost controller %s\n", @@ -1398,8 +1409,19 @@ spdk_vhost_scsi_stop_cb(struct spdk_vhost_dev *vdev, svsession = to_scsi_session(vsession); assert(svsession != NULL); + + /* Stop receiving new I/O requests */ spdk_poller_unregister(&svsession->requestq_poller); + + /* Stop receiving controlq requests, also stop processing the + * asynchronous hotremove events. All the remaining events + * will be finalized by the stop_poller below. + */ spdk_poller_unregister(&svsession->mgmt_poller); + + /* Wait for all pending I/Os to complete, then process all the + * remaining hotremove events one last time. + */ svsession->stop_poller = spdk_poller_register(destroy_session_poller_cb, svsession, 1000);