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

lib/iscsi: Stop splitting immediately when aborting SCSI read I/O 



We changed read I/O submission to stop splitting when detecting LUN
hotplug very recently. We can do the same refinement for read I/O
abortion.

In _iscsi_conn_abort_queued_datain_task(), set all remaining length
to the new task and complete it immediately. We keep the code to
process the case that queued_datain_task completed but is still in
queue, but we can change its if condition to assert.

Simplify the corresponding unit tests accordingly, and set
task->scsi.transfer_len in abort_queued_datain_tasks_test() to
exercise the changed paths.

In iscsi_pdu_payload_po_scsi_read(), if task->scsi.transfer_len is not
larger than SPDK_BDEV_LARGE_BUF_MAX_SIZE, no minimum calculation is
necessary and we can substitute task->scsi.transfer_len to
task->scsi.length simply.  This change is too small to be an independent
patch and is done together.

Signed-off-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Iea93e51b103eae141a007a0abdaf13cbe6d5287f
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/476984


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

lib/iscsi/iscsi.c

+23 −22
Original line number Diff line number Diff line
@@ -3260,7 +3260,7 @@ iscsi_pdu_payload_op_scsi_read(struct spdk_iscsi_conn *conn, struct spdk_iscsi_t 
	if (task->scsi.transfer_len <= SPDK_BDEV_LARGE_BUF_MAX_SIZE) {

		task->parent = NULL;

		task->scsi.offset = 0;

		task->scsi.length = DMIN32(SPDK_BDEV_LARGE_BUF_MAX_SIZE, task->scsi.transfer_len);

		task->scsi.length = task->scsi.transfer_len;

		spdk_scsi_task_set_data(&task->scsi, NULL, 0);



		iscsi_queue_task(conn, task);
@@ -3562,15 +3562,18 @@ _iscsi_conn_abort_queued_datain_task(struct spdk_iscsi_conn *conn, 
	struct spdk_iscsi_task *subtask;

	uint32_t remaining_size;



	while (conn->data_in_cnt < MAX_LARGE_DATAIN_PER_CONNECTION) {

	if (conn->data_in_cnt >= MAX_LARGE_DATAIN_PER_CONNECTION) {

		return -1;

	}



	assert(task->current_datain_offset <= task->scsi.transfer_len);

		/* If any IO is submitted already, abort all subtasks by repetition. */

	/* Stop split and abort read I/O for remaining data. */

	if (task->current_datain_offset < task->scsi.transfer_len) {

		remaining_size = task->scsi.transfer_len - task->current_datain_offset;

		subtask = spdk_iscsi_task_get(conn, task, spdk_iscsi_task_cpl);

		assert(subtask != NULL);

		subtask->scsi.offset = task->current_datain_offset;

			subtask->scsi.length = DMIN32(SPDK_BDEV_LARGE_BUF_MAX_SIZE, remaining_size);

		subtask->scsi.length = remaining_size;

		spdk_scsi_task_set_data(&subtask->scsi, NULL, 0);

		task->current_datain_offset += subtask->scsi.length;
@@ -3579,15 +3582,13 @@ _iscsi_conn_abort_queued_datain_task(struct spdk_iscsi_conn *conn, 
		spdk_iscsi_task_cpl(&subtask->scsi);

	}



		/* Remove the primary task from the list if this is the last subtask */

		if (task->current_datain_offset == task->scsi.transfer_len) {

	/* Remove the primary task from the list because all subtasks are submitted

	 *  or aborted.

	 */

	assert(task->current_datain_offset == task->scsi.transfer_len);

	TAILQ_REMOVE(&conn->queued_datain_tasks, task, link);

	return 0;

}

	}



	return -1;

}



static int

iscsi_conn_abort_queued_datain_task(struct spdk_iscsi_conn *conn,

test/unit/lib/iscsi/iscsi.c/iscsi_ut.c

+15 −50
Original line number Diff line number Diff line
@@ -1105,7 +1105,7 @@ static void 
abort_queued_datain_task_test(void)

{

	struct spdk_iscsi_conn conn = {};

	struct spdk_iscsi_task *task, *task2, *task3;

	struct spdk_iscsi_task *task;

	int rc;



	TAILQ_INIT(&conn.queued_datain_tasks);
@@ -1124,18 +1124,17 @@ abort_queued_datain_task_test(void) 
	rc = _iscsi_conn_abort_queued_datain_task(&conn, task);

	CU_ASSERT(rc != 0);

	CU_ASSERT(!TAILQ_EMPTY(&conn.queued_datain_tasks));

	assert(conn.data_in_cnt == MAX_LARGE_DATAIN_PER_CONNECTION);



	/* havs slots for sub read tasks */

	conn.data_in_cnt = 0;

	rc = _iscsi_conn_abort_queued_datain_task(&conn, task);

	CU_ASSERT(rc == 0);

	CU_ASSERT(TAILQ_EMPTY(&conn.queued_datain_tasks));

	assert(conn.data_in_cnt == 0);

	CU_ASSERT(task->current_datain_offset == SPDK_BDEV_LARGE_BUF_MAX_SIZE * 3);



	/* Case2: Queue one task, and this task is partially executed,

	 * and the slot of sub read tasks are full

	 */

	/* Case2: Queue one task, and this task is partially executed */



	/* No slots for sub read tasks */

	conn.data_in_cnt = MAX_LARGE_DATAIN_PER_CONNECTION;

	task->scsi.transfer_len = SPDK_BDEV_LARGE_BUF_MAX_SIZE * 3;

	task->current_datain_offset = SPDK_BDEV_LARGE_BUF_MAX_SIZE;
@@ -1143,55 +1142,15 @@ abort_queued_datain_task_test(void) 


	rc = _iscsi_conn_abort_queued_datain_task(&conn, task);

	CU_ASSERT(rc != 0);

	CU_ASSERT(!TAILQ_EMPTY(&conn.queued_datain_tasks));



	/* Case3: Queue one task, and this task is partially executed,

	 * and the slot of sub read tasks is not full.

	 */

	conn.data_in_cnt = MAX_LARGE_DATAIN_PER_CONNECTION - 2;

	rc = _iscsi_conn_abort_queued_datain_task(&conn, task);

	CU_ASSERT(rc == 0);

	assert(conn.data_in_cnt == MAX_LARGE_DATAIN_PER_CONNECTION - 2);

	CU_ASSERT(TAILQ_EMPTY(&conn.queued_datain_tasks));



	/* Case4: Queue three tasks and abort each task sequentially */

	/* have slots for sub read tasks */

	conn.data_in_cnt = 0;



	task->tag = 1;

	task->scsi.transfer_len = SPDK_BDEV_LARGE_BUF_MAX_SIZE * 3;

	task->current_datain_offset = 0;

	TAILQ_INSERT_TAIL(&conn.queued_datain_tasks, task, link);



	task2 = spdk_iscsi_task_get(&conn, NULL, NULL);

	SPDK_CU_ASSERT_FATAL(task2 != NULL);

	task2->tag = 2;

	task2->scsi.dxfer_dir = SPDK_SCSI_DIR_FROM_DEV;

	task2->scsi.transfer_len = SPDK_BDEV_LARGE_BUF_MAX_SIZE * 4;

	task2->current_datain_offset = SPDK_BDEV_LARGE_BUF_MAX_SIZE;

	TAILQ_INSERT_TAIL(&conn.queued_datain_tasks, task2, link);



	task3 = spdk_iscsi_task_get(&conn, NULL, NULL);

	SPDK_CU_ASSERT_FATAL(task3 != NULL);

	task3->tag = 3;

	task3->scsi.dxfer_dir = SPDK_SCSI_DIR_FROM_DEV;

	task3->scsi.transfer_len = SPDK_BDEV_LARGE_BUF_MAX_SIZE * 5;

	task3->current_datain_offset = SPDK_BDEV_LARGE_BUF_MAX_SIZE * 2;

	TAILQ_INSERT_TAIL(&conn.queued_datain_tasks, task3, link);



	rc = iscsi_conn_abort_queued_datain_task(&conn, 1);

	CU_ASSERT(rc == 0);



	rc = iscsi_conn_abort_queued_datain_task(&conn, 2);

	CU_ASSERT(rc == 0);



	rc = iscsi_conn_abort_queued_datain_task(&conn, 3);

	rc = _iscsi_conn_abort_queued_datain_task(&conn, task);

	CU_ASSERT(rc == 0);



	CU_ASSERT(TAILQ_EMPTY(&conn.queued_datain_tasks));

	assert(conn.data_in_cnt == 0);

	CU_ASSERT(task->current_datain_offset == SPDK_BDEV_LARGE_BUF_MAX_SIZE * 3);



	spdk_iscsi_task_cpl(&task->scsi);

	spdk_iscsi_task_cpl(&task2->scsi);

	spdk_iscsi_task_cpl(&task3->scsi);

}



static bool
@@ -1233,6 +1192,7 @@ abort_queued_datain_tasks_test(void) 
	pdu1->cmd_sn = alloc_cmd_sn;

	alloc_cmd_sn++;

	task1->current_datain_offset = 0;

	task1->scsi.transfer_len = 512;

	task1->scsi.lun = &lun1;

	spdk_iscsi_task_set_pdu(task1, pdu1);

	TAILQ_INSERT_TAIL(&conn.queued_datain_tasks, task1, link);
@@ -1245,6 +1205,7 @@ abort_queued_datain_tasks_test(void) 
	pdu2->cmd_sn = alloc_cmd_sn;

	alloc_cmd_sn++;

	task2->current_datain_offset = 0;

	task2->scsi.transfer_len = 512;

	task2->scsi.lun = &lun2;

	spdk_iscsi_task_set_pdu(task2, pdu2);

	TAILQ_INSERT_TAIL(&conn.queued_datain_tasks, task2, link);
@@ -1263,6 +1224,7 @@ abort_queued_datain_tasks_test(void) 
	pdu3->cmd_sn = alloc_cmd_sn;

	alloc_cmd_sn++;

	task3->current_datain_offset = 0;

	task3->scsi.transfer_len = 512;

	task3->scsi.lun = &lun1;

	spdk_iscsi_task_set_pdu(task3, pdu3);

	TAILQ_INSERT_TAIL(&conn.queued_datain_tasks, task3, link);
@@ -1275,6 +1237,7 @@ abort_queued_datain_tasks_test(void) 
	pdu4->cmd_sn = alloc_cmd_sn;

	alloc_cmd_sn++;

	task4->current_datain_offset = 0;

	task4->scsi.transfer_len = 512;

	task4->scsi.lun = &lun2;

	spdk_iscsi_task_set_pdu(task4, pdu4);

	TAILQ_INSERT_TAIL(&conn.queued_datain_tasks, task4, link);
@@ -1287,6 +1250,7 @@ abort_queued_datain_tasks_test(void) 
	pdu5->cmd_sn = alloc_cmd_sn;

	alloc_cmd_sn++;

	task5->current_datain_offset = 0;

	task5->scsi.transfer_len = 512;

	task5->scsi.lun = &lun1;

	spdk_iscsi_task_set_pdu(task5, pdu5);

	TAILQ_INSERT_TAIL(&conn.queued_datain_tasks, task5, link);
@@ -1305,6 +1269,7 @@ abort_queued_datain_tasks_test(void) 
	pdu6->cmd_sn = alloc_cmd_sn;

	alloc_cmd_sn++;

	task6->current_datain_offset = 0;

	task6->scsi.transfer_len = 512;

	task6->scsi.lun = &lun2;

	spdk_iscsi_task_set_pdu(task6, pdu6);

	TAILQ_INSERT_TAIL(&conn.queued_datain_tasks, task6, link);