Commit acab54c1 authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Tomasz Zawadzki
Browse files

iscsi: correctly free the deferred pdu for ERL > 0 case 



The type of pdu deferred to be free only have
two types, R2T or DATA_IN. And the two types of pdus
are all assoicated a task, so updateing both the code and unit test case.

Also for all pdu free, we should use spdk_iscsi_conn_free function since
for normal pdu free, we all use this function.

PS: I also tested the calsoft local, it does not trigger the assert.

Fixes #1074.

Signed-off-by: default avatarZiye Yang <ziye.yang@intel.com>
Signed-off-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I0524965baf5349a100210ef717aedaa5f8ff105e
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475657


Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent 64e0e380

lib/iscsi/conn.c

+3 −4
Original line number Diff line number Diff line
@@ -331,11 +331,10 @@ _iscsi_conn_free_tasks(struct spdk_iscsi_conn *conn, struct spdk_scsi_lun *lun) 
	}



	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);

		if (pdu->task && (lun == NULL || lun == pdu->task->scsi.lun)) {

			spdk_iscsi_task_put(pdu->task);

			spdk_iscsi_conn_free_pdu(conn, pdu);

		}

		spdk_put_pdu(pdu);

	}



	TAILQ_FOREACH_SAFE(iscsi_task, &conn->queued_datain_tasks, link, tmp_iscsi_task) {

lib/iscsi/iscsi.c

+2 −8
Original line number Diff line number Diff line
@@ -2698,10 +2698,7 @@ iscsi_send_r2t_recovery(struct spdk_iscsi_conn *conn, 
				task->next_expected_r2t_offset));



		/* remove the old_r2t_pdu */

		if (pdu->task) {

			spdk_iscsi_task_put(pdu->task);

		}

		spdk_put_pdu(pdu);

		spdk_iscsi_conn_free_pdu(conn, pdu);



		/* re-send a new r2t pdu */

		rc = iscsi_send_r2t(conn, task, task->next_expected_r2t_offset,
@@ -4260,10 +4257,7 @@ iscsi_handle_data_ack(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) 
			if ((from_be32(&datain_header->ttt) == transfer_tag) &&

			    (old_datasn == beg_run - 1)) {

				TAILQ_REMOVE(&conn->snack_pdu_list, old_pdu, tailq);

				if (old_pdu->task) {

					spdk_iscsi_task_put(old_pdu->task);

				}

				spdk_put_pdu(old_pdu);

				spdk_iscsi_conn_free_pdu(conn, old_pdu);

				break;

			}

		}

test/unit/lib/iscsi/conn.c/conn_ut.c

+2 −6
Original line number Diff line number Diff line
@@ -536,7 +536,6 @@ free_tasks_on_connection(void) 
	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);

	TAILQ_INSERT_TAIL(&conn.snack_pdu_list, &pdu4, tailq);



	/* Free all PDUs and associated tasks when exiting connection. */

	_iscsi_conn_free_tasks(&conn, NULL);
@@ -544,7 +543,6 @@ free_tasks_on_connection(void) 
	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(!dequeue_pdu(&conn.snack_pdu_list, &pdu4));

	CU_ASSERT(task1.scsi.ref == 0);

	CU_ASSERT(task2.scsi.ref == 0);

	CU_ASSERT(task3.scsi.ref == 0);
@@ -555,15 +553,13 @@ free_tasks_on_connection(void) 
	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);

	TAILQ_INSERT_TAIL(&conn.snack_pdu_list, &pdu4, 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(!dequeue_pdu(&conn.snack_pdu_list, &pdu4));

	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);