From 9396cb9a943bdcae89a07879f0bf0629b15ae723 Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Wed, 10 Aug 2022 20:56:23 +0000 Subject: [PATCH] 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: Jim Harris Change-Id: I5ccc41c14abd541ffcf2a602246e0671386840c7 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/13991 Community-CI: Mellanox Build Bot Reviewed-by: Aleksey Marchuk Reviewed-by: Ben Walker Tested-by: SPDK CI Jenkins --- lib/nvme/nvme_tcp.c | 3 +-- test/unit/lib/nvme/nvme_tcp.c/nvme_tcp_ut.c | 6 +++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/nvme/nvme_tcp.c b/lib/nvme/nvme_tcp.c index 15e367a40..149410e51 100644 --- a/lib/nvme/nvme_tcp.c +++ b/lib/nvme/nvme_tcp.c @@ -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); } diff --git a/test/unit/lib/nvme/nvme_tcp.c/nvme_tcp_ut.c b/test/unit/lib/nvme/nvme_tcp.c/nvme_tcp_ut.c index fa802b2b1..89ede297f 100644 --- a/test/unit/lib/nvme/nvme_tcp.c/nvme_tcp_ut.c +++ b/test/unit/lib/nvme/nvme_tcp.c/nvme_tcp_ut.c @@ -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 */