From 56d8b78576a13dda000d40ae9522a7a1e70a6a2a Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Thu, 11 Apr 2019 18:10:47 +0900 Subject: [PATCH] lib/scsi: Make spdk_scsi_dev_destruct asynchronous This is the end of the patch series. After this patch, delete_target_node RPC will wait for the completion of removal of the SCSI device and then free the iSCSI target. SCSI device holds passed callback and calls it in free_dev(). free_dev() is ensured to be called after all iSCSI sessions are closed. So iSCSI target resource can be freed safely after that. Change-Id: I25921b4014207092b7b3845dfeae58bcdffa2edc Signed-off-by: Shuhei Matsumoto Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/450607 Reviewed-by: Jim Harris Reviewed-by: Changpeng Liu Tested-by: SPDK CI Jenkins --- include/spdk/scsi.h | 6 +++++- lib/iscsi/tgt_node.c | 6 ++---- lib/scsi/dev.c | 24 +++++++++++++++++++++--- lib/scsi/scsi_internal.h | 18 ++++++++++-------- lib/vhost/vhost_scsi.c | 2 +- test/unit/lib/iscsi/common.c | 3 ++- test/unit/lib/scsi/dev.c/dev_ut.c | 20 ++++++++++---------- 7 files changed, 51 insertions(+), 28 deletions(-) diff --git a/include/spdk/scsi.h b/include/spdk/scsi.h index 88a67dfb1f..1e922709d8 100644 --- a/include/spdk/scsi.h +++ b/include/spdk/scsi.h @@ -151,6 +151,7 @@ struct spdk_scsi_lun; struct spdk_scsi_lun_desc; typedef void (*spdk_scsi_lun_remove_cb_t)(struct spdk_scsi_lun *, void *); +typedef void (*spdk_scsi_dev_destruct_cb_t)(void *cb_arg, int rc); /** * Initialize SCSI layer. @@ -241,8 +242,11 @@ bool spdk_scsi_dev_has_pending_tasks(const struct spdk_scsi_dev *dev); * Destruct the SCSI decice. * * \param dev SCSI device. + * \param cb_fn Callback function. + * \param cb_arg Argument to callback function. */ -void spdk_scsi_dev_destruct(struct spdk_scsi_dev *dev); +void spdk_scsi_dev_destruct(struct spdk_scsi_dev *dev, + spdk_scsi_dev_destruct_cb_t cb_fn, void *cb_arg); /** * Execute the SCSI management task. diff --git a/lib/iscsi/tgt_node.c b/lib/iscsi/tgt_node.c index 8d3ef533ec..50bfd34ec2 100644 --- a/lib/iscsi/tgt_node.c +++ b/lib/iscsi/tgt_node.c @@ -664,8 +664,7 @@ iscsi_tgt_node_check_active_conns(void *arg) spdk_poller_unregister(&target->destruct_poller); - spdk_scsi_dev_destruct(target->dev); - _iscsi_tgt_node_destruct(target, 0); + spdk_scsi_dev_destruct(target->dev, _iscsi_tgt_node_destruct, target); return 1; } @@ -699,8 +698,7 @@ iscsi_tgt_node_destruct(struct spdk_iscsi_tgt_node *target, target->destruct_poller = spdk_poller_register(iscsi_tgt_node_check_active_conns, target, 10); } else { - spdk_scsi_dev_destruct(target->dev); - _iscsi_tgt_node_destruct(target, 0); + spdk_scsi_dev_destruct(target->dev, _iscsi_tgt_node_destruct, target); } } diff --git a/lib/scsi/dev.c b/lib/scsi/dev.c index c7cef1ac22..e567b40338 100644 --- a/lib/scsi/dev.c +++ b/lib/scsi/dev.c @@ -68,19 +68,37 @@ free_dev(struct spdk_scsi_dev *dev) assert(dev->removed == true); dev->is_allocated = 0; + + if (dev->remove_cb) { + dev->remove_cb(dev->remove_ctx, 0); + dev->remove_cb = NULL; + } } void -spdk_scsi_dev_destruct(struct spdk_scsi_dev *dev) +spdk_scsi_dev_destruct(struct spdk_scsi_dev *dev, + spdk_scsi_dev_destruct_cb_t cb_fn, void *cb_arg) { int lun_cnt; int i; - if (dev == NULL || dev->removed) { + if (dev == NULL) { + if (cb_fn) { + cb_fn(cb_arg, -EINVAL); + } + return; + } + + if (dev->removed) { + if (cb_fn) { + cb_fn(cb_arg, -EINVAL); + } return; } dev->removed = true; + dev->remove_cb = cb_fn; + dev->remove_ctx = cb_arg; lun_cnt = 0; for (i = 0; i < SPDK_SCSI_DEV_MAX_LUN; i++) { @@ -236,7 +254,7 @@ spdk_scsi_dev_construct(const char *name, const char *bdev_name_list[], rc = spdk_scsi_dev_add_lun(dev, bdev_name_list[i], lun_id_list[i], hotremove_cb, hotremove_ctx); if (rc < 0) { - spdk_scsi_dev_destruct(dev); + spdk_scsi_dev_destruct(dev, NULL, NULL); return NULL; } } diff --git a/lib/scsi/scsi_internal.h b/lib/scsi/scsi_internal.h index c13aec8fb6..7e6d9b329a 100644 --- a/lib/scsi/scsi_internal.h +++ b/lib/scsi/scsi_internal.h @@ -61,18 +61,20 @@ struct spdk_scsi_port { }; struct spdk_scsi_dev { - int id; - int is_allocated; - bool removed; + int id; + int is_allocated; + bool removed; + spdk_scsi_dev_destruct_cb_t remove_cb; + void *remove_ctx; - char name[SPDK_SCSI_DEV_MAX_NAME + 1]; + char name[SPDK_SCSI_DEV_MAX_NAME + 1]; - struct spdk_scsi_lun *lun[SPDK_SCSI_DEV_MAX_LUN]; + struct spdk_scsi_lun *lun[SPDK_SCSI_DEV_MAX_LUN]; - int num_ports; - struct spdk_scsi_port port[SPDK_SCSI_DEV_MAX_PORTS]; + int num_ports; + struct spdk_scsi_port port[SPDK_SCSI_DEV_MAX_PORTS]; - uint8_t protocol_id; + uint8_t protocol_id; }; struct spdk_scsi_lun_desc { diff --git a/lib/vhost/vhost_scsi.c b/lib/vhost/vhost_scsi.c index 304dc749ee..8137286f78 100644 --- a/lib/vhost/vhost_scsi.c +++ b/lib/vhost/vhost_scsi.c @@ -188,7 +188,7 @@ remove_scsi_tgt(struct spdk_vhost_scsi_dev *svdev, state->dev = NULL; assert(state->status == VHOST_SCSI_DEV_REMOVING); state->status = VHOST_SCSI_DEV_EMPTY; - spdk_scsi_dev_destruct(dev); + spdk_scsi_dev_destruct(dev, NULL, NULL); if (state->remove_cb) { state->remove_cb(&svdev->vdev, state->remove_ctx); state->remove_cb = NULL; diff --git a/test/unit/lib/iscsi/common.c b/test/unit/lib/iscsi/common.c index 8b3f81d9b0..d1a5f8ae13 100644 --- a/test/unit/lib/iscsi/common.c +++ b/test/unit/lib/iscsi/common.c @@ -161,7 +161,8 @@ DEFINE_STUB(spdk_scsi_dev_construct, struct spdk_scsi_dev *, void *hotremove_ctx), NULL); -DEFINE_STUB_V(spdk_scsi_dev_destruct, (struct spdk_scsi_dev *dev)); +DEFINE_STUB_V(spdk_scsi_dev_destruct, + (struct spdk_scsi_dev *dev, spdk_scsi_dev_destruct_cb_t cb_fn, void *cb_arg)); DEFINE_STUB(spdk_scsi_dev_add_port, int, (struct spdk_scsi_dev *dev, uint64_t id, const char *name), 0); diff --git a/test/unit/lib/scsi/dev.c/dev_ut.c b/test/unit/lib/scsi/dev.c/dev_ut.c index d4ec3daafd..1089593116 100644 --- a/test/unit/lib/scsi/dev.c/dev_ut.c +++ b/test/unit/lib/scsi/dev.c/dev_ut.c @@ -138,7 +138,7 @@ static void dev_destruct_null_dev(void) { /* pass null for the dev */ - spdk_scsi_dev_destruct(NULL); + spdk_scsi_dev_destruct(NULL, NULL, NULL); } static void @@ -149,7 +149,7 @@ dev_destruct_zero_luns(void) /* No luns attached to the dev */ /* free the dev */ - spdk_scsi_dev_destruct(&dev); + spdk_scsi_dev_destruct(&dev, NULL, NULL); } static void @@ -161,7 +161,7 @@ dev_destruct_null_lun(void) dev.lun[0] = NULL; /* free the dev */ - spdk_scsi_dev_destruct(&dev); + spdk_scsi_dev_destruct(&dev, NULL, NULL); } static void @@ -176,7 +176,7 @@ dev_destruct_success(void) CU_ASSERT(rc == 0); /* free the dev */ - spdk_scsi_dev_destruct(&dev); + spdk_scsi_dev_destruct(&dev, NULL, NULL); } @@ -256,7 +256,7 @@ dev_construct_success(void) CU_ASSERT_TRUE(dev != NULL); /* free the dev */ - spdk_scsi_dev_destruct(dev); + spdk_scsi_dev_destruct(dev, NULL, NULL); } static void @@ -273,7 +273,7 @@ dev_construct_success_lun_zero_not_first(void) CU_ASSERT_TRUE(dev != NULL); /* free the dev */ - spdk_scsi_dev_destruct(dev); + spdk_scsi_dev_destruct(dev, NULL, NULL); } static void @@ -297,7 +297,7 @@ dev_queue_mgmt_task_success(void) spdk_scsi_task_put(task); - spdk_scsi_dev_destruct(dev); + spdk_scsi_dev_destruct(dev, NULL, NULL); } static void @@ -320,7 +320,7 @@ dev_queue_task_success(void) spdk_scsi_task_put(task); - spdk_scsi_dev_destruct(dev); + spdk_scsi_dev_destruct(dev, NULL, NULL); } static void @@ -585,7 +585,7 @@ dev_add_lun_success1(void) CU_ASSERT_EQUAL(rc, 0); - spdk_scsi_dev_destruct(&dev); + spdk_scsi_dev_destruct(&dev, NULL, NULL); } static void @@ -598,7 +598,7 @@ dev_add_lun_success2(void) CU_ASSERT_EQUAL(rc, 0); - spdk_scsi_dev_destruct(&dev); + spdk_scsi_dev_destruct(&dev, NULL, NULL); } int