ivampiresp/Spdk
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

lib/scsi: Merge append and execute SCSI task into a single function

Browse Source 
Both for SCSI IO task and management task, append and execute
operations bad bbeen separated into different functions.

Append operation was for LUN reset, and separating into two different
functions was for clarification and readability.

LUN reset is sufficiently stable now.

Merging append and execute SCSI task into a single function is good
as API and enables us to do optimization.

For SCSI management task, merge spdk_scsi_lun_append_mgmt_task into
spdk_scsi_lun_execute_mgmt_task() simply.

For SCSI IO task, merge spdk_scsi_lun_append_task into
spdk_scsi_lun_execute_task() and do a small optimization.
The refined spdk_scsi_lun_execute_task() adds the IO task to the
pending list if there is any pending management task, executes all
existing penging IO tasks first and then the IO task if there is any
pending IO task, or executes the IO task directly otherwise.

Update unit test accordingly.

Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I26ffc4f4f62747d8cdecb90690f26cd58a9c17f7
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1817
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom CI
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
This commit is contained in:
Shuhei Matsumoto 2020-04-14 02:17:32 +09:00 committed by Tomasz Zawadzki
parent 2aa74d8ba9
commit 799e0a84af
6 changed files with 65 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
lib/scsi/Makefile
View File

@ -34,7 +34,7 @@
SPDK_ROOT_DIR := $(abspath $(CURDIR)/../..) SPDK_ROOT_DIR := $(abspath $(CURDIR)/../..)
include $(SPDK_ROOT_DIR)/mk/spdk.common.mk include $(SPDK_ROOT_DIR)/mk/spdk.common.mk
SO_VER := 2 SO_VER := 3
SO_MINOR := 0 SO_MINOR := 0
SO_SUFFIX := $(SO_VER).$(SO_MINOR) SO_SUFFIX := $(SO_VER).$(SO_MINOR)

6
lib/scsi/dev.c
View File

@ -262,8 +262,7 @@ spdk_scsi_dev_queue_mgmt_task(struct spdk_scsi_dev *dev,
{ {
assert(task != NULL); assert(task != NULL);
spdk_scsi_lun_append_mgmt_task(task->lun, task); spdk_scsi_lun_execute_mgmt_task(task->lun, task);
spdk_scsi_lun_execute_mgmt_task(task->lun);
} }
void void
@ -272,8 +271,7 @@ spdk_scsi_dev_queue_task(struct spdk_scsi_dev *dev,
{ {
assert(task != NULL); assert(task != NULL);
spdk_scsi_lun_append_task(task->lun, task); spdk_scsi_lun_execute_task(task->lun, task);
spdk_scsi_lun_execute_tasks(task->lun);
} }
static struct spdk_scsi_port * static struct spdk_scsi_port *

50
lib/scsi/lun.c
View File

@ -40,6 +40,9 @@
#include "spdk/util.h" #include "spdk/util.h"
#include "spdk/likely.h" #include "spdk/likely.h"
static void scsi_lun_execute_tasks(struct spdk_scsi_lun *lun);
static void scsi_lun_execute_mgmt_task(struct spdk_scsi_lun *lun);
void void
spdk_scsi_lun_complete_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task) spdk_scsi_lun_complete_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task)
{ {
@ -58,7 +61,7 @@ scsi_lun_complete_mgmt_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *ta
task->cpl_fn(task); task->cpl_fn(task);
/* Try to execute the first pending mgmt task if it exists. */ /* Try to execute the first pending mgmt task if it exists. */
spdk_scsi_lun_execute_mgmt_task(lun); scsi_lun_execute_mgmt_task(lun);
} }
static bool static bool
@ -116,15 +119,15 @@ spdk_scsi_lun_complete_reset_task(struct spdk_scsi_lun *lun, struct spdk_scsi_ta
scsi_lun_complete_mgmt_task(lun, task); scsi_lun_complete_mgmt_task(lun, task);
} }
void static void
spdk_scsi_lun_append_mgmt_task(struct spdk_scsi_lun *lun, scsi_lun_append_mgmt_task(struct spdk_scsi_lun *lun,
struct spdk_scsi_task *task) struct spdk_scsi_task *task)
{ {
TAILQ_INSERT_TAIL(&lun->pending_mgmt_tasks, task, scsi_link); TAILQ_INSERT_TAIL(&lun->pending_mgmt_tasks, task, scsi_link);
} }
void static void
spdk_scsi_lun_execute_mgmt_task(struct spdk_scsi_lun *lun) scsi_lun_execute_mgmt_task(struct spdk_scsi_lun *lun)
{ {
struct spdk_scsi_task *task; struct spdk_scsi_task *task;
@ -135,7 +138,7 @@ spdk_scsi_lun_execute_mgmt_task(struct spdk_scsi_lun *lun)
task = TAILQ_FIRST(&lun->pending_mgmt_tasks); task = TAILQ_FIRST(&lun->pending_mgmt_tasks);
if (spdk_likely(task == NULL)) { if (spdk_likely(task == NULL)) {
/* Try to execute all pending tasks */ /* Try to execute all pending tasks */
spdk_scsi_lun_execute_tasks(lun); scsi_lun_execute_tasks(lun);
return; return;
} }
TAILQ_REMOVE(&lun->pending_mgmt_tasks, task, scsi_link); TAILQ_REMOVE(&lun->pending_mgmt_tasks, task, scsi_link);
@ -177,6 +180,14 @@ spdk_scsi_lun_execute_mgmt_task(struct spdk_scsi_lun *lun)
scsi_lun_complete_mgmt_task(lun, task); scsi_lun_complete_mgmt_task(lun, task);
} }
void
spdk_scsi_lun_execute_mgmt_task(struct spdk_scsi_lun *lun,
struct spdk_scsi_task *task)
{
scsi_lun_append_mgmt_task(lun, task);
scsi_lun_execute_mgmt_task(lun);
}
static void static void
_scsi_lun_execute_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task) _scsi_lun_execute_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task)
{ {
@ -212,8 +223,8 @@ _scsi_lun_execute_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task)
} }
} }
void static void
spdk_scsi_lun_append_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task) scsi_lun_append_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task)
{ {
TAILQ_INSERT_TAIL(&lun->pending_tasks, task, scsi_link); TAILQ_INSERT_TAIL(&lun->pending_tasks, task, scsi_link);
} }
@ -230,15 +241,24 @@ scsi_lun_execute_tasks(struct spdk_scsi_lun *lun)
} }
void 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)
{ {
if (scsi_lun_has_pending_mgmt_tasks(lun) || if (spdk_unlikely(scsi_lun_has_pending_mgmt_tasks(lun))) {
scsi_lun_has_outstanding_mgmt_tasks(lun)) { /* Add the IO task to pending list and wait for completion of
/* Pending IO tasks will wait for completion of existing mgmt tasks. * existing mgmt tasks.
*/ */
return; scsi_lun_append_task(lun, task);
} else if (spdk_unlikely(scsi_lun_has_pending_tasks(lun))) {
/* If there is any pending IO task, append the IO task to the
* tail of the pending list, and then execute all pending IO tasks
* from the head to submit IO tasks in order.
*/
scsi_lun_append_task(lun, task);
scsi_lun_execute_tasks(lun);
} else {
/* Execute the IO task directly. */
_scsi_lun_execute_task(lun, task);
} }
scsi_lun_execute_tasks(lun);
} }
static void static void

6
lib/scsi/scsi_internal.h
View File

@ -176,10 +176,8 @@ struct spdk_scsi_lun *spdk_scsi_lun_construct(struct spdk_bdev *bdev,
void *hotremove_ctx); void *hotremove_ctx);
void spdk_scsi_lun_destruct(struct spdk_scsi_lun *lun); void spdk_scsi_lun_destruct(struct spdk_scsi_lun *lun);
void spdk_scsi_lun_append_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task); void spdk_scsi_lun_execute_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_mgmt_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task);
void spdk_scsi_lun_append_mgmt_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task);
void spdk_scsi_lun_execute_mgmt_task(struct spdk_scsi_lun *lun);
bool spdk_scsi_lun_has_pending_mgmt_tasks(const struct spdk_scsi_lun *lun, bool spdk_scsi_lun_has_pending_mgmt_tasks(const struct spdk_scsi_lun *lun,
const struct spdk_scsi_port *initiator_port); const struct spdk_scsi_port *initiator_port);
void spdk_scsi_lun_complete_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task); void spdk_scsi_lun_complete_task(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task);

8
test/unit/lib/scsi/dev.c/dev_ut.c
View File

@ -115,16 +115,12 @@ spdk_bdev_get_by_name(const char *bdev_name)
return NULL; return NULL;
} }
DEFINE_STUB_V(spdk_scsi_lun_append_mgmt_task, DEFINE_STUB_V(spdk_scsi_lun_execute_mgmt_task,
(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task)); (struct spdk_scsi_lun *lun, struct spdk_scsi_task *task));
DEFINE_STUB_V(spdk_scsi_lun_execute_mgmt_task, (struct spdk_scsi_lun *lun)); DEFINE_STUB_V(spdk_scsi_lun_execute_task,
DEFINE_STUB_V(spdk_scsi_lun_append_task,
(struct spdk_scsi_lun *lun, struct spdk_scsi_task *task)); (struct spdk_scsi_lun *lun, struct spdk_scsi_task *task));
DEFINE_STUB_V(spdk_scsi_lun_execute_tasks, (struct spdk_scsi_lun *lun));
DEFINE_STUB(_spdk_scsi_lun_allocate_io_channel, int, DEFINE_STUB(_spdk_scsi_lun_allocate_io_channel, int,
(struct spdk_scsi_lun *lun), 0); (struct spdk_scsi_lun *lun), 0);

69
test/unit/lib/scsi/lun.c/lun_ut.c
View File

@ -181,14 +181,12 @@ lun_task_mgmt_execute_abort_task_not_supported(void)
task.lun = lun; task.lun = lun;
task.cdb = cdb; task.cdb = cdb;
spdk_scsi_lun_append_task(lun, &task); spdk_scsi_lun_execute_task(lun, &task);
spdk_scsi_lun_execute_tasks(lun);
/* task should now be on the tasks list */ /* task should now be on the tasks list */
CU_ASSERT(!TAILQ_EMPTY(&lun->tasks)); CU_ASSERT(!TAILQ_EMPTY(&lun->tasks));
spdk_scsi_lun_append_mgmt_task(lun, &mgmt_task); spdk_scsi_lun_execute_mgmt_task(lun, &mgmt_task);
spdk_scsi_lun_execute_mgmt_task(lun);
/* task abort is not supported */ /* task abort is not supported */
CU_ASSERT(mgmt_task.response == SPDK_SCSI_TASK_MGMT_RESP_REJECT_FUNC_NOT_SUPPORTED); CU_ASSERT(mgmt_task.response == SPDK_SCSI_TASK_MGMT_RESP_REJECT_FUNC_NOT_SUPPORTED);
@ -226,14 +224,12 @@ lun_task_mgmt_execute_abort_task_all_not_supported(void)
task.lun = lun; task.lun = lun;
task.cdb = cdb; task.cdb = cdb;
spdk_scsi_lun_append_task(lun, &task); spdk_scsi_lun_execute_task(lun, &task);
spdk_scsi_lun_execute_tasks(lun);
/* task should now be on the tasks list */ /* task should now be on the tasks list */
CU_ASSERT(!TAILQ_EMPTY(&lun->tasks)); CU_ASSERT(!TAILQ_EMPTY(&lun->tasks));
spdk_scsi_lun_append_mgmt_task(lun, &mgmt_task); spdk_scsi_lun_execute_mgmt_task(lun, &mgmt_task);
spdk_scsi_lun_execute_mgmt_task(lun);
/* task abort is not supported */ /* task abort is not supported */
CU_ASSERT(mgmt_task.response == SPDK_SCSI_TASK_MGMT_RESP_REJECT_FUNC_NOT_SUPPORTED); CU_ASSERT(mgmt_task.response == SPDK_SCSI_TASK_MGMT_RESP_REJECT_FUNC_NOT_SUPPORTED);
@ -262,8 +258,7 @@ lun_task_mgmt_execute_lun_reset(void)
mgmt_task.lun = lun; mgmt_task.lun = lun;
mgmt_task.function = SPDK_SCSI_TASK_FUNC_LUN_RESET; mgmt_task.function = SPDK_SCSI_TASK_FUNC_LUN_RESET;
spdk_scsi_lun_append_mgmt_task(lun, &mgmt_task); spdk_scsi_lun_execute_mgmt_task(lun, &mgmt_task);
spdk_scsi_lun_execute_mgmt_task(lun);
/* Returns success */ /* Returns success */
CU_ASSERT_EQUAL(mgmt_task.status, SPDK_SCSI_STATUS_GOOD); CU_ASSERT_EQUAL(mgmt_task.status, SPDK_SCSI_STATUS_GOOD);
@ -288,8 +283,7 @@ lun_task_mgmt_execute_invalid_case(void)
mgmt_task.function = 5; mgmt_task.function = 5;
/* Pass an invalid value to the switch statement */ /* Pass an invalid value to the switch statement */
spdk_scsi_lun_append_mgmt_task(lun, &mgmt_task); spdk_scsi_lun_execute_mgmt_task(lun, &mgmt_task);
spdk_scsi_lun_execute_mgmt_task(lun);
/* function code is invalid */ /* function code is invalid */
CU_ASSERT_EQUAL(mgmt_task.response, SPDK_SCSI_TASK_MGMT_RESP_REJECT_FUNC_NOT_SUPPORTED); CU_ASSERT_EQUAL(mgmt_task.response, SPDK_SCSI_TASK_MGMT_RESP_REJECT_FUNC_NOT_SUPPORTED);
@ -391,8 +385,7 @@ lun_execute_scsi_task_pending(void)
*/ */
CU_ASSERT(TAILQ_EMPTY(&lun->tasks)); CU_ASSERT(TAILQ_EMPTY(&lun->tasks));
spdk_scsi_lun_append_task(lun, &task); spdk_scsi_lun_execute_task(lun, &task);
spdk_scsi_lun_execute_tasks(lun);
/* Assert the task has been successfully added to the tasks queue */ /* Assert the task has been successfully added to the tasks queue */
CU_ASSERT(!TAILQ_EMPTY(&lun->tasks)); CU_ASSERT(!TAILQ_EMPTY(&lun->tasks));
@ -429,8 +422,7 @@ lun_execute_scsi_task_complete(void)
*/ */
CU_ASSERT(TAILQ_EMPTY(&lun->tasks)); CU_ASSERT(TAILQ_EMPTY(&lun->tasks));
spdk_scsi_lun_append_task(lun, &task); spdk_scsi_lun_execute_task(lun, &task);
spdk_scsi_lun_execute_tasks(lun);
/* Assert the task has not been added to the tasks queue */ /* Assert the task has not been added to the tasks queue */
CU_ASSERT(TAILQ_EMPTY(&lun->tasks)); CU_ASSERT(TAILQ_EMPTY(&lun->tasks));
@ -495,26 +487,16 @@ lun_reset_task_wait_scsi_task_complete(void)
mgmt_task.lun = lun; mgmt_task.lun = lun;
mgmt_task.function = SPDK_SCSI_TASK_FUNC_LUN_RESET; mgmt_task.function = SPDK_SCSI_TASK_FUNC_LUN_RESET;
/* Append a task to the pending task list. */
spdk_scsi_lun_append_task(lun, &task);
CU_ASSERT(!TAILQ_EMPTY(&lun->pending_tasks));
/* Execute the task but it is still in the task list. */ /* Execute the task but it is still in the task list. */
spdk_scsi_lun_execute_tasks(lun); spdk_scsi_lun_execute_task(lun, &task);
CU_ASSERT(TAILQ_EMPTY(&lun->pending_tasks)); CU_ASSERT(TAILQ_EMPTY(&lun->pending_tasks));
CU_ASSERT(!TAILQ_EMPTY(&lun->tasks)); CU_ASSERT(!TAILQ_EMPTY(&lun->tasks));
/* Append a reset task to the pending mgmt task list. */
spdk_scsi_lun_append_mgmt_task(lun, &mgmt_task);
CU_ASSERT(!TAILQ_EMPTY(&lun->pending_mgmt_tasks));
/* Execute the reset task */ /* Execute the reset task */
spdk_scsi_lun_execute_mgmt_task(lun); spdk_scsi_lun_execute_mgmt_task(lun, &mgmt_task);
/* The reset task should be still on the submitted mgmt task list and /* The reset task should be on the submitted mgmt task list and
* a poller is created because the task prior to the reset task is pending. * a poller is created because the task prior to the reset task is pending.
*/ */
CU_ASSERT(!TAILQ_EMPTY(&lun->mgmt_tasks)); CU_ASSERT(!TAILQ_EMPTY(&lun->mgmt_tasks));
@ -566,22 +548,17 @@ lun_reset_task_suspend_scsi_task(void)
mgmt_task.function = SPDK_SCSI_TASK_FUNC_LUN_RESET; mgmt_task.function = SPDK_SCSI_TASK_FUNC_LUN_RESET;
/* Append a reset task to the pending mgmt task list. */ /* Append a reset task to the pending mgmt task list. */
spdk_scsi_lun_append_mgmt_task(lun, &mgmt_task); scsi_lun_append_mgmt_task(lun, &mgmt_task);
CU_ASSERT(!TAILQ_EMPTY(&lun->pending_mgmt_tasks)); CU_ASSERT(!TAILQ_EMPTY(&lun->pending_mgmt_tasks));
/* Append a task to the pending task list. */ /* Execute the task but it is on the pending task list. */
spdk_scsi_lun_append_task(lun, &task); spdk_scsi_lun_execute_task(lun, &task);
CU_ASSERT(!TAILQ_EMPTY(&lun->pending_tasks));
/* Execute the task but it is still on the pending task list. */
spdk_scsi_lun_execute_tasks(lun);
CU_ASSERT(!TAILQ_EMPTY(&lun->pending_tasks)); CU_ASSERT(!TAILQ_EMPTY(&lun->pending_tasks));
/* Execute the reset task. The task will be executed then. */ /* Execute the reset task. The task will be executed then. */
spdk_scsi_lun_execute_mgmt_task(lun); scsi_lun_execute_mgmt_task(lun);
CU_ASSERT(TAILQ_EMPTY(&lun->mgmt_tasks)); CU_ASSERT(TAILQ_EMPTY(&lun->mgmt_tasks));
CU_ASSERT(lun->reset_poller == NULL); CU_ASSERT(lun->reset_poller == NULL);
@ -688,13 +665,13 @@ abort_pending_mgmt_tasks_when_lun_is_removed(void)
CU_ASSERT(g_task_count == 3); CU_ASSERT(g_task_count == 3);
spdk_scsi_lun_append_mgmt_task(lun, &task1); scsi_lun_append_mgmt_task(lun, &task1);
spdk_scsi_lun_append_mgmt_task(lun, &task2); scsi_lun_append_mgmt_task(lun, &task2);
spdk_scsi_lun_append_mgmt_task(lun, &task3); scsi_lun_append_mgmt_task(lun, &task3);
CU_ASSERT(!TAILQ_EMPTY(&lun->pending_mgmt_tasks)); CU_ASSERT(!TAILQ_EMPTY(&lun->pending_mgmt_tasks));
spdk_scsi_lun_execute_mgmt_task(lun); scsi_lun_execute_mgmt_task(lun);
CU_ASSERT(TAILQ_EMPTY(&lun->pending_mgmt_tasks)); CU_ASSERT(TAILQ_EMPTY(&lun->pending_mgmt_tasks));
CU_ASSERT(TAILQ_EMPTY(&lun->mgmt_tasks)); CU_ASSERT(TAILQ_EMPTY(&lun->mgmt_tasks));
@ -713,15 +690,15 @@ abort_pending_mgmt_tasks_when_lun_is_removed(void)
CU_ASSERT(g_task_count == 3); CU_ASSERT(g_task_count == 3);
spdk_scsi_lun_append_mgmt_task(lun, &task1); scsi_lun_append_mgmt_task(lun, &task1);
spdk_scsi_lun_append_mgmt_task(lun, &task2); scsi_lun_append_mgmt_task(lun, &task2);
spdk_scsi_lun_append_mgmt_task(lun, &task3); scsi_lun_append_mgmt_task(lun, &task3);
CU_ASSERT(!TAILQ_EMPTY(&lun->pending_mgmt_tasks)); CU_ASSERT(!TAILQ_EMPTY(&lun->pending_mgmt_tasks));
lun->removed = true; lun->removed = true;
spdk_scsi_lun_execute_mgmt_task(lun); scsi_lun_execute_mgmt_task(lun);
CU_ASSERT(TAILQ_EMPTY(&lun->pending_mgmt_tasks)); CU_ASSERT(TAILQ_EMPTY(&lun->pending_mgmt_tasks));
CU_ASSERT(TAILQ_EMPTY(&lun->mgmt_tasks)); CU_ASSERT(TAILQ_EMPTY(&lun->mgmt_tasks));