From e79a29b264af787721eea2da3affbee951ea7b95 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Tue, 19 Nov 2019 15:04:43 +0900 Subject: [PATCH] lib/iscsi: Make spdk_del_transfer_task() return success/failure After recent refinement of LUN hotplug, it is possible that for large write I/O, primary task is freed doubly as a github issue is reported. However we could not notice the case because spdk_del_transfer_task() had not return success/failure, and to make matters worse, the second call of TAILQ_REMOVE() to the same header and instance caused no error if the first call succeeded. This patch changes spdk_del_transfer_task() to return success/failure. Besides, the next after patch expects the stub of spdk_del_transfer_task() returns true in the unit test, and hence do that. The next after patch will fix the issue of double free of primary task by using this patch. Signed-off-by: Shuhei Matsumoto Change-Id: Ibc0b65723050362d5fafa913417b64393feb874e Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475042 Tested-by: SPDK CI Jenkins Reviewed-by: Tomasz Zawadzki Reviewed-by: Jim Harris --- lib/iscsi/iscsi.c | 9 +++++---- lib/iscsi/iscsi.h | 2 +- test/unit/lib/iscsi/conn.c/conn_ut.c | 4 ++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/iscsi/iscsi.c b/lib/iscsi/iscsi.c index ef41c1f02..41bc6215a 100644 --- a/lib/iscsi/iscsi.c +++ b/lib/iscsi/iscsi.c @@ -2797,7 +2797,8 @@ start_queued_transfer_tasks(struct spdk_iscsi_conn *conn) } } -void spdk_del_transfer_task(struct spdk_iscsi_conn *conn, uint32_t task_tag) +bool +spdk_del_transfer_task(struct spdk_iscsi_conn *conn, uint32_t task_tag) { struct spdk_iscsi_task *task; int i; @@ -2812,11 +2813,11 @@ void spdk_del_transfer_task(struct spdk_iscsi_conn *conn, uint32_t task_tag) conn->outstanding_r2t_tasks[i] = conn->outstanding_r2t_tasks[i + 1]; } conn->outstanding_r2t_tasks[conn->pending_r2t] = NULL; - break; + start_queued_transfer_tasks(conn); + return true; } } - - start_queued_transfer_tasks(conn); + return false; } static void diff --git a/lib/iscsi/iscsi.h b/lib/iscsi/iscsi.h index 73fee1c98..146f8251e 100644 --- a/lib/iscsi/iscsi.h +++ b/lib/iscsi/iscsi.h @@ -426,7 +426,7 @@ void spdk_free_sess(struct spdk_iscsi_sess *sess); void spdk_clear_all_transfer_task(struct spdk_iscsi_conn *conn, struct spdk_scsi_lun *lun, struct spdk_iscsi_pdu *pdu); -void spdk_del_transfer_task(struct spdk_iscsi_conn *conn, uint32_t CmdSN); +bool spdk_del_transfer_task(struct spdk_iscsi_conn *conn, uint32_t CmdSN); bool spdk_iscsi_is_deferred_free_pdu(struct spdk_iscsi_pdu *pdu); void spdk_iscsi_task_cpl(struct spdk_scsi_task *scsi_task); diff --git a/test/unit/lib/iscsi/conn.c/conn_ut.c b/test/unit/lib/iscsi/conn.c/conn_ut.c index e64d66542..01290dd32 100644 --- a/test/unit/lib/iscsi/conn.c/conn_ut.c +++ b/test/unit/lib/iscsi/conn.c/conn_ut.c @@ -156,8 +156,8 @@ DEFINE_STUB_V(spdk_iscsi_task_mgmt_response, DEFINE_STUB_V(spdk_iscsi_send_nopin, (struct spdk_iscsi_conn *conn)); -DEFINE_STUB_V(spdk_del_transfer_task, - (struct spdk_iscsi_conn *conn, uint32_t task_tag)); +DEFINE_STUB(spdk_del_transfer_task, bool, + (struct spdk_iscsi_conn *conn, uint32_t task_tag), true); int spdk_iscsi_conn_handle_queued_datain_tasks(struct spdk_iscsi_conn *conn)