From ee7f9616552740806a5c8d4a5d0f624657944457 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Mon, 29 Jun 2020 18:11:48 +0900 Subject: [PATCH] lib/iscsi: Unify outstanding_r2t_tasks array and active_r2t_tasks list into the latter We have no particular requirement to keep both conn->outstanding_r2t_tasks array and conn->active_r2t_tasks list now. To improve readability and maintaineability, unify two into the latter, conn->outstanding_r2t_tasks list. Update unit test accordingly. Signed-off-by: Shuhei Matsumoto Change-Id: I25cf7cffbe39ac66e102eb3052340de6ef65c8f1 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3115 Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Changpeng Liu Reviewed-by: Jim Harris --- lib/iscsi/conn.c | 4 -- lib/iscsi/conn.h | 1 - lib/iscsi/iscsi.c | 90 ++++++++------------------ test/unit/lib/iscsi/iscsi.c/iscsi_ut.c | 4 +- 4 files changed, 29 insertions(+), 70 deletions(-) diff --git a/lib/iscsi/conn.c b/lib/iscsi/conn.c index 741b54be8..ead89e03c 100644 --- a/lib/iscsi/conn.c +++ b/lib/iscsi/conn.c @@ -259,10 +259,6 @@ iscsi_conn_construct(struct spdk_iscsi_portal *portal, conn->sess_param_state_negotiated[i] = false; } - for (i = 0; i < DEFAULT_MAXR2T; i++) { - conn->outstanding_r2t_tasks[i] = NULL; - } - conn->pdu_recv_state = ISCSI_PDU_RECV_STATE_AWAIT_PDU_READY; TAILQ_INIT(&conn->write_pdu_list); diff --git a/lib/iscsi/conn.h b/lib/iscsi/conn.h index 8c072025f..a85d2ddeb 100644 --- a/lib/iscsi/conn.h +++ b/lib/iscsi/conn.h @@ -138,7 +138,6 @@ struct spdk_iscsi_conn { TAILQ_HEAD(, spdk_iscsi_pdu) snack_pdu_list; int pending_r2t; - struct spdk_iscsi_task *outstanding_r2t_tasks[DEFAULT_MAXR2T]; uint16_t cid; diff --git a/lib/iscsi/iscsi.c b/lib/iscsi/iscsi.c index 5d9fc4c84..d370adea3 100644 --- a/lib/iscsi/iscsi.c +++ b/lib/iscsi/iscsi.c @@ -2734,7 +2734,6 @@ add_transfer_task(struct spdk_iscsi_conn *conn, struct spdk_iscsi_task *task) size_t segment_len; size_t data_len; int len; - int idx; int rc; int data_out_req; @@ -2756,9 +2755,8 @@ add_transfer_task(struct spdk_iscsi_conn *conn, struct spdk_iscsi_task *task) } conn->data_out_cnt += data_out_req; - idx = conn->pending_r2t++; + conn->pending_r2t++; - conn->outstanding_r2t_tasks[idx] = task; task->next_expected_r2t_offset = data_len; task->current_r2t_length = 0; task->R2TSN = 0; @@ -2813,20 +2811,14 @@ start_queued_transfer_tasks(struct spdk_iscsi_conn *conn) bool iscsi_del_transfer_task(struct spdk_iscsi_conn *conn, uint32_t task_tag) { - struct spdk_iscsi_task *task; - int i; + struct spdk_iscsi_task *task, *tmp; - for (i = 0; i < conn->pending_r2t; i++) { - if (conn->outstanding_r2t_tasks[i]->tag == task_tag) { - task = conn->outstanding_r2t_tasks[i]; + TAILQ_FOREACH_SAFE(task, &conn->active_r2t_tasks, link, tmp) { + if (task->tag == task_tag) { assert(conn->data_out_cnt >= task->data_out_cnt); conn->data_out_cnt -= task->data_out_cnt; conn->pending_r2t--; - for (; i < conn->pending_r2t; i++) { - conn->outstanding_r2t_tasks[i] = conn->outstanding_r2t_tasks[i + 1]; - } - conn->outstanding_r2t_tasks[conn->pending_r2t] = NULL; assert(task->is_r2t_active == true); TAILQ_REMOVE(&conn->active_r2t_tasks, task, link); @@ -2840,26 +2832,25 @@ iscsi_del_transfer_task(struct spdk_iscsi_conn *conn, uint32_t task_tag) return false; } -static void -del_connection_queued_task(struct spdk_iscsi_conn *conn, void *tailq, - struct spdk_scsi_lun *lun, - struct spdk_iscsi_pdu *pdu) +void iscsi_clear_all_transfer_task(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; - /* - * Temporary used to index spdk_scsi_task related - * queues of the connection. - */ - TAILQ_HEAD(queued_tasks, spdk_iscsi_task) *head; - head = (struct queued_tasks *)tailq; - - TAILQ_FOREACH_SAFE(task, head, link, task_tmp) { + TAILQ_FOREACH_SAFE(task, &conn->active_r2t_tasks, link, task_tmp) { pdu_tmp = iscsi_task_get_pdu(task); if ((lun == NULL || lun == task->scsi.lun) && (pdu == NULL || spdk_sn32_lt(pdu_tmp->cmd_sn, pdu->cmd_sn))) { - TAILQ_REMOVE(head, task, link); + task->outstanding_r2t = 0; + task->next_r2t_offset = 0; + task->next_expected_r2t_offset = 0; + assert(conn->data_out_cnt >= task->data_out_cnt); + conn->data_out_cnt -= task->data_out_cnt; + conn->pending_r2t--; + + TAILQ_REMOVE(&conn->active_r2t_tasks, task, link); task->is_r2t_active = false; if (lun != NULL && spdk_scsi_lun_is_removing(lun)) { spdk_scsi_task_process_null_lun(&task->scsi); @@ -2868,59 +2859,32 @@ del_connection_queued_task(struct spdk_iscsi_conn *conn, void *tailq, iscsi_task_put(task); } } -} -void iscsi_clear_all_transfer_task(struct spdk_iscsi_conn *conn, - struct spdk_scsi_lun *lun, - struct spdk_iscsi_pdu *pdu) -{ - int i, j, pending_r2t; - struct spdk_iscsi_task *task; - struct spdk_iscsi_pdu *pdu_tmp; - - pending_r2t = conn->pending_r2t; - for (i = 0; i < pending_r2t; i++) { - task = conn->outstanding_r2t_tasks[i]; + TAILQ_FOREACH_SAFE(task, &conn->queued_r2t_tasks, link, task_tmp) { pdu_tmp = iscsi_task_get_pdu(task); if ((lun == NULL || lun == task->scsi.lun) && (pdu == NULL || spdk_sn32_lt(pdu_tmp->cmd_sn, pdu->cmd_sn))) { - conn->outstanding_r2t_tasks[i] = NULL; - task->outstanding_r2t = 0; - task->next_r2t_offset = 0; - task->next_expected_r2t_offset = 0; - assert(conn->data_out_cnt >= task->data_out_cnt); - conn->data_out_cnt -= task->data_out_cnt; - conn->pending_r2t--; - } - } - - for (i = 0; i < pending_r2t; i++) { - if (conn->outstanding_r2t_tasks[i] != NULL) { - continue; - } - for (j = i + 1; j < pending_r2t; j++) { - if (conn->outstanding_r2t_tasks[j] != NULL) { - conn->outstanding_r2t_tasks[i] = conn->outstanding_r2t_tasks[j]; - conn->outstanding_r2t_tasks[j] = NULL; - break; + TAILQ_REMOVE(&conn->queued_r2t_tasks, task, link); + task->is_r2t_active = false; + if (lun != NULL && spdk_scsi_lun_is_removing(lun)) { + spdk_scsi_task_process_null_lun(&task->scsi); + iscsi_task_response(conn, task); } + iscsi_task_put(task); } } - del_connection_queued_task(conn, &conn->active_r2t_tasks, lun, pdu); - del_connection_queued_task(conn, &conn->queued_r2t_tasks, lun, pdu); - start_queued_transfer_tasks(conn); } static struct spdk_iscsi_task * get_transfer_task(struct spdk_iscsi_conn *conn, uint32_t transfer_tag) { - int i; + struct spdk_iscsi_task *task; - for (i = 0; i < conn->pending_r2t; i++) { - if (conn->outstanding_r2t_tasks[i]->ttt == transfer_tag) { - return (conn->outstanding_r2t_tasks[i]); + TAILQ_FOREACH(task, &conn->active_r2t_tasks, link) { + if (task->ttt == transfer_tag) { + return task; } } diff --git a/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c b/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c index 1a8298fdd..f96afd999 100644 --- a/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c +++ b/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c @@ -687,7 +687,6 @@ add_transfer_task_test(void) CU_ASSERT(conn.data_out_cnt == 255); CU_ASSERT(conn.pending_r2t == 1); - CU_ASSERT(conn.outstanding_r2t_tasks[0] == &task); CU_ASSERT(conn.ttt == 1); CU_ASSERT(task.data_out_cnt == 255); @@ -1871,6 +1870,7 @@ pdu_hdr_op_data_test(void) conn.sess = &sess; conn.dev = &dev; + TAILQ_INIT(&conn.active_r2t_tasks); /* Case 1 - SCSI Data-Out PDU is acceptable only on normal session. */ sess.session_type = SESSION_TYPE_DISCOVERY; @@ -1898,7 +1898,7 @@ pdu_hdr_op_data_test(void) */ primary.desired_data_transfer_length = SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH - 1; conn.pending_r2t = 1; - conn.outstanding_r2t_tasks[0] = &primary; + TAILQ_INSERT_TAIL(&conn.active_r2t_tasks, &primary, link); rc = iscsi_pdu_hdr_op_data(&conn, &pdu); CU_ASSERT(rc == SPDK_ISCSI_CONNECTION_FATAL);