From b24bd0390a4f89e6d35fdc03cae3c8d7e8e42c18 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Thu, 28 Nov 2019 12:54:06 -0500 Subject: [PATCH] lib/iscsi: Simplify iscsi_conn_free_tasks() iscsi_conn_free_task() is used only when exiting connection now. Hence we can remove the parameter lun and simplify the function and its unit tests. Signed-off-by: Shuhei Matsumoto Change-Id: I6e7bf09672edca1f70c042ac58f098114d71ec78 Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/476115 Tested-by: SPDK CI Jenkins Reviewed-by: Changpeng Liu Reviewed-by: Jim Harris Reviewed-by: Ziye Yang --- lib/iscsi/conn.c | 30 ++++---------- test/unit/lib/iscsi/conn.c/conn_ut.c | 61 ++-------------------------- 2 files changed, 11 insertions(+), 80 deletions(-) diff --git a/lib/iscsi/conn.c b/lib/iscsi/conn.c index e11c56162..7d971768e 100644 --- a/lib/iscsi/conn.c +++ b/lib/iscsi/conn.c @@ -311,21 +311,19 @@ spdk_iscsi_conn_free_pdu(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pd spdk_put_pdu(pdu); } -static void -_iscsi_conn_free_tasks(struct spdk_iscsi_conn *conn, struct spdk_scsi_lun *lun) +static int +iscsi_conn_free_tasks(struct spdk_iscsi_conn *conn) { struct spdk_iscsi_pdu *pdu, *tmp_pdu; struct spdk_iscsi_task *iscsi_task, *tmp_iscsi_task; TAILQ_FOREACH_SAFE(pdu, &conn->snack_pdu_list, tailq, tmp_pdu) { - if (lun == NULL || lun == pdu->task->scsi.lun) { - TAILQ_REMOVE(&conn->snack_pdu_list, pdu, tailq); - spdk_iscsi_conn_free_pdu(conn, pdu); - } + TAILQ_REMOVE(&conn->snack_pdu_list, pdu, tailq); + spdk_iscsi_conn_free_pdu(conn, pdu); } TAILQ_FOREACH_SAFE(iscsi_task, &conn->queued_datain_tasks, link, tmp_iscsi_task) { - if ((!iscsi_task->is_queued) && (lun == NULL || lun == iscsi_task->scsi.lun)) { + if (!iscsi_task->is_queued) { TAILQ_REMOVE(&conn->queued_datain_tasks, iscsi_task, link); spdk_iscsi_task_put(iscsi_task); } @@ -338,23 +336,9 @@ _iscsi_conn_free_tasks(struct spdk_iscsi_conn *conn, struct spdk_scsi_lun *lun) * have to ensure there is no associated task in conn->queued_datain_tasks. */ TAILQ_FOREACH_SAFE(pdu, &conn->write_pdu_list, tailq, tmp_pdu) { - /* If connection is exited (no LUN is specified) or the PDU's LUN matches - * the LUN that was removed, free this PDU immediately. If the pdu's LUN - * is NULL, then we know the datain handling code already detected the hot - * removal, so we can free that PDU as well. - */ - if ((lun == NULL) || - (pdu->task && (lun == pdu->task->scsi.lun || NULL == pdu->task->scsi.lun))) { - TAILQ_REMOVE(&conn->write_pdu_list, pdu, tailq); - spdk_iscsi_conn_free_pdu(conn, pdu); - } + TAILQ_REMOVE(&conn->write_pdu_list, pdu, tailq); + spdk_iscsi_conn_free_pdu(conn, pdu); } -} - -static int -iscsi_conn_free_tasks(struct spdk_iscsi_conn *conn) -{ - _iscsi_conn_free_tasks(conn, NULL); if (conn->pending_task_cnt) { return -1; diff --git a/test/unit/lib/iscsi/conn.c/conn_ut.c b/test/unit/lib/iscsi/conn.c/conn_ut.c index bf7c5c4ad..61644f4ab 100644 --- a/test/unit/lib/iscsi/conn.c/conn_ut.c +++ b/test/unit/lib/iscsi/conn.c/conn_ut.c @@ -589,32 +589,13 @@ free_tasks_on_connection(void) TAILQ_INSERT_TAIL(&conn.write_pdu_list, &pdu4, tailq); /* Free all PDUs when exiting connection. */ - _iscsi_conn_free_tasks(&conn, NULL); + iscsi_conn_free_tasks(&conn); CU_ASSERT(TAILQ_EMPTY(&conn.write_pdu_list)); CU_ASSERT(task1.scsi.ref == 0); CU_ASSERT(task2.scsi.ref == 0); CU_ASSERT(task3.scsi.ref == 0); - task1.scsi.ref = 1; - task2.scsi.ref = 1; - task3.scsi.ref = 1; - TAILQ_INSERT_TAIL(&conn.write_pdu_list, &pdu1, tailq); - TAILQ_INSERT_TAIL(&conn.write_pdu_list, &pdu2, tailq); - TAILQ_INSERT_TAIL(&conn.write_pdu_list, &pdu3, tailq); - TAILQ_INSERT_TAIL(&conn.write_pdu_list, &pdu4, tailq); - - /* Free PDUs whose LUN matches the passed LUN or is NULL. */ - _iscsi_conn_free_tasks(&conn, &lun1); - - CU_ASSERT(!dequeue_pdu(&conn.write_pdu_list, &pdu1)); - CU_ASSERT(dequeue_pdu(&conn.write_pdu_list, &pdu2)); - CU_ASSERT(!dequeue_pdu(&conn.write_pdu_list, &pdu3)); - CU_ASSERT(dequeue_pdu(&conn.write_pdu_list, &pdu4)); - CU_ASSERT(task1.scsi.ref == 0); - CU_ASSERT(task2.scsi.ref == 1); - CU_ASSERT(task3.scsi.ref == 0); - /* Test conn->snack_pdu_list */ task1.scsi.ref = 1; @@ -625,7 +606,7 @@ free_tasks_on_connection(void) TAILQ_INSERT_TAIL(&conn.snack_pdu_list, &pdu3, tailq); /* Free all PDUs and associated tasks when exiting connection. */ - _iscsi_conn_free_tasks(&conn, NULL); + iscsi_conn_free_tasks(&conn); CU_ASSERT(!dequeue_pdu(&conn.snack_pdu_list, &pdu1)); CU_ASSERT(!dequeue_pdu(&conn.snack_pdu_list, &pdu2)); @@ -634,23 +615,6 @@ free_tasks_on_connection(void) CU_ASSERT(task2.scsi.ref == 0); CU_ASSERT(task3.scsi.ref == 0); - task1.scsi.ref = 1; - task2.scsi.ref = 1; - task3.scsi.ref = 1; - TAILQ_INSERT_TAIL(&conn.snack_pdu_list, &pdu1, tailq); - TAILQ_INSERT_TAIL(&conn.snack_pdu_list, &pdu2, tailq); - TAILQ_INSERT_TAIL(&conn.snack_pdu_list, &pdu3, tailq); - - /* Free all PDUs and free associated tasks whose lun matches the passed LUN. */ - _iscsi_conn_free_tasks(&conn, &lun1); - - CU_ASSERT(!dequeue_pdu(&conn.snack_pdu_list, &pdu1)); - CU_ASSERT(dequeue_pdu(&conn.snack_pdu_list, &pdu2)); - CU_ASSERT(dequeue_pdu(&conn.snack_pdu_list, &pdu3)); - CU_ASSERT(task1.scsi.ref == 0); - CU_ASSERT(task2.scsi.ref == 1); - CU_ASSERT(task3.scsi.ref == 1); - /* Test conn->queued_datain_tasks */ task1.scsi.ref = 1; @@ -661,7 +625,7 @@ free_tasks_on_connection(void) TAILQ_INSERT_TAIL(&conn.queued_datain_tasks, &task3, link); /* Free all tasks which is not queued when exiting connection. */ - _iscsi_conn_free_tasks(&conn, NULL); + iscsi_conn_free_tasks(&conn); CU_ASSERT(!dequeue_task(&conn.queued_datain_tasks, &task1)); CU_ASSERT(!dequeue_task(&conn.queued_datain_tasks, &task2)); @@ -669,23 +633,6 @@ free_tasks_on_connection(void) CU_ASSERT(task1.scsi.ref == 0); CU_ASSERT(task2.scsi.ref == 0); CU_ASSERT(task3.scsi.ref == 1); - - task1.scsi.ref = 1; - task2.scsi.ref = 1; - task3.scsi.ref = 1; - TAILQ_INSERT_TAIL(&conn.queued_datain_tasks, &task1, link); - TAILQ_INSERT_TAIL(&conn.queued_datain_tasks, &task2, link); - TAILQ_INSERT_TAIL(&conn.queued_datain_tasks, &task3, link); - - /* Free all tasks which is not queued and.whose LUN matches the passed LUN. */ - _iscsi_conn_free_tasks(&conn, &lun1); - - CU_ASSERT(!dequeue_task(&conn.queued_datain_tasks, &task1)); - CU_ASSERT(dequeue_task(&conn.queued_datain_tasks, &task2)); - CU_ASSERT(dequeue_task(&conn.queued_datain_tasks, &task3)); - CU_ASSERT(task1.scsi.ref == 0); - CU_ASSERT(task2.scsi.ref == 1); - CU_ASSERT(task3.scsi.ref == 1); } static void @@ -727,7 +674,7 @@ free_tasks_with_queued_datain(void) TAILQ_INSERT_TAIL(&conn.queued_datain_tasks, &task5, link); TAILQ_INSERT_TAIL(&conn.queued_datain_tasks, &task6, link); - _iscsi_conn_free_tasks(&conn, NULL); + iscsi_conn_free_tasks(&conn); CU_ASSERT(TAILQ_EMPTY(&conn.write_pdu_list)); CU_ASSERT(TAILQ_EMPTY(&conn.queued_datain_tasks));