From 6415d84113484df1552fec0546025da79dda0355 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Thu, 28 Nov 2019 11:49:06 -0500 Subject: [PATCH] lib/iscsi: Introduce struct spdk_iscsi_lun to refine LUN hotplug Previously we had allocated temporary context, struct _iscsi_conn_remove_ctx to process LUN hotplug. However, we had to consider out of memory during connection is active, and we could not use any poller to process LUN hotplug because LUN hotplug may conflict with connection exit, and this possible conflict made us very difficult to use poller for LUN hotplug. Introducing struct spdk_iscsi_lun will resolve all these issues. Allocate struct spdk_iscsi_lun per LUN and store connection, LUN, and LUN's descriptor into the struct. Then use the struct for LUN hotplug and free the struct after LUN is closed when LUN is removed or connection exits. struct spdk_iscsi_lun is similar with struct spdk_nvmf_ns in NVMf library. Signed-off-by: Shuhei Matsumoto Change-Id: Ice26330f3948070c96d2fb53b94941be3b467079 Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/476113 Tested-by: SPDK CI Jenkins Reviewed-by: Changpeng Liu Reviewed-by: Jim Harris --- lib/iscsi/conn.c | 67 ++++++++++++++++++++++++++---------------------- lib/iscsi/conn.h | 9 ++++++- 2 files changed, 44 insertions(+), 32 deletions(-) diff --git a/lib/iscsi/conn.c b/lib/iscsi/conn.c index dc9890e6f..55caff8e7 100644 --- a/lib/iscsi/conn.c +++ b/lib/iscsi/conn.c @@ -258,7 +258,7 @@ spdk_iscsi_conn_construct(struct spdk_iscsi_portal *portal, TAILQ_INIT(&conn->queued_r2t_tasks); TAILQ_INIT(&conn->active_r2t_tasks); TAILQ_INIT(&conn->queued_datain_tasks); - memset(&conn->open_lun_descs, 0, sizeof(conn->open_lun_descs)); + memset(&conn->luns, 0, sizeof(conn->luns)); rc = spdk_sock_getaddr(sock, conn->target_addr, sizeof conn->target_addr, NULL, conn->initiator_addr, sizeof conn->initiator_addr, NULL); @@ -438,14 +438,18 @@ end: static void iscsi_conn_close_lun(struct spdk_iscsi_conn *conn, int lun_id) { - struct spdk_scsi_lun_desc *desc; + struct spdk_iscsi_lun *iscsi_lun; - desc = conn->open_lun_descs[lun_id]; - if (desc != NULL) { - spdk_scsi_lun_free_io_channel(desc); - spdk_scsi_lun_close(desc); - conn->open_lun_descs[lun_id] = NULL; + iscsi_lun = conn->luns[lun_id]; + if (iscsi_lun == NULL) { + return; } + + spdk_scsi_lun_free_io_channel(iscsi_lun->desc); + spdk_scsi_lun_close(iscsi_lun->desc); + free(iscsi_lun); + + conn->luns[lun_id] = NULL; } static void @@ -458,21 +462,14 @@ iscsi_conn_close_luns(struct spdk_iscsi_conn *conn) } } -struct _iscsi_conn_remove_ctx { - struct spdk_iscsi_conn *conn; - struct spdk_scsi_lun *lun; -}; - static void -_iscsi_conn_remove_lun(void *_ctx) +_iscsi_conn_remove_lun(void *ctx) { - struct _iscsi_conn_remove_ctx *ctx = _ctx; - struct spdk_iscsi_conn *conn = ctx->conn; - struct spdk_scsi_lun *lun = ctx->lun; + struct spdk_iscsi_lun *iscsi_lun = ctx; + struct spdk_iscsi_conn *conn = iscsi_lun->conn; + struct spdk_scsi_lun *lun = iscsi_lun->lun; int lun_id = spdk_scsi_lun_get_id(lun); - free(ctx); - assert(spdk_io_channel_get_thread(spdk_io_channel_from_ctx(conn->pg)) == spdk_get_thread()); @@ -491,19 +488,17 @@ static void iscsi_conn_remove_lun(struct spdk_scsi_lun *lun, void *remove_ctx) { struct spdk_iscsi_conn *conn = remove_ctx; - struct _iscsi_conn_remove_ctx *ctx; + int lun_id = spdk_scsi_lun_get_id(lun); + struct spdk_iscsi_lun *iscsi_lun; - ctx = calloc(1, sizeof(*ctx)); - if (!ctx) { - SPDK_ERRLOG("Unable to remove lun from connection\n"); + iscsi_lun = conn->luns[lun_id]; + if (iscsi_lun == NULL) { + SPDK_ERRLOG("LUN hotplug was notified to the unallocated LUN %d.\n", lun_id); return; } - ctx->conn = conn; - ctx->lun = lun; - spdk_thread_send_msg(spdk_io_channel_get_thread(spdk_io_channel_from_ctx(conn->pg)), - _iscsi_conn_remove_lun, ctx); + _iscsi_conn_remove_lun, iscsi_lun); } static int @@ -511,20 +506,30 @@ iscsi_conn_open_lun(struct spdk_iscsi_conn *conn, int lun_id, struct spdk_scsi_lun *lun) { int rc; - struct spdk_scsi_lun_desc *desc; + struct spdk_iscsi_lun *iscsi_lun; - rc = spdk_scsi_lun_open(lun, iscsi_conn_remove_lun, conn, &desc); + iscsi_lun = calloc(1, sizeof(*iscsi_lun)); + if (iscsi_lun == NULL) { + return -ENOMEM; + } + + iscsi_lun->conn = conn; + iscsi_lun->lun = lun; + + rc = spdk_scsi_lun_open(lun, iscsi_conn_remove_lun, conn, &iscsi_lun->desc); if (rc != 0) { + free(iscsi_lun); return rc; } - rc = spdk_scsi_lun_allocate_io_channel(desc); + rc = spdk_scsi_lun_allocate_io_channel(iscsi_lun->desc); if (rc != 0) { - spdk_scsi_lun_close(desc); + spdk_scsi_lun_close(iscsi_lun->desc); + free(iscsi_lun); return rc; } - conn->open_lun_descs[lun_id] = desc; + conn->luns[lun_id] = iscsi_lun; return 0; } diff --git a/lib/iscsi/conn.h b/lib/iscsi/conn.h index d99ac981c..852850eb0 100644 --- a/lib/iscsi/conn.h +++ b/lib/iscsi/conn.h @@ -82,6 +82,13 @@ enum iscsi_pdu_recv_state { }; struct spdk_poller; +struct spdk_iscsi_conn; + +struct spdk_iscsi_lun { + struct spdk_iscsi_conn *conn; + struct spdk_scsi_lun *lun; + struct spdk_scsi_lun_desc *desc; +}; struct spdk_iscsi_conn { int id; @@ -188,7 +195,7 @@ struct spdk_iscsi_conn { TAILQ_HEAD(active_r2t_tasks, spdk_iscsi_task) active_r2t_tasks; TAILQ_HEAD(queued_datain_tasks, spdk_iscsi_task) queued_datain_tasks; - struct spdk_scsi_lun_desc *open_lun_descs[SPDK_SCSI_DEV_MAX_LUN]; + struct spdk_iscsi_lun *luns[SPDK_SCSI_DEV_MAX_LUN]; }; extern struct spdk_iscsi_conn *g_conns_array;