From 64a268e2e110831b1d430e606818b8903b6176b8 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Tue, 23 Oct 2018 13:10:37 +0900 Subject: [PATCH] iscsi: Fix double dequeue of the primary write task in error handling By running random, 64KB, 70% write, and 30% read workload and periodical TCP disconnection from the initiator, segmentation fault have occurred. By investigation, when the last write task completed and dequeued the primary write task in spdk_iscsi_task_cpl(), the primary write task was already dequeued in spdk_clear_all_transfer_task(). This patch dequeues all primary write tasks after all outstanding tasks complete. After starting to exit the connection, the connection will not receive any command. Hence waiting for completion of all pending tasks first will avoid double dequeue safely. Change-Id: I8e2b6b756be3a6e096675acc81cdb93fb6a7e350 Signed-off-by: Shuhei Matsumoto Reviewed-on: https://review.gerrithub.io/430397 Chandler-Test-Pool: SPDK Automated Test System Tested-by: SPDK CI Jenkins Reviewed-by: Changpeng Liu Reviewed-by: Jim Harris --- lib/iscsi/conn.c | 42 ++++++++++++++++++++++------ test/unit/lib/iscsi/conn.c/conn_ut.c | 6 ++++ 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/lib/iscsi/conn.c b/lib/iscsi/conn.c index 97c1e5503..d5cd5d1e9 100644 --- a/lib/iscsi/conn.c +++ b/lib/iscsi/conn.c @@ -517,17 +517,11 @@ _spdk_iscsi_conn_check_shutdown(void *arg) return -1; } -void -spdk_iscsi_conn_destruct(struct spdk_iscsi_conn *conn) +static void +_spdk_iscsi_conn_destruct(struct spdk_iscsi_conn *conn) { int rc; - conn->state = ISCSI_CONN_STATE_EXITED; - - if (conn->sess != NULL && conn->pending_task_cnt > 0) { - spdk_iscsi_conn_cleanup_backend(conn); - } - spdk_clear_all_transfer_task(conn, NULL); spdk_iscsi_poll_group_remove_conn_sock(conn); spdk_sock_close(&conn->sock); @@ -544,6 +538,38 @@ spdk_iscsi_conn_destruct(struct spdk_iscsi_conn *conn) } } +static int +_spdk_iscsi_conn_check_pending_tasks(void *arg) +{ + struct spdk_iscsi_conn *conn = arg; + + if (conn->dev != NULL && spdk_scsi_dev_has_pending_tasks(conn->dev)) { + return -1; + } + + spdk_poller_unregister(&conn->shutdown_timer); + + _spdk_iscsi_conn_destruct(conn); + + return -1; +} + +void +spdk_iscsi_conn_destruct(struct spdk_iscsi_conn *conn) +{ + conn->state = ISCSI_CONN_STATE_EXITED; + + if (conn->sess != NULL && conn->pending_task_cnt > 0) { + spdk_iscsi_conn_cleanup_backend(conn); + } + + if (conn->dev != NULL && spdk_scsi_dev_has_pending_tasks(conn->dev)) { + conn->shutdown_timer = spdk_poller_register(_spdk_iscsi_conn_check_pending_tasks, conn, 1000); + } else { + _spdk_iscsi_conn_destruct(conn); + } +} + static int spdk_iscsi_get_active_conns(void) { diff --git a/test/unit/lib/iscsi/conn.c/conn_ut.c b/test/unit/lib/iscsi/conn.c/conn_ut.c index 5a9c852c8..88d23423b 100644 --- a/test/unit/lib/iscsi/conn.c/conn_ut.c +++ b/test/unit/lib/iscsi/conn.c/conn_ut.c @@ -154,6 +154,12 @@ spdk_scsi_dev_get_lun(struct spdk_scsi_dev *dev, int lun_id) return NULL; } +bool +spdk_scsi_dev_has_pending_tasks(const struct spdk_scsi_dev *dev) +{ + return true; +} + int spdk_scsi_lun_open(struct spdk_scsi_lun *lun, spdk_scsi_remove_cb_t hotremove_cb, void *hotremove_ctx, struct spdk_scsi_desc **desc)