Commit 27a30e7a authored by Jacek Kalwas's avatar Jacek Kalwas Committed by Tomasz Zawadzki
Browse files

nvme: move nvme_fabric_qpair_poll_cleanup to generic layer 



It is to apply recently introduced fixes in TCP to RDMA transport.

For details see
- nvme/tcp: fix qpair poll_status memleak
- nvme: fix qpair's fabric_poll_status leak
- nvme: use explicit fabric poll cleanup

This patch also introduces a behavioral change for TCP, as this cleanup
is no longer done immediately after disconnect but instead when
disconnect_done is invoked. This ensures that the cleanup occurs only
after the requests queued on the generic layer have been aborted. This
is mandatory for RDMA, which does not abort requests immediately upon
disconnect.

While here also removed redundant nvme_fabric_qpair_auth_cleanup
in spdk_nvme_ctrlr_free_io_qpair as it is covered by
nvme_transport_ctrlr_disconnect_qpair_done now.

Also moved assertion from nvme_tcp_ctrlr_delete_io_qpair to
nvme_qpair_deinit to cover both TCP and RDMA; poll/auth cleanup should
already be done in the _disconnect_done callback at this stage, and
_abort_reqs should be a nop.

Change-Id: I1aa955f8e1470b78f57f657ae9c70ec3920c00cb
Signed-off-by: default avatarJacek Kalwas <jacek.kalwas@nutanix.com>
Reviewed-on: https://review.spdk.io/c/spdk/spdk/+/26806


Reviewed-by: default avatarKonrad Sztyber <ksztyber@nvidia.com>
Tested-by: default avatarSPDK Automated Test System <spdkbot@gmail.com>
Community-CI: Mellanox Build Bot
Reviewed-by: default avatarJim Harris <jim.harris@nvidia.com>
parent 9d890d3f

lib/nvme/nvme_ctrlr.c

+0 −2
Original line number Diff line number Diff line
@@ -608,8 +608,6 @@ spdk_nvme_ctrlr_free_io_qpair(struct spdk_nvme_qpair *qpair) 
		return 0;

	}



	nvme_fabric_qpair_auth_cleanup(qpair, -ECANCELED);



	qpair->destroy_in_progress = 1;



	nvme_transport_ctrlr_disconnect_qpair(ctrlr, qpair);

lib/nvme/nvme_qpair.c

+2 −0
Original line number Diff line number Diff line
@@ -1008,6 +1008,8 @@ nvme_qpair_deinit(struct spdk_nvme_qpair *qpair) 
{

	struct nvme_error_cmd *cmd, *entry;



	assert(!qpair->fabric_poll_status);



	nvme_qpair_abort_queued_reqs(qpair);

	_nvme_qpair_complete_abort_queued_reqs(qpair);

	nvme_qpair_complete_error_reqs(qpair);

lib/nvme/nvme_tcp.c

+0 −7
Original line number Diff line number Diff line
@@ -444,12 +444,6 @@ nvme_tcp_ctrlr_disconnect_qpair(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_ 
		assert(TAILQ_EMPTY(&tqpair->outstanding_reqs));

		nvme_transport_ctrlr_disconnect_qpair_done(qpair);

	}



	/* A Fabric command may be outstanding before a disconnect is invoked. */

	if (qpair->fabric_poll_status && !(qpair->auth.flags.in_auth_poll || qpair->in_connect_poll)) {

		nvme_fabric_qpair_poll_cleanup(qpair);

		nvme_fabric_qpair_auth_cleanup(qpair, -ECANCELED);

	}

}



static int
@@ -461,7 +455,6 @@ nvme_tcp_ctrlr_delete_io_qpair(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_q 
	nvme_tcp_qpair_abort_reqs(qpair, qpair->abort_dnr);

	assert(TAILQ_EMPTY(&tqpair->outstanding_reqs));



	assert(!qpair->fabric_poll_status);

	nvme_qpair_deinit(qpair);

	nvme_tcp_free_reqs(tqpair);

	if (!tqpair->shared_stats) {

lib/nvme/nvme_transport.c

+6 −0
Original line number Diff line number Diff line
@@ -598,6 +598,12 @@ nvme_transport_ctrlr_disconnect_qpair_done(struct spdk_nvme_qpair *qpair) 
	if (qpair->poll_group) {

		nvme_poll_group_write_disconnect_qpair_fd(qpair->poll_group->group);

	}



	/* A Fabric command may be outstanding before a disconnect was invoked. */

	if (qpair->fabric_poll_status && !(qpair->auth.flags.in_auth_poll || qpair->in_connect_poll)) {

		nvme_fabric_qpair_poll_cleanup(qpair);

		nvme_fabric_qpair_auth_cleanup(qpair, -ECANCELED);

	}

}



int