From 3ef8dc107407b2c9b5a98a6055045642049e9686 Mon Sep 17 00:00:00 2001 From: Dariusz Stojaczyk Date: Mon, 8 Jan 2018 12:20:57 +0100 Subject: [PATCH] scsi/lun: simplify task submission path Removed task `pending` queue. All tasks were temporarily put in this queue just to be moved out of it yet within the same reactor cycle. Change-Id: I32d402f7abd3cfa21c263f41149425abdc71992f Signed-off-by: Dariusz Stojaczyk Reviewed-on: https://review.gerrithub.io/393911 Tested-by: SPDK Automated Test System Reviewed-by: Reviewed-by: Ben Walker Reviewed-by: Jim Harris --- lib/scsi/dev.c | 5 +- lib/scsi/lun.c | 98 +++++++++---------------------- lib/scsi/scsi_internal.h | 6 +- test/unit/lib/scsi/dev.c/dev_ut.c | 8 +-- test/unit/lib/scsi/lun.c/lun_ut.c | 36 ++---------- 5 files changed, 39 insertions(+), 114 deletions(-) diff --git a/lib/scsi/dev.c b/lib/scsi/dev.c index f95bf0aa3..12c4daa58 100644 --- a/lib/scsi/dev.c +++ b/lib/scsi/dev.c @@ -203,10 +203,7 @@ spdk_scsi_dev_queue_task(struct spdk_scsi_dev *dev, { assert(task != NULL); - if (spdk_scsi_lun_append_task(task->lun, task) == 0) { - /* ready to execute, disk is valid for LUN access */ - spdk_scsi_lun_execute_tasks(task->lun); - } + spdk_scsi_lun_execute_task(task->lun, task); } static struct spdk_scsi_port * diff --git a/lib/scsi/lun.c b/lib/scsi/lun.c index 5e26b81c6..9c1a8e787 100644 --- a/lib/scsi/lun.c +++ b/lib/scsi/lun.c @@ -49,46 +49,18 @@ spdk_scsi_lun_complete_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *ta task->cpl_fn(task); } -static int -spdk_scsi_lun_clear_all(struct spdk_scsi_lun *lun) -{ - struct spdk_scsi_task *task, *task_tmp; - int rc = 0; - - /* - * This function is called from one location, after the backend LUN - * device was reset. If there are active tasks in the backend, it - * means that LUN reset fails, and we return and set failure status - * to LUN reset task. If LUN reset succeeds, we need to abort any - * pending tasks.. - * - * ( 'tasks' = active, and 'pending' = newest) - */ - - if (!TAILQ_EMPTY(&lun->tasks)) { - SPDK_ERRLOG("lun->tasks should be empty after reset\n"); - rc = -1; - } - - TAILQ_FOREACH_SAFE(task, &lun->pending_tasks, scsi_link, task_tmp) { - TAILQ_REMOVE(&lun->pending_tasks, task, scsi_link); - spdk_scsi_task_set_status(task, SPDK_SCSI_STATUS_CHECK_CONDITION, - SPDK_SCSI_SENSE_ABORTED_COMMAND, - SPDK_SCSI_ASC_NO_ADDITIONAL_SENSE, - SPDK_SCSI_ASCQ_CAUSE_NOT_REPORTABLE); - spdk_trace_record(TRACE_SCSI_TASK_DONE, lun->dev->id, 0, (uintptr_t)task, 0); - task->cpl_fn(task); - } - - return rc; -} - void spdk_scsi_lun_complete_mgmt_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task) { if (task->function == SPDK_SCSI_TASK_FUNC_LUN_RESET && task->status == SPDK_SCSI_STATUS_GOOD) { - if (spdk_scsi_lun_clear_all(task->lun)) { + /* + * The backend LUN device was just reset. If there are active tasks + * in the backend, it means that LUN reset fails, and we set failure + * status to LUN reset task. + */ + if (spdk_scsi_lun_has_pending_tasks(lun)) { + SPDK_ERRLOG("lun->tasks should be empty after reset\n"); task->response = SPDK_SCSI_TASK_MGMT_RESP_TARGET_FAILURE; } } @@ -178,45 +150,34 @@ spdk_scsi_task_process_null_lun(struct spdk_scsi_task *task) } } -int -spdk_scsi_lun_append_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task) -{ - TAILQ_INSERT_TAIL(&lun->pending_tasks, task, scsi_link); - return 0; -} - void -spdk_scsi_lun_execute_tasks(struct spdk_scsi_lun *lun) +spdk_scsi_lun_execute_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task) { - struct spdk_scsi_task *task, *task_tmp; int rc; - TAILQ_FOREACH_SAFE(task, &lun->pending_tasks, scsi_link, task_tmp) { - task->status = SPDK_SCSI_STATUS_GOOD; - spdk_trace_record(TRACE_SCSI_TASK_START, lun->dev->id, task->length, (uintptr_t)task, 0); - TAILQ_REMOVE(&lun->pending_tasks, task, scsi_link); - TAILQ_INSERT_TAIL(&lun->tasks, task, scsi_link); - if (!lun->removed) { - rc = spdk_bdev_scsi_execute(task); - } else { - spdk_scsi_task_set_status(task, SPDK_SCSI_STATUS_CHECK_CONDITION, - SPDK_SCSI_SENSE_ABORTED_COMMAND, - SPDK_SCSI_ASC_NO_ADDITIONAL_SENSE, - SPDK_SCSI_ASCQ_CAUSE_NOT_REPORTABLE); - rc = SPDK_SCSI_TASK_COMPLETE; - } + task->status = SPDK_SCSI_STATUS_GOOD; + spdk_trace_record(TRACE_SCSI_TASK_START, lun->dev->id, task->length, (uintptr_t)task, 0); + TAILQ_INSERT_TAIL(&lun->tasks, task, scsi_link); + if (!lun->removed) { + rc = spdk_bdev_scsi_execute(task); + } else { + spdk_scsi_task_set_status(task, SPDK_SCSI_STATUS_CHECK_CONDITION, + SPDK_SCSI_SENSE_ABORTED_COMMAND, + SPDK_SCSI_ASC_NO_ADDITIONAL_SENSE, + SPDK_SCSI_ASCQ_CAUSE_NOT_REPORTABLE); + rc = SPDK_SCSI_TASK_COMPLETE; + } - switch (rc) { - case SPDK_SCSI_TASK_PENDING: - break; + switch (rc) { + case SPDK_SCSI_TASK_PENDING: + break; - case SPDK_SCSI_TASK_COMPLETE: - spdk_scsi_lun_complete_task(lun, task); - break; + case SPDK_SCSI_TASK_COMPLETE: + spdk_scsi_lun_complete_task(lun, task); + break; - default: - abort(); - } + default: + abort(); } } @@ -299,7 +260,6 @@ spdk_scsi_lun_construct(const char *name, struct spdk_bdev *bdev, } TAILQ_INIT(&lun->tasks); - TAILQ_INIT(&lun->pending_tasks); lun->bdev = bdev; lun->io_channel = NULL; @@ -393,5 +353,5 @@ spdk_scsi_lun_get_dev(const struct spdk_scsi_lun *lun) bool spdk_scsi_lun_has_pending_tasks(const struct spdk_scsi_lun *lun) { - return !TAILQ_EMPTY(&lun->pending_tasks) || !TAILQ_EMPTY(&lun->tasks); + return !TAILQ_EMPTY(&lun->tasks); } diff --git a/lib/scsi/scsi_internal.h b/lib/scsi/scsi_internal.h index 297aaf2bc..62e96d8bd 100644 --- a/lib/scsi/scsi_internal.h +++ b/lib/scsi/scsi_internal.h @@ -110,8 +110,7 @@ struct spdk_scsi_lun { /** Argument for hotremove_cb */ void *hotremove_ctx; - TAILQ_HEAD(tasks, spdk_scsi_task) tasks; /* submitted tasks */ - TAILQ_HEAD(pending_tasks, spdk_scsi_task) pending_tasks; /* pending tasks */ + TAILQ_HEAD(tasks, spdk_scsi_task) tasks; /* pending tasks */ }; struct spdk_lun_db_entry { @@ -131,8 +130,7 @@ _spdk_scsi_lun *spdk_scsi_lun_construct(const char *name, struct spdk_bdev *bdev void *hotremove_ctx); int spdk_scsi_lun_destruct(struct spdk_scsi_lun *lun); -int spdk_scsi_lun_append_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task); -void spdk_scsi_lun_execute_tasks(struct spdk_scsi_lun *lun); +void spdk_scsi_lun_execute_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task); int spdk_scsi_lun_task_mgmt_execute(struct spdk_scsi_task *task, enum spdk_scsi_task_func func); void spdk_scsi_lun_complete_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task); void spdk_scsi_lun_complete_mgmt_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task); diff --git a/test/unit/lib/scsi/dev.c/dev_ut.c b/test/unit/lib/scsi/dev.c/dev_ut.c index 4446a5964..0ca6d12f9 100644 --- a/test/unit/lib/scsi/dev.c/dev_ut.c +++ b/test/unit/lib/scsi/dev.c/dev_ut.c @@ -157,14 +157,8 @@ spdk_scsi_lun_task_mgmt_execute(struct spdk_scsi_task *task, enum spdk_scsi_task return 0; } -int -spdk_scsi_lun_append_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task) -{ - return 0; -} - void -spdk_scsi_lun_execute_tasks(struct spdk_scsi_lun *lun) +spdk_scsi_lun_execute_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task) { } diff --git a/test/unit/lib/scsi/lun.c/lun_ut.c b/test/unit/lib/scsi/lun.c/lun_ut.c index b7a71626d..96cf2de7e 100644 --- a/test/unit/lib/scsi/lun.c/lun_ut.c +++ b/test/unit/lib/scsi/lun.c/lun_ut.c @@ -209,10 +209,6 @@ lun_construct(void) lun = spdk_scsi_lun_construct("lun0", &bdev, NULL, NULL); SPDK_CU_ASSERT_FATAL(lun != NULL); - if (lun != NULL) { - SPDK_CU_ASSERT_FATAL(TAILQ_EMPTY(&lun->pending_tasks)); - } - return lun; } @@ -275,12 +271,7 @@ lun_task_mgmt_execute_abort_task_not_supported(void) task.lun = lun; task.cdb = cdb; - spdk_scsi_lun_append_task(lun, &task); - - /* task should now be on the pending_task list */ - CU_ASSERT(!TAILQ_EMPTY(&lun->pending_tasks)); - - spdk_scsi_lun_execute_tasks(lun); + spdk_scsi_lun_execute_task(lun, &task); /* task should now be on the tasks list */ CU_ASSERT(!TAILQ_EMPTY(&lun->tasks)); @@ -341,12 +332,7 @@ lun_task_mgmt_execute_abort_task_all_not_supported(void) task.lun = lun; task.cdb = cdb; - spdk_scsi_lun_append_task(lun, &task); - - /* task should now be on the pending_task list */ - CU_ASSERT(!TAILQ_EMPTY(&lun->pending_tasks)); - - spdk_scsi_lun_execute_tasks(lun); + spdk_scsi_lun_execute_task(lun, &task); /* task should now be on the tasks list */ CU_ASSERT(!TAILQ_EMPTY(&lun->tasks)); @@ -515,17 +501,12 @@ lun_execute_scsi_task_pending(void) g_lun_execute_fail = false; g_lun_execute_status = SPDK_SCSI_TASK_PENDING; - spdk_scsi_lun_append_task(lun, &task); - - /* task should now be on the pending_task list */ - CU_ASSERT(!TAILQ_EMPTY(&lun->pending_tasks)); - - /* but the tasks list should still be empty since it has not been + /* the tasks list should still be empty since it has not been executed yet */ CU_ASSERT(TAILQ_EMPTY(&lun->tasks)); - spdk_scsi_lun_execute_tasks(lun); + spdk_scsi_lun_execute_task(lun, &task); /* Assert the task has been successfully added to the tasks queue */ CU_ASSERT(!TAILQ_EMPTY(&lun->tasks)); @@ -553,17 +534,12 @@ lun_execute_scsi_task_complete(void) g_lun_execute_fail = false; g_lun_execute_status = SPDK_SCSI_TASK_COMPLETE; - spdk_scsi_lun_append_task(lun, &task); - - /* task should now be on the pending_task list */ - CU_ASSERT(!TAILQ_EMPTY(&lun->pending_tasks)); - - /* but the tasks list should still be empty since it has not been + /* the tasks list should still be empty since it has not been executed yet */ CU_ASSERT(TAILQ_EMPTY(&lun->tasks)); - spdk_scsi_lun_execute_tasks(lun); + spdk_scsi_lun_execute_task(lun, &task); /* Assert the task has not been added to the tasks queue */ CU_ASSERT(TAILQ_EMPTY(&lun->tasks));