iscsi: Abort queued datain task before submitting abort to SCSI layer

By up to the previous patch in the patch series, unexpected behavior
due to write tasks in task management commands have been fixed.

But unexpected behavior due to read tasks in task management commands
have been still observed.

Remaining patches in the patch series will fix the unexpected behavior
due to read tasks in task management commands.

This patch is for ABORT TASK.

ABORT TASK is not supported in SCSI layer yet. But the initiator
doesn't care about the failure is due to not-supported or failure.
It must be avoided that the task management command returns SCSI Good
but some tasks are not aborted and return SCSI Good later. On the other
hand, it is acceptable that the task management command returns
failure but some tasks are partially aborted.

Hence this patch adds operation without checking the support status
in SCSI layer.

iSCSI layer doesn't have pending queue and hence if the target task
is read task and is queued in queued_datain_tasks, it must be
aborted before submitting ABORT TASK to SCSI layer.

Aborting the target task may not complete by an iteration because
submitted read tasks are limited. Hence use poller to complete abortion
by repetition.

Change-Id: I030a8b2f19c2f7c7d2f7b0b2c633579534db631b
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/436076
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
This commit is contained in:
Shuhei Matsumoto 2018-12-10 08:12:11 +09:00 committed by Ben Walker
parent 728972cdf3
commit 5124f87f4e
4 changed files with 185 additions and 4 deletions

View File

@ -3170,6 +3170,90 @@ spdk_get_transfer_task(struct spdk_iscsi_conn *conn, uint32_t transfer_tag)
return NULL;
}
static int
_spdk_iscsi_conn_abort_queued_datain_task(struct spdk_iscsi_conn *conn,
struct spdk_iscsi_task *task)
{
struct spdk_iscsi_task *subtask;
uint32_t remaining_size;
while (conn->data_in_cnt < MAX_LARGE_DATAIN_PER_CONNECTION) {
assert(task->current_datain_offset <= task->scsi.transfer_len);
/* If no IO is submitted yet, just abort the primary task. */
if (task->current_datain_offset == 0) {
TAILQ_REMOVE(&conn->queued_datain_tasks, task, link);
spdk_scsi_task_process_abort(&task->scsi);
spdk_iscsi_task_cpl(&task->scsi);
return 0;
}
/* If any IO is submitted already, abort all subtasks by repetition. */
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);
spdk_scsi_task_set_data(&subtask->scsi, NULL, 0);
task->current_datain_offset += subtask->scsi.length;
conn->data_in_cnt++;
subtask->scsi.transfer_len = subtask->scsi.length;
spdk_scsi_task_process_abort(&subtask->scsi);
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) {
TAILQ_REMOVE(&conn->queued_datain_tasks, task, link);
return 0;
}
}
}
return -1;
}
static int
spdk_iscsi_conn_abort_queued_datain_task(struct spdk_iscsi_conn *conn,
uint32_t ref_task_tag)
{
struct spdk_iscsi_task *task;
TAILQ_FOREACH(task, &conn->queued_datain_tasks, link) {
if (task->tag == ref_task_tag) {
return _spdk_iscsi_conn_abort_queued_datain_task(conn, task);
}
}
return 0;
}
static int
_spdk_iscsi_op_abort_task(void *arg)
{
struct spdk_iscsi_task *task = arg;
int rc;
rc = spdk_iscsi_conn_abort_queued_datain_task(task->conn,
task->scsi.abort_id);
if (rc != 0) {
return 0;
}
spdk_poller_unregister(&task->mgmt_poller);
spdk_iscsi_queue_mgmt_task(task->conn, task);
return 1;
}
static void
spdk_iscsi_op_abort_task(struct spdk_iscsi_task *task, uint32_t ref_task_tag)
{
task->scsi.abort_id = ref_task_tag;
task->scsi.function = SPDK_SCSI_TASK_FUNC_ABORT_TASK;
task->mgmt_poller = spdk_poller_register(_spdk_iscsi_op_abort_task, task, 10);
}
static int
spdk_iscsi_op_task(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu)
{
@ -3226,9 +3310,7 @@ spdk_iscsi_op_task(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu)
case ISCSI_TASK_FUNC_ABORT_TASK:
SPDK_NOTICELOG("ABORT_TASK\n");
task->scsi.abort_id = ref_task_tag;
task->scsi.function = SPDK_SCSI_TASK_FUNC_ABORT_TASK;
spdk_iscsi_queue_mgmt_task(conn, task);
spdk_iscsi_op_abort_task(task, ref_task_tag);
return SPDK_SUCCESS;

View File

@ -93,6 +93,8 @@ struct spdk_iscsi_task {
*/
int lun_id;
struct spdk_poller *mgmt_poller;
TAILQ_ENTRY(spdk_iscsi_task) link;
TAILQ_HEAD(subtask_list, spdk_iscsi_task) subtask_list;

View File

@ -24,7 +24,10 @@ spdk_iscsi_task_get(struct spdk_iscsi_conn *conn,
struct spdk_iscsi_task *task;
task = calloc(1, sizeof(*task));
if (!task) {
return NULL;
}
task->scsi.iovs = &task->scsi.iov;
return task;
}
@ -76,6 +79,11 @@ spdk_scsi_task_process_null_lun(struct spdk_scsi_task *task)
{
}
void
spdk_scsi_task_process_abort(struct spdk_scsi_task *task)
{
}
void
spdk_scsi_dev_queue_task(struct spdk_scsi_dev *dev,
struct spdk_scsi_task *task)
@ -214,7 +222,12 @@ spdk_shutdown_iscsi_conns(void)
void
spdk_iscsi_task_cpl(struct spdk_scsi_task *scsi_task)
{
struct spdk_iscsi_task *iscsi_task;
if (scsi_task != NULL) {
iscsi_task = spdk_iscsi_task_from_scsi_task(scsi_task);
free(iscsi_task);
}
}
void

View File

@ -988,6 +988,88 @@ clear_all_transfer_tasks_test(void)
spdk_put_pdu(pdu1);
}
static void
abort_queued_datain_task_test(void)
{
struct spdk_iscsi_conn conn;
struct spdk_iscsi_task *task, *task2, *task3;
int rc;
TAILQ_INIT(&conn.queued_datain_tasks);
task = spdk_iscsi_task_get(&conn, NULL, NULL);
SPDK_CU_ASSERT_FATAL(task != NULL);
TAILQ_INSERT_TAIL(&conn.queued_datain_tasks, task, link);
/* Slot of data in tasks are full */
conn.data_in_cnt = MAX_LARGE_DATAIN_PER_CONNECTION;
rc = _spdk_iscsi_conn_abort_queued_datain_task(&conn, task);
CU_ASSERT(rc != 0);
/* Only one slot remains and no subtasks are submitted yet. */
conn.data_in_cnt--;
task->current_datain_offset = 0;
rc = _spdk_iscsi_conn_abort_queued_datain_task(&conn, task);
CU_ASSERT(rc == 0);
CU_ASSERT(TAILQ_EMPTY(&conn.queued_datain_tasks));
task = spdk_iscsi_task_get(&conn, NULL, NULL);
SPDK_CU_ASSERT_FATAL(task != NULL);
TAILQ_INSERT_TAIL(&conn.queued_datain_tasks, task, link);
/* Only one slot remains and a subtask is submitted. */
task->scsi.transfer_len = SPDK_BDEV_LARGE_BUF_MAX_SIZE * 3;
task->current_datain_offset = SPDK_BDEV_LARGE_BUF_MAX_SIZE;
rc = _spdk_iscsi_conn_abort_queued_datain_task(&conn, task);
CU_ASSERT(rc != 0);
CU_ASSERT(task->current_datain_offset == SPDK_BDEV_LARGE_BUF_MAX_SIZE * 2);
CU_ASSERT(conn.data_in_cnt == MAX_LARGE_DATAIN_PER_CONNECTION);
/* Additional one slot becomes vacant. */
conn.data_in_cnt--;
rc = _spdk_iscsi_conn_abort_queued_datain_task(&conn, task);
CU_ASSERT(rc == 0);
CU_ASSERT(TAILQ_EMPTY(&conn.queued_datain_tasks));
spdk_iscsi_task_cpl(&task->scsi);
/* Queue three data in tasks and abort each task sequentially */
task = spdk_iscsi_task_get(&conn, NULL, NULL);
SPDK_CU_ASSERT_FATAL(task != NULL);
task->tag = 1;
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->current_datain_offset = 0;
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->current_datain_offset = 0;
TAILQ_INSERT_TAIL(&conn.queued_datain_tasks, task3, link);
conn.data_in_cnt--;
rc = spdk_iscsi_conn_abort_queued_datain_task(&conn, 1);
CU_ASSERT(rc == 0);
rc = spdk_iscsi_conn_abort_queued_datain_task(&conn, 2);
CU_ASSERT(rc == 0);
rc = spdk_iscsi_conn_abort_queued_datain_task(&conn, 3);
CU_ASSERT(rc == 0);
CU_ASSERT(TAILQ_EMPTY(&conn.queued_datain_tasks));
}
int
main(int argc, char **argv)
{
@ -1020,6 +1102,8 @@ main(int argc, char **argv)
|| CU_add_test(suite, "del transfer task test", del_transfer_task_test) == NULL
|| CU_add_test(suite, "clear all transfer tasks test",
clear_all_transfer_tasks_test) == NULL
|| CU_add_test(suite, "abort_queued_datain_task_test",
abort_queued_datain_task_test) == NULL
) {
CU_cleanup_registry();
return CU_get_error();