Commit 3ef8dc10 authored by Dariusz Stojaczyk's avatar Dariusz Stojaczyk Committed by Jim Harris
Browse files

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: default avatarDariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/393911


Tested-by: default avatarSPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: default avatar <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
parent 77311f59

lib/scsi/dev.c

+1 −4
Original line number Diff line number Diff line
@@ -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 *

lib/scsi/lun.c

+29 −69
Original line number Diff line number Diff line
@@ -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,23 +150,13 @@ 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);
@@ -218,7 +180,6 @@ spdk_scsi_lun_execute_tasks(struct spdk_scsi_lun *lun) 
		abort();

	}

}

}



static void

spdk_scsi_lun_hotplug(void *arg)
@@ -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);

}

lib/scsi/scsi_internal.h

+2 −4
Original line number Diff line number Diff line
@@ -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);

test/unit/lib/scsi/dev.c/dev_ut.c

+1 −7
Original line number Diff line number Diff line
@@ -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)

{

}

test/unit/lib/scsi/lun.c/lun_ut.c

+6 −30
Original line number Diff line number Diff line
@@ -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));