Commit 3e787bba authored by Jim Harris's avatar Jim Harris Committed by Tomasz Zawadzki
Browse files

nvmf: initialize sgroup->queued when poll group is created 



We need sgroup->queued to be ready immediately, since it is possible that
a CONNECT could come in after the subsystem has been added but before it has
been fully activated.

So do the TAILQ_INIT on all sgroups when the poll group is first created.
This allows us to safely queue CONNECT commands on the sgroup if it comes in
while we are waiting to ACTIVATE.

Note that previously, if a subsystem was removed from some index and replaced
with another in that same index, any queued requests would have just been
leaked since we didn't check the TAILQ before the TAILQ_INIT. So instead of
just removing the original TAILQ_INIT, replace it with a check that will print
an ERRLOG, free those queued requests, and assert. It should never be possible
for this to happen but the checks will help debug it if it were ever to occur.

Fixes issue #3316.

Signed-off-by: default avatarJim Harris <jim.harris@samsung.com>
Change-Id: I401c4ef31feb9521c15daf5ebc7e2cb97da446a3
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/22474


Reviewed-by: default avatarPeng Lian <peng.lian@smartx.com>
Community-CI: Mellanox Build Bot
Reviewed-by: default avatarShuhei Matsumoto <smatsumoto@nvidia.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarTomasz Zawadzki <tomasz.zawadzki@intel.com>
parent b269b0ed

lib/nvmf/nvmf.c

+16 −1
Original line number Diff line number Diff line
@@ -259,6 +259,7 @@ nvmf_tgt_create_poll_group(void *io_device, void *ctx_buf) 
	struct spdk_nvmf_transport *transport;

	struct spdk_nvmf_subsystem *subsystem;

	struct spdk_thread *thread = spdk_get_thread();

	uint32_t i;

	int rc;



	group->tgt = tgt;
@@ -286,6 +287,10 @@ nvmf_tgt_create_poll_group(void *io_device, void *ctx_buf) 
		return -ENOMEM;

	}



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

		TAILQ_INIT(&group->sgroups[i].queued);

	}



	for (subsystem = spdk_nvmf_subsystem_get_first(tgt);

	     subsystem != NULL;

	     subsystem = spdk_nvmf_subsystem_get_next(subsystem)) {
@@ -1632,9 +1637,19 @@ nvmf_poll_group_add_subsystem(struct spdk_nvmf_poll_group *group, 
{

	int rc = 0;

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

	struct spdk_nvmf_request *req, *tmp;

	uint32_t i;



	TAILQ_INIT(&sgroup->queued);

	if (!TAILQ_EMPTY(&sgroup->queued)) {

		SPDK_ERRLOG("sgroup->queued not empty when adding subsystem\n");

		TAILQ_FOREACH_SAFE(req, &sgroup->queued, link, tmp) {

			TAILQ_REMOVE(&sgroup->queued, req, link);

			if (nvmf_transport_req_free(req)) {

				SPDK_ERRLOG("Transport request free error!\n");

			}

		}

		assert(false);

	}



	rc = poll_group_update_subsystem(group, subsystem);

	if (rc) {