From a207f575fb7331f74b1eda5a2eebff436681daef Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Mon, 10 Dec 2018 08:17:59 +0900 Subject: [PATCH] iscsi: Abort queued datain tasks before submitting abort task set to SCSI layer This patch is for ABORT TASK SET and LUN RESET. iSCSI layer doesn't have pending queue and hence if the target task is read task and is queued in queued_datain_tasks, it must be aborted before submitting ABORT TASK to SCSI layer. Aborting the target task may not complete by an iteration because submitted read tasks are limited. Hence use poller to complete abortion by repetition. Change-Id: Idabf4931d751ee698e9809eafd5c151b979f048b Signed-off-by: Shuhei Matsumoto Reviewed-on: https://review.gerrithub.io/436077 Tested-by: SPDK CI Jenkins Chandler-Test-Pool: SPDK Automated Test System Reviewed-by: Jim Harris Reviewed-by: Ben Walker --- lib/iscsi/iscsi.c | 57 +++++++++- test/unit/lib/iscsi/iscsi.c/iscsi_ut.c | 149 +++++++++++++++++++++++++ 2 files changed, 200 insertions(+), 6 deletions(-) diff --git a/lib/iscsi/iscsi.c b/lib/iscsi/iscsi.c index becae54d1..c3a3adbf3 100644 --- a/lib/iscsi/iscsi.c +++ b/lib/iscsi/iscsi.c @@ -3229,6 +3229,31 @@ spdk_iscsi_conn_abort_queued_datain_task(struct spdk_iscsi_conn *conn, return 0; } +static int +spdk_iscsi_conn_abort_queued_datain_tasks(struct spdk_iscsi_conn *conn, + struct spdk_scsi_lun *lun, + struct spdk_iscsi_pdu *pdu) +{ + struct spdk_iscsi_task *task, *task_tmp; + struct spdk_iscsi_pdu *pdu_tmp; + int rc; + + assert(pdu != NULL); + + TAILQ_FOREACH_SAFE(task, &conn->queued_datain_tasks, link, task_tmp) { + pdu_tmp = spdk_iscsi_task_get_pdu(task); + if ((lun == NULL || lun == task->scsi.lun) && + (SN32_LT(pdu_tmp->cmd_sn, pdu->cmd_sn))) { + rc = _spdk_iscsi_conn_abort_queued_datain_task(conn, task); + if (rc != 0) { + return rc; + } + } + } + + return 0; +} + static int _spdk_iscsi_op_abort_task(void *arg) { @@ -3254,6 +3279,30 @@ spdk_iscsi_op_abort_task(struct spdk_iscsi_task *task, uint32_t ref_task_tag) task->mgmt_poller = spdk_poller_register(_spdk_iscsi_op_abort_task, task, 10); } +static int +_spdk_iscsi_op_abort_task_set(void *arg) +{ + struct spdk_iscsi_task *task = arg; + int rc; + + rc = spdk_iscsi_conn_abort_queued_datain_tasks(task->conn, task->scsi.lun, + task->pdu); + if (rc != 0) { + return 0; + } + + spdk_poller_unregister(&task->mgmt_poller); + spdk_iscsi_queue_mgmt_task(task->conn, task); + return 1; +} + +static void +spdk_iscsi_op_abort_task_set(struct spdk_iscsi_task *task, uint8_t function) +{ + task->scsi.function = function; + task->mgmt_poller = spdk_poller_register(_spdk_iscsi_op_abort_task_set, task, 10); +} + static int spdk_iscsi_op_task(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) { @@ -3318,9 +3367,7 @@ spdk_iscsi_op_task(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) case ISCSI_TASK_FUNC_ABORT_TASK_SET: SPDK_NOTICELOG("ABORT_TASK_SET\n"); - task->scsi.function = SPDK_SCSI_TASK_FUNC_ABORT_TASK_SET; - spdk_iscsi_queue_mgmt_task(conn, task); - + spdk_iscsi_op_abort_task_set(task, SPDK_SCSI_TASK_FUNC_ABORT_TASK_SET); return SPDK_SUCCESS; case ISCSI_TASK_FUNC_CLEAR_TASK_SET: @@ -3336,9 +3383,7 @@ spdk_iscsi_op_task(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) case ISCSI_TASK_FUNC_LOGICAL_UNIT_RESET: SPDK_NOTICELOG("LOGICAL_UNIT_RESET\n"); - task->scsi.function = SPDK_SCSI_TASK_FUNC_LUN_RESET; - spdk_iscsi_queue_mgmt_task(conn, task); - + spdk_iscsi_op_abort_task_set(task, SPDK_SCSI_TASK_FUNC_LUN_RESET); return SPDK_SUCCESS; case ISCSI_TASK_FUNC_TARGET_WARM_RESET: diff --git a/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c b/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c index f5f5aaeae..3c6ed8c46 100644 --- a/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c +++ b/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c @@ -1070,6 +1070,153 @@ abort_queued_datain_task_test(void) CU_ASSERT(TAILQ_EMPTY(&conn.queued_datain_tasks)); } +static bool +datain_task_is_queued(struct spdk_iscsi_conn *conn, + struct spdk_iscsi_task *task) +{ + struct spdk_iscsi_task *tmp; + + TAILQ_FOREACH(tmp, &conn->queued_datain_tasks, link) { + if (tmp == task) { + return true; + } + } + return false; +} + +static void +abort_queued_datain_tasks_test(void) +{ + struct spdk_iscsi_conn conn; + struct spdk_iscsi_task *task1, *task2, *task3, *task4, *task5, *task6; + struct spdk_iscsi_task *task, *tmp; + struct spdk_iscsi_pdu *pdu1, *pdu2, *pdu3, *pdu4, *pdu5, *pdu6; + struct spdk_iscsi_pdu *mgmt_pdu1, *mgmt_pdu2; + struct spdk_scsi_lun lun1, lun2; + uint32_t alloc_cmd_sn; + int rc; + + TAILQ_INIT(&conn.queued_datain_tasks); + conn.data_in_cnt = 0; + + alloc_cmd_sn = 88; + + task1 = spdk_iscsi_task_get(&conn, NULL, NULL); + SPDK_CU_ASSERT_FATAL(task1 != NULL); + pdu1 = spdk_get_pdu(); + SPDK_CU_ASSERT_FATAL(pdu1 != NULL); + + pdu1->cmd_sn = alloc_cmd_sn; + alloc_cmd_sn++; + task1->current_datain_offset = 0; + task1->scsi.lun = &lun1; + spdk_iscsi_task_set_pdu(task1, pdu1); + TAILQ_INSERT_TAIL(&conn.queued_datain_tasks, task1, link); + + task2 = spdk_iscsi_task_get(&conn, NULL, NULL); + SPDK_CU_ASSERT_FATAL(task2 != NULL); + pdu2 = spdk_get_pdu(); + SPDK_CU_ASSERT_FATAL(pdu2 != NULL); + + pdu2->cmd_sn = alloc_cmd_sn; + alloc_cmd_sn++; + task2->current_datain_offset = 0; + task2->scsi.lun = &lun2; + spdk_iscsi_task_set_pdu(task2, pdu2); + TAILQ_INSERT_TAIL(&conn.queued_datain_tasks, task2, link); + + mgmt_pdu1 = spdk_get_pdu(); + SPDK_CU_ASSERT_FATAL(mgmt_pdu1 != NULL); + + mgmt_pdu1->cmd_sn = alloc_cmd_sn; + alloc_cmd_sn++; + + task3 = spdk_iscsi_task_get(&conn, NULL, NULL); + SPDK_CU_ASSERT_FATAL(task3 != NULL); + pdu3 = spdk_get_pdu(); + SPDK_CU_ASSERT_FATAL(pdu3 != NULL); + + pdu3->cmd_sn = alloc_cmd_sn; + alloc_cmd_sn++; + task3->current_datain_offset = 0; + task3->scsi.lun = &lun1; + spdk_iscsi_task_set_pdu(task3, pdu3); + TAILQ_INSERT_TAIL(&conn.queued_datain_tasks, task3, link); + + task4 = spdk_iscsi_task_get(&conn, NULL, NULL); + SPDK_CU_ASSERT_FATAL(task4 != NULL); + pdu4 = spdk_get_pdu(); + SPDK_CU_ASSERT_FATAL(pdu4 != NULL); + + pdu4->cmd_sn = alloc_cmd_sn; + alloc_cmd_sn++; + task4->current_datain_offset = 0; + task4->scsi.lun = &lun2; + spdk_iscsi_task_set_pdu(task4, pdu4); + TAILQ_INSERT_TAIL(&conn.queued_datain_tasks, task4, link); + + task5 = spdk_iscsi_task_get(&conn, NULL, NULL); + SPDK_CU_ASSERT_FATAL(task5 != NULL); + pdu5 = spdk_get_pdu(); + SPDK_CU_ASSERT_FATAL(pdu5 != NULL); + + pdu5->cmd_sn = alloc_cmd_sn; + alloc_cmd_sn++; + task5->current_datain_offset = 0; + task5->scsi.lun = &lun1; + spdk_iscsi_task_set_pdu(task5, pdu5); + TAILQ_INSERT_TAIL(&conn.queued_datain_tasks, task5, link); + + mgmt_pdu2 = spdk_get_pdu(); + SPDK_CU_ASSERT_FATAL(mgmt_pdu2 != NULL); + + mgmt_pdu2->cmd_sn = alloc_cmd_sn; + alloc_cmd_sn++; + + task6 = spdk_iscsi_task_get(&conn, NULL, NULL); + SPDK_CU_ASSERT_FATAL(task6 != NULL); + pdu6 = spdk_get_pdu(); + SPDK_CU_ASSERT_FATAL(pdu6 != NULL); + + pdu6->cmd_sn = alloc_cmd_sn; + alloc_cmd_sn++; + task6->current_datain_offset = 0; + task6->scsi.lun = &lun2; + spdk_iscsi_task_set_pdu(task6, pdu6); + TAILQ_INSERT_TAIL(&conn.queued_datain_tasks, task6, link); + + rc = spdk_iscsi_conn_abort_queued_datain_tasks(&conn, &lun1, mgmt_pdu1); + CU_ASSERT(rc == 0); + CU_ASSERT(!datain_task_is_queued(&conn, task1)); + CU_ASSERT(datain_task_is_queued(&conn, task2)); + CU_ASSERT(datain_task_is_queued(&conn, task3)); + CU_ASSERT(datain_task_is_queued(&conn, task4)); + CU_ASSERT(datain_task_is_queued(&conn, task5)); + CU_ASSERT(datain_task_is_queued(&conn, task6)); + + rc = spdk_iscsi_conn_abort_queued_datain_tasks(&conn, &lun2, mgmt_pdu2); + CU_ASSERT(rc == 0); + CU_ASSERT(!datain_task_is_queued(&conn, task2)); + CU_ASSERT(datain_task_is_queued(&conn, task3)); + CU_ASSERT(!datain_task_is_queued(&conn, task4)); + CU_ASSERT(datain_task_is_queued(&conn, task5)); + CU_ASSERT(datain_task_is_queued(&conn, task6)); + + TAILQ_FOREACH_SAFE(task, &conn.queued_datain_tasks, link, tmp) { + TAILQ_REMOVE(&conn.queued_datain_tasks, task, link); + spdk_iscsi_task_cpl(&task->scsi); + } + + spdk_put_pdu(mgmt_pdu2); + spdk_put_pdu(mgmt_pdu1); + spdk_put_pdu(pdu6); + spdk_put_pdu(pdu5); + spdk_put_pdu(pdu4); + spdk_put_pdu(pdu3); + spdk_put_pdu(pdu2); + spdk_put_pdu(pdu1); +} + int main(int argc, char **argv) { @@ -1104,6 +1251,8 @@ main(int argc, char **argv) clear_all_transfer_tasks_test) == NULL || CU_add_test(suite, "abort_queued_datain_task_test", abort_queued_datain_task_test) == NULL + || CU_add_test(suite, "abort_queued_datain_tasks_test", + abort_queued_datain_tasks_test) == NULL ) { CU_cleanup_registry(); return CU_get_error();