Commit 16d0fbd0 authored by Ben Walker's avatar Ben Walker
Browse files

nvmf: Statically size the subsystems arrays 



The realloc breaks TAILQs inside the structures, which causes
subtle bugs. Instead, statically allocate all of the subsystem
arrays. This sets up the maximum number of subsystems to be
configurable, but does not actually expose it through the
config file yet.

Change-Id: I7347b6002b6babc0678ce59cd218a454fe3a6f88
Signed-off-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/410521


Tested-by: default avatarSPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: default avatarDaniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
parent bfb73837

include/spdk/nvmf.h

+1 −0
Original line number Diff line number Diff line
@@ -66,6 +66,7 @@ struct spdk_nvmf_tgt_opts { 
	uint16_t max_qpairs_per_ctrlr;

	uint32_t in_capsule_data_size;

	uint32_t max_io_size;

	uint32_t max_subsystems;

};

/**

 * Initialize the default value of opts.

lib/nvmf/ctrlr_discovery.c

+1 −1
Original line number Diff line number Diff line
@@ -69,7 +69,7 @@ nvmf_update_discovery_log(struct spdk_nvmf_tgt *tgt) 
		return;

	}



	for (sid = 0; sid < tgt->max_sid; sid++) {

	for (sid = 0; sid < tgt->opts.max_subsystems; sid++) {

		subsystem = tgt->subsystems[sid];

		if (subsystem == NULL) {

			continue;

lib/nvmf/nvmf.c

+14 −28
Original line number Diff line number Diff line
@@ -46,12 +46,11 @@ 


SPDK_LOG_REGISTER_COMPONENT("nvmf", SPDK_LOG_NVMF)



#define MAX_SUBSYSTEMS 4



#define SPDK_NVMF_DEFAULT_MAX_QUEUE_DEPTH 128

#define SPDK_NVMF_DEFAULT_MAX_QPAIRS_PER_CTRLR 64

#define SPDK_NVMF_DEFAULT_IN_CAPSULE_DATA_SIZE 4096

#define SPDK_NVMF_DEFAULT_MAX_IO_SIZE 131072

#define SPDK_NVMF_DEFAULT_MAX_SUBSYSTEMS 1024



void

spdk_nvmf_tgt_opts_init(struct spdk_nvmf_tgt_opts *opts)
@@ -60,6 +59,7 @@ spdk_nvmf_tgt_opts_init(struct spdk_nvmf_tgt_opts *opts) 
	opts->max_qpairs_per_ctrlr = SPDK_NVMF_DEFAULT_MAX_QPAIRS_PER_CTRLR;

	opts->in_capsule_data_size = SPDK_NVMF_DEFAULT_IN_CAPSULE_DATA_SIZE;

	opts->max_io_size = SPDK_NVMF_DEFAULT_MAX_IO_SIZE;

	opts->max_subsystems = SPDK_NVMF_DEFAULT_MAX_SUBSYSTEMS;

}



static int
@@ -95,13 +95,13 @@ spdk_nvmf_tgt_create_poll_group(void *io_device, void *ctx_buf) 
		spdk_nvmf_poll_group_add_transport(group, transport);

	}



	group->num_sgroups = tgt->max_sid;

	group->sgroups = calloc(group->num_sgroups, sizeof(struct spdk_nvmf_subsystem_poll_group));

	group->num_sgroups = tgt->opts.max_subsystems;

	group->sgroups = calloc(tgt->opts.max_subsystems, sizeof(struct spdk_nvmf_subsystem_poll_group));

	if (!group->sgroups) {

		return -1;

	}



	for (sid = 0; sid < group->num_sgroups; sid++) {

	for (sid = 0; sid < tgt->opts.max_subsystems; sid++) {

		struct spdk_nvmf_subsystem *subsystem;



		subsystem = tgt->subsystems[sid];
@@ -168,10 +168,14 @@ spdk_nvmf_tgt_create(struct spdk_nvmf_tgt_opts *opts) 
	tgt->discovery_genctr = 0;

	tgt->discovery_log_page = NULL;

	tgt->discovery_log_page_size = 0;

	tgt->subsystems = NULL;

	tgt->max_sid = 0;

	TAILQ_INIT(&tgt->transports);



	tgt->subsystems = calloc(tgt->opts.max_subsystems, sizeof(struct spdk_nvmf_subsystem *));

	if (!tgt->subsystems) {

		free(tgt);

		return NULL;

	}



	spdk_io_device_register(tgt,

				spdk_nvmf_tgt_create_poll_group,

				spdk_nvmf_tgt_destroy_poll_group,
@@ -198,7 +202,7 @@ spdk_nvmf_tgt_destroy(struct spdk_nvmf_tgt *tgt) 
	}



	if (tgt->subsystems) {

		for (i = 0; i < tgt->max_sid; i++) {

		for (i = 0; i < tgt->opts.max_subsystems; i++) {

			if (tgt->subsystems[i]) {

				spdk_nvmf_subsystem_destroy(tgt->subsystems[i]);

			}
@@ -311,7 +315,7 @@ spdk_nvmf_tgt_find_subsystem(struct spdk_nvmf_tgt *tgt, const char *subnqn) 
		return NULL;

	}



	for (sid = 0; sid < tgt->max_sid; sid++) {

	for (sid = 0; sid < tgt->opts.max_subsystems; sid++) {

		subsystem = tgt->subsystems[sid];

		if (subsystem == NULL) {

			continue;
@@ -445,23 +449,9 @@ poll_group_update_subsystem(struct spdk_nvmf_poll_group *group, 


	/* Make sure our poll group has memory for this subsystem allocated */

	if (subsystem->id >= group->num_sgroups) {

		void *buf;



		buf = realloc(group->sgroups, (subsystem->id + 1) * sizeof(*sgroup));

		if (!buf) {

		return -ENOMEM;

	}



		group->sgroups = buf;



		/* Zero out the newly allocated memory */

		memset(&group->sgroups[group->num_sgroups],

		       0,

		       (subsystem->id + 1 - group->num_sgroups) * sizeof(group->sgroups[0]));



		group->num_sgroups = subsystem->id + 1;

	}



	sgroup = &group->sgroups[subsystem->id];



	/* Make sure the array of channels is the correct size */
@@ -630,10 +620,6 @@ spdk_nvmf_poll_group_resume_subsystem(struct spdk_nvmf_poll_group *group, 
		return rc;

	}



	/* poll_group_update_subsystem may realloc the sgroups array. We need

	 * to do a new lookup to get the correct pointer. */

	sgroup = &group->sgroups[subsystem->id];



	sgroup->state = SPDK_NVMF_SUBSYSTEM_ACTIVE;



	/* Release all queued requests */

lib/nvmf/nvmf_internal.h

+1 −2
Original line number Diff line number Diff line
@@ -60,9 +60,8 @@ struct spdk_nvmf_tgt { 


	uint64_t				discovery_genctr;



	/* Array of subsystem pointers of size max_sid indexed by sid */

	/* Array of subsystem pointers of size max_subsystems indexed by sid */

	struct spdk_nvmf_subsystem		**subsystems;

	uint32_t				max_sid;



	struct spdk_nvmf_discovery_log_page	*discovery_log_page;

	size_t					discovery_log_page_size;

lib/nvmf/subsystem.c

+5 −13
Original line number Diff line number Diff line
@@ -253,22 +253,14 @@ spdk_nvmf_subsystem_create(struct spdk_nvmf_tgt *tgt, 
	}



	/* Find a free subsystem id (sid) */

	for (sid = 0; sid < tgt->max_sid; sid++) {

	for (sid = 0; sid < tgt->opts.max_subsystems; sid++) {

		if (tgt->subsystems[sid] == NULL) {

			break;

		}

	}

	if (sid == tgt->max_sid) {

		struct spdk_nvmf_subsystem **subsys_array;

		/* No free slots. Add more. */

		tgt->max_sid++;

		subsys_array = realloc(tgt->subsystems, tgt->max_sid * sizeof(struct spdk_nvmf_subsystem *));

		if (!subsys_array) {

			tgt->max_sid--;

	if (sid >= tgt->opts.max_subsystems) {

		return NULL;

	}

		tgt->subsystems = subsys_array;

	}



	subsystem = calloc(1, sizeof(struct spdk_nvmf_subsystem));

	if (subsystem == NULL) {
@@ -560,7 +552,7 @@ spdk_nvmf_subsystem_get_first(struct spdk_nvmf_tgt *tgt) 
	struct spdk_nvmf_subsystem	*subsystem;

	uint32_t sid;



	for (sid = 0; sid < tgt->max_sid; sid++) {

	for (sid = 0; sid < tgt->opts.max_subsystems; sid++) {

		subsystem = tgt->subsystems[sid];

		if (subsystem) {

			return subsystem;
@@ -582,7 +574,7 @@ spdk_nvmf_subsystem_get_next(struct spdk_nvmf_subsystem *subsystem) 


	tgt = subsystem->tgt;



	for (sid = subsystem->id + 1; sid < tgt->max_sid; sid++) {

	for (sid = subsystem->id + 1; sid < tgt->opts.max_subsystems; sid++) {

		subsystem = tgt->subsystems[sid];

		if (subsystem) {

			return subsystem;