Commit 4265fc50 authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Tomasz Zawadzki
Browse files

scsi: Manage LUNs per device by not fixed size array but linked list 



Change a fixed size array to a linked list to manage LUNs per SCSI
device.

Keep the linked list sorted by LUN ID because this is necessary to
efficiently find the lowest free LUN ID or check the specified LUN is free.

To avoid traversing the linked list twice, change scsi_dev_find_free_lun()
to return the LUN which comes just before where we want to insert an new LUN.

Additionally, previously spdk_scsi_dev_add_lun_ext() had not checked if
the specified LUN ID was duplicated. Fix the bug in this patch.

Add unit test cases for the function scsi_dev_find_free_lun().

These changes will enable the following patches to increase
SPDK_SCSI_DEV_MAX_LUN from 64 to 256 without consuming additional memory.

Signed-off-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I7f6f070ddc680127cf86ae255055da2d1d29e4ec
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9630


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
parent e9030f36

lib/scsi/dev.c

+96 −79
Original line number Diff line number Diff line
@@ -54,6 +54,7 @@ allocate_dev(void) 
			memset(dev, 0, sizeof(*dev));

			dev->id = i;

			dev->is_allocated = 1;

			TAILQ_INIT(&dev->luns);

			return dev;

		}

	}
@@ -79,8 +80,7 @@ void 
spdk_scsi_dev_destruct(struct spdk_scsi_dev *dev,

		       spdk_scsi_dev_destruct_cb_t cb_fn, void *cb_arg)

{

	int lun_cnt;

	int i;

	struct spdk_scsi_lun *lun, *tmp_lun;



	if (dev == NULL) {

		if (cb_fn) {
@@ -99,39 +99,62 @@ spdk_scsi_dev_destruct(struct spdk_scsi_dev *dev, 
	dev->removed = true;

	dev->remove_cb = cb_fn;

	dev->remove_ctx = cb_arg;

	lun_cnt = 0;



	for (i = 0; i < SPDK_SCSI_DEV_MAX_LUN; i++) {

		if (dev->lun[i] == NULL) {

			continue;

	if (TAILQ_EMPTY(&dev->luns)) {

		free_dev(dev);

		return;

	}



	TAILQ_FOREACH_SAFE(lun, &dev->luns, tailq, tmp_lun) {

		/*

		 * LUN will remove itself from this dev when all outstanding IO

		 * is done. When no more LUNs, dev will be deleted.

		 */

		scsi_lun_destruct(dev->lun[i]);

		lun_cnt++;

	}



	if (lun_cnt == 0) {

		free_dev(dev);

		return;

		scsi_lun_destruct(lun);

	}

}



/*

 * Search the lowest free LUN ID if the LUN ID is default, or check if the LUN ID is free otherwise,

 * and also return the LUN which comes just before where we want to insert an new LUN.

 */

static int

scsi_dev_find_lowest_free_lun_id(struct spdk_scsi_dev *dev)

scsi_dev_find_free_lun(struct spdk_scsi_dev *dev, int lun_id,

		       struct spdk_scsi_lun **prev_lun)

{

	int i;

	struct spdk_scsi_lun *lun, *_prev_lun = NULL;



	for (i = 0; i < SPDK_SCSI_DEV_MAX_LUN; i++) {

		if (dev->lun[i] == NULL) {

			return i;

	if (prev_lun == NULL) {

		return -EINVAL;

	}



	if (lun_id == -1) {

		lun_id = 0;



		TAILQ_FOREACH(lun, &dev->luns, tailq) {

			if (lun->id > lun_id) {

				break;

			}

			lun_id = lun->id + 1;

			_prev_lun = lun;

		}



	return -1;

		if (lun_id >= SPDK_SCSI_DEV_MAX_LUN) {

			return -ENOSPC;

		}

	} else {

		TAILQ_FOREACH(lun, &dev->luns, tailq) {

			if (lun->id == lun_id) {

				return -EEXIST;

			} else if (lun->id > lun_id) {

				break;

			}

			_prev_lun = lun;

		}

	}



	*prev_lun = _prev_lun;

	return 0;

}



int
@@ -151,20 +174,18 @@ spdk_scsi_dev_add_lun_ext(struct spdk_scsi_dev *dev, const char *bdev_name, int 
			  void (*hotremove_cb)(const struct spdk_scsi_lun *, void *),

			  void *hotremove_ctx)

{

	struct spdk_scsi_lun *lun;

	struct spdk_scsi_lun *lun, *prev_lun = NULL;

	int rc;



	if (lun_id >= SPDK_SCSI_DEV_MAX_LUN) {

		SPDK_ERRLOG("LUN ID %d is more than the maximum.\n", lun_id);

		return -1;

	}



	/* Search the lowest free LUN ID if LUN ID is default */

	if (lun_id == -1) {

		lun_id = scsi_dev_find_lowest_free_lun_id(dev);

		if (lun_id == -1) {

			SPDK_ERRLOG("Free LUN ID is not found\n");

			return -1;

		}

	rc = scsi_dev_find_free_lun(dev, lun_id, &prev_lun);

	if (rc != 0) {

		SPDK_ERRLOG("%s\n", rc == -EEXIST ? "LUN ID is duplicated" : "Free LUN ID is not found");

		return rc;

	}



	lun = scsi_lun_construct(bdev_name, resize_cb, resize_ctx, hotremove_cb, hotremove_ctx);
@@ -172,9 +193,21 @@ spdk_scsi_dev_add_lun_ext(struct spdk_scsi_dev *dev, const char *bdev_name, int 
		return -1;

	}



	lun->id = lun_id;

	lun->dev = dev;

	dev->lun[lun_id] = lun;



	if (lun_id != -1) {

		lun->id = lun_id;

	} else if (prev_lun == NULL) {

		lun->id = 0;

	} else {

		lun->id = prev_lun->id + 1;

	}



	if (prev_lun == NULL) {

		TAILQ_INSERT_HEAD(&dev->luns, lun, tailq);

	} else {

		TAILQ_INSERT_AFTER(&dev->luns, prev_lun, lun, tailq);

	}

	return 0;

}
@@ -182,20 +215,9 @@ void 
spdk_scsi_dev_delete_lun(struct spdk_scsi_dev *dev,

			 struct spdk_scsi_lun *lun)

{

	int lun_cnt = 0;

	int i;

	TAILQ_REMOVE(&dev->luns, lun, tailq);



	for (i = 0; i < SPDK_SCSI_DEV_MAX_LUN; i++) {

		if (dev->lun[i] == lun) {

			dev->lun[i] = NULL;

		}



		if (dev->lun[i]) {

			lun_cnt++;

		}

	}



	if (dev->removed == true && lun_cnt == 0) {

	if (dev->removed && TAILQ_EMPTY(&dev->luns)) {

		free_dev(dev);

	}

}
@@ -382,26 +404,21 @@ spdk_scsi_dev_find_port_by_id(struct spdk_scsi_dev *dev, uint64_t id) 
void

spdk_scsi_dev_free_io_channels(struct spdk_scsi_dev *dev)

{

	int i;

	struct spdk_scsi_lun *lun, *tmp_lun;



	for (i = 0; i < SPDK_SCSI_DEV_MAX_LUN; i++) {

		if (dev->lun[i] == NULL) {

			continue;

		}

		scsi_lun_free_io_channel(dev->lun[i]);

	TAILQ_FOREACH_SAFE(lun, &dev->luns, tailq, tmp_lun) {

		scsi_lun_free_io_channel(lun);

	}

}



int

spdk_scsi_dev_allocate_io_channels(struct spdk_scsi_dev *dev)

{

	int i, rc;

	struct spdk_scsi_lun *lun, *tmp_lun;

	int rc;



	for (i = 0; i < SPDK_SCSI_DEV_MAX_LUN; i++) {

		if (dev->lun[i] == NULL) {

			continue;

		}

		rc = scsi_lun_allocate_io_channel(dev->lun[i]);

	TAILQ_FOREACH_SAFE(lun, &dev->luns, tailq, tmp_lun) {

		rc = scsi_lun_allocate_io_channel(lun);

		if (rc < 0) {

			spdk_scsi_dev_free_io_channels(dev);

			return -1;
@@ -428,28 +445,26 @@ spdk_scsi_dev_get_lun(struct spdk_scsi_dev *dev, int lun_id) 
{

	struct spdk_scsi_lun *lun;



	if (lun_id < 0 || lun_id >= SPDK_SCSI_DEV_MAX_LUN) {

		return NULL;

	}



	lun = dev->lun[lun_id];



	if (lun != NULL && !spdk_scsi_lun_is_removing(lun)) {

	TAILQ_FOREACH(lun, &dev->luns, tailq) {

		if (lun->id == lun_id) {

			if (!spdk_scsi_lun_is_removing(lun)) {

				return lun;

			} else {

				return NULL;

			}

		}

	}



	return NULL;

}



struct spdk_scsi_lun *

spdk_scsi_dev_get_first_lun(struct spdk_scsi_dev *dev)

{

	struct spdk_scsi_lun *lun;

	int lun_id;



	for (lun_id = 0; lun_id < SPDK_SCSI_DEV_MAX_LUN; lun_id++) {

		lun = dev->lun[lun_id];

		if (lun != NULL && !spdk_scsi_lun_is_removing(lun)) {

	TAILQ_FOREACH(lun, &dev->luns, tailq) {

		if (!spdk_scsi_lun_is_removing(lun)) {

			return lun;

		}

	}
@@ -462,7 +477,6 @@ spdk_scsi_dev_get_next_lun(struct spdk_scsi_lun *prev_lun) 
{

	struct spdk_scsi_dev *dev;

	struct spdk_scsi_lun *lun;

	int lun_id;



	if (prev_lun == NULL) {

		return NULL;
@@ -470,26 +484,29 @@ spdk_scsi_dev_get_next_lun(struct spdk_scsi_lun *prev_lun) 


	dev = prev_lun->dev;



	for (lun_id = prev_lun->id + 1; lun_id < SPDK_SCSI_DEV_MAX_LUN; lun_id++) {

		lun = dev->lun[lun_id];

		if (lun != NULL && !spdk_scsi_lun_is_removing(lun)) {

			return lun;

	lun = TAILQ_NEXT(prev_lun, tailq);

	if (lun == NULL) {

		return NULL;

	}



	TAILQ_FOREACH_FROM(lun, &dev->luns, tailq) {

		if (!spdk_scsi_lun_is_removing(lun)) {

			break;

		}

	}



	return NULL;

	return lun;

}



bool

spdk_scsi_dev_has_pending_tasks(const struct spdk_scsi_dev *dev,

				const struct spdk_scsi_port *initiator_port)

{

	int i;

	struct spdk_scsi_lun *lun;



	for (i = 0; i < SPDK_SCSI_DEV_MAX_LUN; ++i) {

		if (dev->lun[i] &&

		    (scsi_lun_has_pending_tasks(dev->lun[i], initiator_port) ||

		     scsi_lun_has_pending_mgmt_tasks(dev->lun[i], initiator_port))) {

	TAILQ_FOREACH(lun, &dev->luns, tailq) {

		if (scsi_lun_has_pending_tasks(lun, initiator_port) ||

		    scsi_lun_has_pending_mgmt_tasks(lun, initiator_port)) {

			return true;

		}

	}

lib/scsi/scsi_internal.h

+4 −1
Original line number Diff line number Diff line
@@ -92,7 +92,7 @@ struct spdk_scsi_dev { 


	char					name[SPDK_SCSI_DEV_MAX_NAME + 1];



	struct spdk_scsi_lun			*lun[SPDK_SCSI_DEV_MAX_LUN];

	TAILQ_HEAD(, spdk_scsi_lun)		luns;



	int					num_ports;

	struct spdk_scsi_port			port[SPDK_SCSI_DEV_MAX_PORTS];
@@ -173,6 +173,9 @@ struct spdk_scsi_lun { 


	/** poller to check completion of tasks prior to reset */

	struct spdk_poller *reset_poller;



	/** A structure to connect LUNs in a list. */

	TAILQ_ENTRY(spdk_scsi_lun) tailq;

};



struct spdk_scsi_lun *scsi_lun_construct(const char *bdev_name,

test/unit/lib/iscsi/common.c

+5 −2
Original line number Diff line number Diff line
@@ -113,9 +113,12 @@ DEFINE_STUB_V(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)

{

	assert(dev->lun[0] != NULL);

	struct spdk_scsi_lun *lun;



	TAILQ_INSERT_TAIL(&dev->lun[0]->tasks, task, scsi_link);

	lun = TAILQ_FIRST(&dev->luns);

	SPDK_CU_ASSERT_FATAL(lun != NULL);



	TAILQ_INSERT_TAIL(&lun->tasks, task, scsi_link);

}



DEFINE_STUB(spdk_scsi_dev_find_port_by_id, struct spdk_scsi_port *,

test/unit/lib/iscsi/iscsi.c/iscsi_ut.c

+26 −12
Original line number Diff line number Diff line
@@ -131,11 +131,15 @@ DEFINE_STUB(spdk_scsi_lun_is_removing, bool, (const struct spdk_scsi_lun *lun), 
struct spdk_scsi_lun *

spdk_scsi_dev_get_lun(struct spdk_scsi_dev *dev, int lun_id)

{

	if (lun_id < 0 || lun_id >= SPDK_SCSI_DEV_MAX_LUN) {

		return NULL;

	struct spdk_scsi_lun *lun;



	TAILQ_FOREACH(lun, &dev->luns, tailq) {

		if (lun->id == lun_id) {

			break;

		}

	}



	return dev->lun[lun_id];

	return lun;

}



DEFINE_STUB(spdk_scsi_lun_id_int_to_fmt, uint64_t, (int lun_id), 0);
@@ -294,7 +298,8 @@ maxburstlength_test(void) 


	lun.id = 0;



	dev.lun[0] = &lun;

	TAILQ_INIT(&dev.luns);

	TAILQ_INSERT_TAIL(&dev.luns, &lun, tailq);



	conn.full_feature = 1;

	conn.sess = &sess;
@@ -380,7 +385,8 @@ underflow_for_read_transfer_test(void) 
	conn.sess = &sess;

	conn.MaxRecvDataSegmentLength = 8192;



	dev.lun[0] = &lun;

	TAILQ_INIT(&dev.luns);

	TAILQ_INSERT_TAIL(&dev.luns, &lun, tailq);

	conn.dev = &dev;



	pdu = iscsi_get_pdu(&conn);
@@ -443,7 +449,8 @@ underflow_for_zero_read_transfer_test(void) 
	conn.sess = &sess;

	conn.MaxRecvDataSegmentLength = 8192;



	dev.lun[0] = &lun;

	TAILQ_INIT(&dev.luns);

	TAILQ_INSERT_TAIL(&dev.luns, &lun, tailq);

	conn.dev = &dev;



	pdu = iscsi_get_pdu(&conn);
@@ -508,7 +515,8 @@ underflow_for_request_sense_test(void) 
	conn.sess = &sess;

	conn.MaxRecvDataSegmentLength = 8192;



	dev.lun[0] = &lun;

	TAILQ_INIT(&dev.luns);

	TAILQ_INSERT_TAIL(&dev.luns, &lun, tailq);

	conn.dev = &dev;



	pdu1 = iscsi_get_pdu(&conn);
@@ -598,7 +606,8 @@ underflow_for_check_condition_test(void) 
	conn.sess = &sess;

	conn.MaxRecvDataSegmentLength = 8192;



	dev.lun[0] = &lun;

	TAILQ_INIT(&dev.luns);

	TAILQ_INSERT_TAIL(&dev.luns, &lun, tailq);

	conn.dev = &dev;



	pdu = iscsi_get_pdu(&conn);
@@ -1627,7 +1636,8 @@ pdu_hdr_op_scsi_test(void) 
	CU_ASSERT(pdu.task == NULL);



	/* Case 5 - SCSI read command PDU is correct, and the configured iSCSI task is set to the PDU. */

	dev.lun[0] = &lun;

	TAILQ_INIT(&dev.luns);

	TAILQ_INSERT_TAIL(&dev.luns, &lun, tailq);



	rc = iscsi_pdu_hdr_op_scsi(&conn, &pdu);

	CU_ASSERT(rc == 0);
@@ -1750,7 +1760,8 @@ pdu_hdr_op_task_mgmt_test(void) 
	check_iscsi_task_mgmt_response(ISCSI_TASK_FUNC_RESP_LUN_NOT_EXIST, 1234, 0, 0, 1);



	/* Case 3 - Unassigned function is specified.  "Function rejected" response is sent. */

	dev.lun[0] = &lun;

	TAILQ_INIT(&dev.luns);

	TAILQ_INSERT_TAIL(&dev.luns, &lun, tailq);

	task_reqh->flags = 0;



	rc = iscsi_pdu_hdr_op_task(&conn, &pdu);
@@ -1960,7 +1971,8 @@ pdu_hdr_op_data_test(void) 
	/* Case 10 - SCSI Data-Out PDU is correct and processed. Its F bit is 0 and hence

	 * R2T is not sent.

	 */

	dev.lun[0] = &lun;

	TAILQ_INIT(&dev.luns);

	TAILQ_INSERT_TAIL(&dev.luns, &lun, tailq);

	to_be32(&data_reqh->data_sn, primary.r2t_datasn);

	to_be32(&data_reqh->buffer_offset, primary.next_expected_r2t_offset);
@@ -2179,7 +2191,7 @@ static void 
data_out_pdu_sequence_test(void)

{

	struct spdk_scsi_lun lun = { .tasks = TAILQ_HEAD_INITIALIZER(lun.tasks), };

	struct spdk_scsi_dev dev = { .lun[0] = &lun,    };

	struct spdk_scsi_dev dev = { .luns = TAILQ_HEAD_INITIALIZER(dev.luns), };

	struct spdk_iscsi_sess sess = {

		.session_type = SESSION_TYPE_NORMAL,

		.MaxBurstLength = SPDK_ISCSI_MAX_BURST_LENGTH,
@@ -2197,6 +2209,8 @@ data_out_pdu_sequence_test(void) 
	struct iscsi_bhs_data_out *data_reqh;

	int rc;



	TAILQ_INSERT_TAIL(&dev.luns, &lun, tailq);



	mobj1.buf = calloc(1, SPDK_BDEV_BUF_SIZE_WITH_MD(SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH));

	SPDK_CU_ASSERT_FATAL(mobj1.buf != NULL);

test/unit/lib/iscsi/tgt_node.c/tgt_node_ut.c

+7 −3
Original line number Diff line number Diff line
@@ -91,11 +91,15 @@ DEFINE_STUB(iscsi_portal_grp_find_portal_by_addr, 
struct spdk_scsi_lun *

spdk_scsi_dev_get_lun(struct spdk_scsi_dev *dev, int lun_id)

{

	if (lun_id < 0 || lun_id >= SPDK_SCSI_DEV_MAX_LUN) {

		return NULL;

	struct spdk_scsi_lun *lun;



	TAILQ_FOREACH(lun, &dev->luns, tailq) {

		if (lun->id == lun_id) {

			break;

		}

	}



	return dev->lun[lun_id];

	return lun;

}



int