From fbb59ae25a242a8a2375427366a95ee1c883b343 Mon Sep 17 00:00:00 2001 From: Ziye Yang Date: Fri, 1 Nov 2019 18:41:06 +0800 Subject: [PATCH] iscsi: fix the recursive calling issue. After I review the function iscsi_conn_flush_pdus_internal, I think that it may cause recursive function call issue. One of the recursive calls in iscsi_conn_flush_pdus_internal is: spdk_iscsi_conn_free_pdu spdk_iscsi_conn_handle_queued_datain_tasks ... spdk_iscsi_task_cpl(&task->scsi); ... process_read_task_completion spdk_iscsi_task_response iscsi_transfer_in iscsi_send_datain spdk_iscsi_conn_write_pdu iscsi_conn_flush_pdus iscsi_conn_flush_pdus_internal So we have to create another list to solve this recursive issue in the while loop. And we face the the similar issue in NVMe/TCP before. With this patch, we can fix issues caused by recursive calls. Fixes #issue 1023 Signed-off-by: Ziye Yang Change-Id: I7150b962bfb30e74f53ba1a2a826fb78c73d8ea6 Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/472999 Reviewed-by: Jim Harris Reviewed-by: Shuhei Matsumoto Tested-by: SPDK CI Jenkins --- lib/iscsi/conn.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/lib/iscsi/conn.c b/lib/iscsi/conn.c index 9b5267147..ffdfcf18b 100644 --- a/lib/iscsi/conn.c +++ b/lib/iscsi/conn.c @@ -1253,6 +1253,7 @@ iscsi_conn_flush_pdus_internal(struct spdk_iscsi_conn *conn) uint32_t mapped_length = 0; struct spdk_iscsi_pdu *pdu; int pdu_length; + TAILQ_HEAD(, spdk_iscsi_pdu) completed_pdus_list; pdu = TAILQ_FIRST(&conn->write_pdu_list); @@ -1297,6 +1298,7 @@ iscsi_conn_flush_pdus_internal(struct spdk_iscsi_conn *conn) * partially written, update its writev_offset so that next * time only the unwritten portion will be sent to writev(). */ + TAILQ_INIT(&completed_pdus_list); while (bytes > 0) { pdu_length = iscsi_get_pdu_length(pdu, conn->header_digest, conn->data_digest); @@ -1305,18 +1307,7 @@ iscsi_conn_flush_pdus_internal(struct spdk_iscsi_conn *conn) if (bytes >= pdu_length) { bytes -= pdu_length; TAILQ_REMOVE(&conn->write_pdu_list, pdu, tailq); - - if ((conn->full_feature) && - (conn->sess->ErrorRecoveryLevel >= 1) && - spdk_iscsi_is_deferred_free_pdu(pdu)) { - SPDK_DEBUGLOG(SPDK_LOG_ISCSI, "stat_sn=%d\n", - from_be32(&pdu->bhs.stat_sn)); - TAILQ_INSERT_TAIL(&conn->snack_pdu_list, pdu, - tailq); - } else { - spdk_iscsi_conn_free_pdu(conn, pdu); - } - + TAILQ_INSERT_TAIL(&completed_pdus_list, pdu, tailq); pdu = TAILQ_FIRST(&conn->write_pdu_list); } else { pdu->writev_offset += bytes; @@ -1324,6 +1315,21 @@ iscsi_conn_flush_pdus_internal(struct spdk_iscsi_conn *conn) } } + while (!TAILQ_EMPTY(&completed_pdus_list)) { + pdu = TAILQ_FIRST(&completed_pdus_list); + TAILQ_REMOVE(&completed_pdus_list, pdu, tailq); + if ((conn->full_feature) && + (conn->sess->ErrorRecoveryLevel >= 1) && + spdk_iscsi_is_deferred_free_pdu(pdu)) { + SPDK_DEBUGLOG(SPDK_LOG_ISCSI, "stat_sn=%d\n", + from_be32(&pdu->bhs.stat_sn)); + TAILQ_INSERT_TAIL(&conn->snack_pdu_list, pdu, + tailq); + } else { + spdk_iscsi_conn_free_pdu(conn, pdu); + } + } + return TAILQ_EMPTY(&conn->write_pdu_list) ? 0 : 1; }