From c5b00933ceea0e1e4f2f2b563699ab8cad0742e5 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Wed, 11 Jul 2018 14:24:10 +0900 Subject: [PATCH] iscsi: Fix the issue that queued iSCSI tasks are not migrated when clearing tasks When multiple LUNs are attached and spdk_clear_all_transfer_tasks() is called with a LUN, all tasks whose target is the LUN are removed and there will be room to start queued tasks for other LUNs. However currently no migration is done. Add UT code to verify the fix too. Change-Id: I082d370ab86a46e5b4a74a16293a572fae663add Signed-off-by: Shuhei Matsumoto Reviewed-on: https://review.gerrithub.io/418765 Chandler-Test-Pool: SPDK Automated Test System Reviewed-by: Ben Walker Reviewed-by: Seth Howell Reviewed-by: Jim Harris Reviewed-by: Ziye Yang Reviewed-by: Changpeng Liu Tested-by: SPDK CI Jenkins --- lib/iscsi/iscsi.c | 38 ++++++++++++++++---------- test/unit/lib/iscsi/iscsi.c/iscsi_ut.c | 4 +-- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/lib/iscsi/iscsi.c b/lib/iscsi/iscsi.c index ee849135a..e6fdf111d 100644 --- a/lib/iscsi/iscsi.c +++ b/lib/iscsi/iscsi.c @@ -3516,9 +3516,28 @@ spdk_add_transfer_task(struct spdk_iscsi_conn *conn, return SPDK_SUCCESS; } -void spdk_del_transfer_task(struct spdk_iscsi_conn *conn, uint32_t task_tag) +/* If there are additional large writes queued for R2Ts, start them now. + * This is called when a large write is just completed or when multiple LUNs + * are attached and large write tasks for the specific LUN are cleared. + */ +static void +spdk_start_queued_transfer_tasks(struct spdk_iscsi_conn *conn) { struct spdk_iscsi_task *task, *tmp; + + TAILQ_FOREACH_SAFE(task, &conn->queued_r2t_tasks, link, tmp) { + if (conn->pending_r2t < DEFAULT_MAXR2T) { + TAILQ_REMOVE(&conn->queued_r2t_tasks, task, link); + spdk_add_transfer_task(conn, task); + } else { + break; + } + } +} + +void spdk_del_transfer_task(struct spdk_iscsi_conn *conn, uint32_t task_tag) +{ + struct spdk_iscsi_task *task; int i; for (i = 0; i < conn->pending_r2t; i++) { @@ -3535,20 +3554,7 @@ void spdk_del_transfer_task(struct spdk_iscsi_conn *conn, uint32_t task_tag) } } - /* - * A large write was just completed, so if there are additional large - * writes queued for R2Ts, start them now. But first check to make - * sure each of the tasks will fit without the connection's allotment - * for total R2T tasks. - */ - TAILQ_FOREACH_SAFE(task, &conn->queued_r2t_tasks, link, tmp) { - if (conn->pending_r2t < DEFAULT_MAXR2T) { - TAILQ_REMOVE(&conn->queued_r2t_tasks, task, link); - spdk_add_transfer_task(conn, task); - } else { - break; - } - } + spdk_start_queued_transfer_tasks(conn); } static void @@ -3604,6 +3610,8 @@ void spdk_clear_all_transfer_task(struct spdk_iscsi_conn *conn, spdk_del_connection_queued_task(&conn->active_r2t_tasks, lun); spdk_del_connection_queued_task(&conn->queued_r2t_tasks, lun); + + spdk_start_queued_transfer_tasks(conn); } /* This function is used to handle the r2t snack */ diff --git a/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c b/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c index d5ca9096a..7c459f312 100644 --- a/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c +++ b/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c @@ -886,18 +886,18 @@ clear_all_transfer_tasks_test(void) spdk_clear_all_transfer_task(&conn, &lun1); + CU_ASSERT(TAILQ_EMPTY(&conn.queued_r2t_tasks)); CU_ASSERT(spdk_get_transfer_task(&conn, 1) == NULL); CU_ASSERT(spdk_get_transfer_task(&conn, 2) == NULL); CU_ASSERT(spdk_get_transfer_task(&conn, 3) == NULL); CU_ASSERT(spdk_get_transfer_task(&conn, 4) == task4); - CU_ASSERT(spdk_get_transfer_task(&conn, 5) == NULL); + CU_ASSERT(spdk_get_transfer_task(&conn, 5) == task5); spdk_clear_all_transfer_task(&conn, NULL); CU_ASSERT(spdk_get_transfer_task(&conn, 4) == NULL); CU_ASSERT(spdk_get_transfer_task(&conn, 5) == NULL); - CU_ASSERT(TAILQ_EMPTY(&conn.queued_r2t_tasks)); CU_ASSERT(TAILQ_EMPTY(&conn.active_r2t_tasks)); while (!TAILQ_EMPTY(&g_write_pdu_list)) { pdu = TAILQ_FIRST(&g_write_pdu_list);