From f8dcdfa99e44aeeb824bf7bd3588efa40c94e4ba Mon Sep 17 00:00:00 2001 From: Ziye Yang Date: Wed, 15 Nov 2017 12:37:24 +0800 Subject: [PATCH] scsi: Make the pending tasks handling logic correct for reset In principle, we should not have active tasks, which means that lun->tasks should be empty. But for the exceptional case, we may still need to handle those active tasks, to make the scsi related application continue running instead of quit. We should not directly call spdk_scsi_lun_complete_task since those tasks are already sent to bdev layer. And finally, those tasks will be return (even aborted) by call backs. So we only need to set task status to condition, but not call spdk_scsi_lun_complete_task directly. Change-Id: I6af2bda0f0b1de7b0c655d2ac2980ddca48c1162 Signed-off-by: Ziye Yang Signed-off-by: Shuhei Matsumoto Reviewed-on: https://review.gerrithub.io/386817 Tested-by: SPDK Automated Test System Reviewed-by: Jim Harris --- lib/scsi/lun.c | 72 ++++++++++--------- lib/scsi/scsi_internal.h | 1 - test/unit/lib/scsi/scsi_bdev.c/scsi_bdev_ut.c | 5 -- 3 files changed, 37 insertions(+), 41 deletions(-) diff --git a/lib/scsi/lun.c b/lib/scsi/lun.c index e2fb908e2..3b29a79cf 100644 --- a/lib/scsi/lun.c +++ b/lib/scsi/lun.c @@ -49,50 +49,52 @@ 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) { - spdk_scsi_lun_clear_all(task->lun); + if (spdk_scsi_lun_clear_all(task->lun)) { + task->response = SPDK_SCSI_TASK_MGMT_RESP_TARGET_FAILURE; + } } task->cpl_fn(task); } -void -spdk_scsi_lun_clear_all(struct spdk_scsi_lun *lun) -{ - struct spdk_scsi_task *task, *task_tmp; - - /* - * This function is called from one location, after the backend LUN - * device was reset. Can assume are no active tasks in the - * backend that need to be terminated. Just need to queue all tasks - * back to frontend for any final processing and cleanup. - * - * Queue the tasks back roughly in the order they were received - * ('cleanup' = oldest, 'tasks' = current, and 'pending' = newest) - */ - - TAILQ_FOREACH_SAFE(task, &lun->tasks, scsi_link, task_tmp) { - 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_scsi_lun_complete_task(lun, task); - } - - TAILQ_FOREACH_SAFE(task, &lun->pending_tasks, scsi_link, task_tmp) { - TAILQ_REMOVE(&lun->pending_tasks, task, scsi_link); - TAILQ_INSERT_TAIL(&lun->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_scsi_lun_complete_task(lun, task); - } -} - int spdk_scsi_lun_task_mgmt_execute(struct spdk_scsi_task *task, enum spdk_scsi_task_func func) diff --git a/lib/scsi/scsi_internal.h b/lib/scsi/scsi_internal.h index b1fdcdb91..3e33f2833 100644 --- a/lib/scsi/scsi_internal.h +++ b/lib/scsi/scsi_internal.h @@ -134,7 +134,6 @@ _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); -void spdk_scsi_lun_clear_all(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); int spdk_scsi_lun_task_mgmt_execute(struct spdk_scsi_task *task, enum spdk_scsi_task_func func); diff --git a/test/unit/lib/scsi/scsi_bdev.c/scsi_bdev_ut.c b/test/unit/lib/scsi/scsi_bdev.c/scsi_bdev_ut.c index 5e61900e4..6e1ef2dcb 100644 --- a/test/unit/lib/scsi/scsi_bdev.c/scsi_bdev_ut.c +++ b/test/unit/lib/scsi/scsi_bdev.c/scsi_bdev_ut.c @@ -114,11 +114,6 @@ spdk_bdev_has_write_cache(const struct spdk_bdev *bdev) return false; } -void -spdk_scsi_lun_clear_all(struct spdk_scsi_lun *lun) -{ -} - void spdk_scsi_lun_complete_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task) {