From 72925e3db811f79859ac06ac82fd20046b608f41 Mon Sep 17 00:00:00 2001 From: Konrad Sztyber Date: Tue, 12 Apr 2022 12:18:43 +0200 Subject: [PATCH] nvmf/tcp: delay completion for zcopy reqs w/ in-progress writes When a qpair is disconnected, any outstanding zero-copy requests are freed to release their buffers before the qpair gets destroyed. However, if there is a PDU being sent to the host as part of this request (e.g. C2HData/R2T), we need to wait until that write is done before freeing the request to avoid freeing it twice. Fixes #2445 Signed-off-by: Konrad Sztyber Change-Id: I2a6e82f26a4f011dfd18c55c821e9039de7e584a Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/12255 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Reviewed-by: Ben Walker Reviewed-by: Aleksey Marchuk Tested-by: SPDK CI Jenkins --- lib/nvmf/tcp.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/lib/nvmf/tcp.c b/lib/nvmf/tcp.c index 05fe50e64..20f409f36 100644 --- a/lib/nvmf/tcp.c +++ b/lib/nvmf/tcp.c @@ -902,6 +902,12 @@ _req_pdu_write_done(void *req, int err) assert(tcp_req->pdu_in_use); tcp_req->pdu_in_use = false; + /* If the request is in a completed state, we're waiting for write completion to free it */ + if (spdk_unlikely(tcp_req->state == TCP_REQUEST_STATE_COMPLETED)) { + nvmf_tcp_request_free(tcp_req); + return; + } + if (spdk_unlikely(err != 0)) { nvmf_tcp_qpair_disconnect(tqpair); return; @@ -2956,6 +2962,18 @@ nvmf_tcp_req_process(struct spdk_nvmf_tcp_transport *ttransport, case TCP_REQUEST_STATE_COMPLETED: spdk_trace_record(TRACE_TCP_REQUEST_STATE_COMPLETED, tqpair->qpair.qid, 0, (uintptr_t)tcp_req, tqpair); + /* If there's an outstanding PDU sent to the host, the request is completed + * due to the qpair being disconnected. We must delay the completion until + * that write is done to avoid freeing the request twice. */ + if (spdk_unlikely(tcp_req->pdu_in_use)) { + SPDK_DEBUGLOG(nvmf_tcp, "Delaying completion due to outstanding " + "write on req=%p\n", tcp_req); + /* This can only happen for zcopy requests */ + assert(spdk_nvmf_request_using_zcopy(&tcp_req->req)); + assert(tqpair->qpair.state != SPDK_NVMF_QPAIR_ACTIVE); + break; + } + if (tcp_req->req.data_from_pool) { spdk_nvmf_request_free_buffers(&tcp_req->req, group, transport); } else if (spdk_unlikely(tcp_req->has_in_capsule_data &&