Commit 58216dd0 authored by Seth Howell's avatar Seth Howell Committed by Tomasz Zawadzki
Browse files

lib/nvme: fix mem leak in req submit. 



Signed-off-by: default avatarSeth Howell <seth.howell@intel.com>
Change-Id: If64c06177605a8f57d87ba22b86fe58ddebd6f7a
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3921


Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: default avatarMichael Haeuptle <michaelhaeuptle@gmail.com>
Reviewed-by: default avatarPaul Luse <paul.e.luse@intel.com>
parent 4803dc36

lib/nvme/nvme_qpair.c

+12 −11
Original line number Diff line number Diff line
@@ -826,7 +826,18 @@ _nvme_qpair_submit_request(struct spdk_nvme_qpair *qpair, struct nvme_request *r 


	nvme_qpair_check_enabled(qpair);



	if (nvme_qpair_get_state(qpair) == NVME_QPAIR_DISCONNECTED) {

	if (spdk_unlikely(nvme_qpair_get_state(qpair) == NVME_QPAIR_DISCONNECTED ||

			  nvme_qpair_get_state(qpair) == NVME_QPAIR_DISCONNECTING ||

			  nvme_qpair_get_state(qpair) == NVME_QPAIR_DESTROYING)) {

		TAILQ_FOREACH_SAFE(child_req, &req->children, child_tailq, tmp) {

			nvme_request_remove_child(req, child_req);

			nvme_request_free_children(child_req);

			nvme_free_request(child_req);

		}

		if (req->parent != NULL) {

			nvme_request_remove_child(req->parent, req);

		}

		nvme_free_request(req);

		return -ENXIO;

	}
@@ -946,16 +957,6 @@ nvme_qpair_submit_request(struct spdk_nvme_qpair *qpair, struct nvme_request *re 
{

	int rc;



	/* This prevents us from entering an infinite loop when freeing queued I/O in disconnect. */

	if (spdk_unlikely(nvme_qpair_get_state(qpair) == NVME_QPAIR_DISCONNECTING ||

			  nvme_qpair_get_state(qpair) == NVME_QPAIR_DESTROYING)) {

		if (req->parent != NULL) {

			nvme_request_remove_child(req->parent, req);

		}

		nvme_free_request(req);

		return -ENXIO;

	}



	if (spdk_unlikely(!STAILQ_EMPTY(&qpair->queued_req) && req->num_children == 0)) {

		/*

		 * requests that have no children should be sent to the transport after all

test/unit/lib/nvme/nvme_qpair.c/nvme_qpair_ut.c

+30 −15
Original line number Diff line number Diff line
@@ -496,16 +496,11 @@ test_nvme_qpair_add_cmd_error_injection(void) 
	cleanup_submit_request_test(&qpair);

}



static void

test_nvme_qpair_submit_request(void)

static struct nvme_request *

allocate_request_tree(struct spdk_nvme_qpair *qpair)

{

	int				rc;

	struct spdk_nvme_qpair		qpair = {};

	struct spdk_nvme_ctrlr		ctrlr = {};

	struct nvme_request	*req, *req1, *req2, *req3, *req2_1, *req2_2, *req2_3;



	prepare_submit_request_test(&qpair, &ctrlr);



	/*

	 *  Build a request chain like the following:

	 *            req
@@ -518,51 +513,71 @@ test_nvme_qpair_submit_request(void) 
	 *     |       |       |

	 *   req2_1  req2_2  req2_3

	 */

	req = nvme_allocate_request_null(&qpair, NULL, NULL);

	req = nvme_allocate_request_null(qpair, NULL, NULL);

	CU_ASSERT(req != NULL);

	TAILQ_INIT(&req->children);



	req1 = nvme_allocate_request_null(&qpair, NULL, NULL);

	req1 = nvme_allocate_request_null(qpair, NULL, NULL);

	CU_ASSERT(req1 != NULL);

	req->num_children++;

	TAILQ_INSERT_TAIL(&req->children, req1, child_tailq);

	req1->parent = req;



	req2 = nvme_allocate_request_null(&qpair, NULL, NULL);

	req2 = nvme_allocate_request_null(qpair, NULL, NULL);

	CU_ASSERT(req2 != NULL);

	TAILQ_INIT(&req2->children);

	req->num_children++;

	TAILQ_INSERT_TAIL(&req->children, req2, child_tailq);

	req2->parent = req;



	req3 = nvme_allocate_request_null(&qpair, NULL, NULL);

	req3 = nvme_allocate_request_null(qpair, NULL, NULL);

	CU_ASSERT(req3 != NULL);

	req->num_children++;

	TAILQ_INSERT_TAIL(&req->children, req3, child_tailq);

	req3->parent = req;



	req2_1 = nvme_allocate_request_null(&qpair, NULL, NULL);

	req2_1 = nvme_allocate_request_null(qpair, NULL, NULL);

	CU_ASSERT(req2_1 != NULL);

	req2->num_children++;

	TAILQ_INSERT_TAIL(&req2->children, req2_1, child_tailq);

	req2_1->parent = req2;



	req2_2 = nvme_allocate_request_null(&qpair, NULL, NULL);

	req2_2 = nvme_allocate_request_null(qpair, NULL, NULL);

	CU_ASSERT(req2_2 != NULL);

	req2->num_children++;

	TAILQ_INSERT_TAIL(&req2->children, req2_2, child_tailq);

	req2_2->parent = req2;



	req2_3 = nvme_allocate_request_null(&qpair, NULL, NULL);

	req2_3 = nvme_allocate_request_null(qpair, NULL, NULL);

	CU_ASSERT(req2_3 != NULL);

	req2->num_children++;

	TAILQ_INSERT_TAIL(&req2->children, req2_3, child_tailq);

	req2_3->parent = req2;



	return req;

}



static void

test_nvme_qpair_submit_request(void)

{

	int				rc;

	struct spdk_nvme_qpair		qpair = {};

	struct spdk_nvme_ctrlr		ctrlr = {};

	struct nvme_request		*req;



	prepare_submit_request_test(&qpair, &ctrlr);



	req = allocate_request_tree(&qpair);

	ctrlr.is_failed = true;

	rc = nvme_qpair_submit_request(&qpair, req);

	SPDK_CU_ASSERT_FATAL(rc == -ENXIO);



	req = allocate_request_tree(&qpair);

	ctrlr.is_failed = false;

	qpair.state = NVME_QPAIR_DISCONNECTING;

	rc = nvme_qpair_submit_request(&qpair, req);

	SPDK_CU_ASSERT_FATAL(rc == -ENXIO);



	cleanup_submit_request_test(&qpair);

}