From ceb07eb8f4a13c72e06c2279ffc0cba70e8aa2a5 Mon Sep 17 00:00:00 2001 From: Ziye Yang Date: Thu, 25 Jun 2020 17:56:32 +0800 Subject: [PATCH] nvme/tcp: Fix send_cb and recv pdu function contention when there is R2T. When using uring socket, we see following assert nvme_tcp.c:1018: nvme_tcp_capsule_resp_hdr_handle: Assertion `tcp_req->state == NVME_TCP_REQ_ACTIVE' failed. Detailed info is in https://ci.spdk.io/results/autotest-per-patch/builds/19205/archive/nvmf-tcp-vg-autotest/build.log We face this issue, because there is also code execution ordering between "sending callback function" and "pdu receving function". We did not find it in physical machine testing, but finding it in vagrant machine in CI. Signed-off-by: Ziye Yang Change-Id: I5eb241d564c0fc42ce0601b7c85999a2550f0de3 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3046 Tested-by: SPDK CI Jenkins Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Reviewed-by: Ben Walker Reviewed-by: Shuhei Matsumoto --- lib/nvme/nvme_tcp.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/nvme/nvme_tcp.c b/lib/nvme/nvme_tcp.c index 574cd1aa5..5a9ad1245 100644 --- a/lib/nvme/nvme_tcp.c +++ b/lib/nvme/nvme_tcp.c @@ -505,6 +505,7 @@ static inline void nvme_tcp_req_put_safe(struct nvme_tcp_req *tcp_req) { if (tcp_req->ordering.send_ack && tcp_req->ordering.data_recv) { + assert(tcp_req->state == NVME_TCP_REQ_ACTIVE); assert(tcp_req->tqpair != NULL); nvme_tcp_req_put(tcp_req->tqpair, tcp_req); } @@ -1015,7 +1016,6 @@ nvme_tcp_capsule_resp_hdr_handle(struct nvme_tcp_qpair *tqpair, struct nvme_tcp_ } assert(tcp_req->req != NULL); - assert(tcp_req->state == NVME_TCP_REQ_ACTIVE); nvme_tcp_req_complete(tcp_req->req, &cpl); (*reaped)++; @@ -1122,12 +1122,15 @@ nvme_tcp_qpair_h2c_data_send_complete(void *cb_arg) assert(tcp_req != NULL); + tcp_req->ordering.send_ack = 1; if (tcp_req->r2tl_remain) { nvme_tcp_send_h2c_data(tcp_req); } else { assert(tcp_req->active_r2ts > 0); tcp_req->active_r2ts--; tcp_req->state = NVME_TCP_REQ_ACTIVE; + /* Need also call this function to free the resource */ + nvme_tcp_req_put_safe(tcp_req); } } @@ -1139,6 +1142,8 @@ nvme_tcp_send_h2c_data(struct nvme_tcp_req *tcp_req) struct spdk_nvme_tcp_h2c_data_hdr *h2c_data; uint32_t plen, pdo, alignment; + /* Reinit the send_ack */ + tcp_req->ordering.send_ack = 0; rsp_pdu = tcp_req->send_pdu; memset(rsp_pdu, 0, sizeof(*rsp_pdu)); h2c_data = &rsp_pdu->hdr.h2c_data;