From ad69e739e12274ebc4c7690bb25b9e563884ce2d Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Mon, 29 Jun 2020 23:32:45 +0900 Subject: [PATCH] nvme/tcp: Dequeue request from outstanding list before calling completion Each request has a callback context as cb_arg, and the callback to nvme_complete_request() for the completed request may reuse the context to the new request. On the other hand, TCP transport dequeues tcp_req from tqpair->outstanding_reqs after calling nvme_complete_request() for the request pointe by tcp_req. Hence while nvme_complete_request() is executed, tqpair->outstanding_reqs may have two requests which has the same callback context, the completed request and the new submitted request. The upcoming patch will search all requests whose cb_arg matches to abort them. In the above case, the search may find two requests by mistake. To avoid such error, move dequeueing tcp_req from tqpair->outstanding_reqs before calling nvme_request_complete(). One exception is the case that only nvme_tcp_req_put() is called. For the case remove tcp_req from tqpair->outstanding_reqs before calling nvme_tcp_req_put(). Signed-off-by: Shuhei Matsumoto Change-Id: I5f2ac292c60431ac1e27b8657db92b220860a0a8 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2865 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Aleksey Marchuk Reviewed-by: Ben Walker Reviewed-by: Michael Haeuptle Reviewed-by: Jim Harris --- lib/nvme/nvme_tcp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/nvme/nvme_tcp.c b/lib/nvme/nvme_tcp.c index 10484e25d..036ff5550 100644 --- a/lib/nvme/nvme_tcp.c +++ b/lib/nvme/nvme_tcp.c @@ -187,7 +187,6 @@ nvme_tcp_req_put(struct nvme_tcp_qpair *tqpair, struct nvme_tcp_req *tcp_req) { assert(tcp_req->state != NVME_TCP_REQ_FREE); tcp_req->state = NVME_TCP_REQ_FREE; - TAILQ_REMOVE(&tqpair->outstanding_reqs, tcp_req, link); TAILQ_INSERT_TAIL(&tqpair->free_reqs, tcp_req, link); } @@ -603,6 +602,7 @@ 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; } @@ -625,6 +625,7 @@ nvme_tcp_req_complete(struct nvme_tcp_req *tcp_req, assert(tcp_req->req != NULL); req = tcp_req->req; + TAILQ_REMOVE(&tcp_req->tqpair->outstanding_reqs, tcp_req, link); nvme_complete_request(req->cb_fn, req->cb_arg, req->qpair, req, rsp); nvme_free_request(req); }