Commit 9396cb9a authored by Jim Harris's avatar Jim Harris Committed by Tomasz Zawadzki
Browse files

nvme/tcp: simplify outstanding_reqs handling 



Avoid putting a new req on the outstanding_reqs
TAILQ until we know it can be initialized
successfully.  This avoids adding to the TAILQ
only to remove it just after.

This allow simplifies the outstanding_reqs TAILQ
handling, since reqs are now only inserted and
removed in one place each.

Signed-off-by: default avatarJim Harris <james.r.harris@intel.com>
Change-Id: I5ccc41c14abd541ffcf2a602246e0671386840c7
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/13991


Community-CI: Mellanox Build Bot
Reviewed-by: default avatarAleksey Marchuk <alexeymar@nvidia.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent b0396da0

lib/nvme/nvme_tcp.c

+1 −2
Original line number Diff line number Diff line
@@ -200,7 +200,6 @@ nvme_tcp_req_get(struct nvme_tcp_qpair *tqpair) 
	tcp_req->ordering.raw = 0;

	memset(tcp_req->pdu, 0, sizeof(struct nvme_tcp_pdu));

	memset(&tcp_req->rsp, 0, sizeof(struct spdk_nvme_cpl));

	TAILQ_INSERT_TAIL(&tqpair->outstanding_reqs, tcp_req, link);



	return tcp_req;

}
@@ -727,11 +726,11 @@ nvme_tcp_qpair_submit_request(struct spdk_nvme_qpair *qpair, 


	if (nvme_tcp_req_init(tqpair, req, tcp_req)) {

		SPDK_ERRLOG("nvme_tcp_req_init() failed\n");

		TAILQ_REMOVE(&tcp_req->tqpair->outstanding_reqs, tcp_req, link);

		nvme_tcp_req_put(tqpair, tcp_req);

		return -1;

	}



	TAILQ_INSERT_TAIL(&tqpair->outstanding_reqs, tcp_req, link);

	return nvme_tcp_qpair_capsule_cmd_send(tqpair, tcp_req);

}

test/unit/lib/nvme/nvme_tcp.c/nvme_tcp_ut.c

+5 −1
Original line number Diff line number Diff line
@@ -581,7 +581,11 @@ test_nvme_tcp_req_get(void) 
	CU_ASSERT(tcp_req.r2tl_remain == 0);

	CU_ASSERT(tcp_req.iovcnt == 0);

	CU_ASSERT(tcp_req.ordering.raw == 0);

	CU_ASSERT(!TAILQ_EMPTY(&tqpair.outstanding_reqs));

	/* outstanding_reqs should still be empty - caller is responsible

	 * for putting it on the TAILQ after any other initialization is

	 * completed.

	 */

	CU_ASSERT(TAILQ_EMPTY(&tqpair.outstanding_reqs));

	CU_ASSERT(TAILQ_EMPTY(&tqpair.free_reqs));



	/* No tcp request available, expect fail */