From 92f5548a91c8cf843c8073786e835fba2beeaea0 Mon Sep 17 00:00:00 2001 From: Seth Howell Date: Thu, 7 Feb 2019 10:53:33 -0700 Subject: [PATCH] rdma: properly account num_outstanding_data_wr This value was not being decremented when we got SEND completions for write operations because we were using the recv send to indicate when we had completed all writes associated with the request. I also erroneously made the assumption that spdk_nvmf_rdma_request_parse_sgl would properly reset this value to zero for all requests. However, for requests that return SPDK_NVME_DATA_NONE rom spdk_nvmf_rdma_request_get_xfer, this funxtion is skipped and the value is never reset. This can cause a coherency issue on admin queues when we request multiple log files. When the keep_alive request is resent, it can pick up an old rdma_req which reports the wrong number of outstanding_wrs and it will permanently increment the qpairs curr_send_depth. This change decrements num_outstanding_data_wrs on writes, and also resets that value when the request is freed to ensure that this problem doesn't occur again. Change-Id: I5866af97c946a0a58c30507499b43359fb6d0f64 Signed-off-by: Seth Howell Reviewed-on: https://review.gerrithub.io/c/443811 Tested-by: SPDK CI Jenkins Reviewed-by: Ben Walker Reviewed-by: Sasha Kotchubievsky Reviewed-by: Shuhei Matsumoto Reviewed-by: Jim Harris --- lib/nvmf/rdma.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/nvmf/rdma.c b/lib/nvmf/rdma.c index 9b4063abc..3557725f9 100644 --- a/lib/nvmf/rdma.c +++ b/lib/nvmf/rdma.c @@ -1437,6 +1437,7 @@ nvmf_rdma_request_free(struct spdk_nvmf_rdma_request *rdma_req, spdk_nvmf_rdma_request_free_buffers(rdma_req, &rgroup->group, &rtransport->transport); } + rdma_req->num_outstanding_data_wr = 0; rdma_req->req.length = 0; rdma_req->req.iovcnt = 0; rdma_req->req.data = NULL; @@ -2779,6 +2780,8 @@ spdk_nvmf_rdma_poller_poll(struct spdk_nvmf_rdma_transport *rtransport, * the completed state. */ spdk_nvmf_rdma_request_set_state(rdma_req, RDMA_REQUEST_STATE_COMPLETED); rqpair->current_send_depth--; + + assert(rdma_req->num_outstanding_data_wr == 0); spdk_nvmf_rdma_request_process(rtransport, rdma_req); break; case RDMA_WR_TYPE_RECV: @@ -2801,10 +2804,10 @@ spdk_nvmf_rdma_poller_poll(struct spdk_nvmf_rdma_transport *rtransport, rqpair = SPDK_CONTAINEROF(rdma_req->req.qpair, struct spdk_nvmf_rdma_qpair, qpair); SPDK_ERRLOG("data=%p length=%u\n", rdma_req->req.data, rdma_req->req.length); + rdma_req->num_outstanding_data_wr--; if (rdma_req->data.wr.opcode == IBV_WR_RDMA_READ) { assert(rdma_req->num_outstanding_data_wr > 0); rqpair->current_read_depth--; - rdma_req->num_outstanding_data_wr--; if (rdma_req->num_outstanding_data_wr == 0) { spdk_nvmf_rdma_request_set_state(rdma_req, RDMA_REQUEST_STATE_COMPLETED); } @@ -2862,6 +2865,7 @@ spdk_nvmf_rdma_poller_poll(struct spdk_nvmf_rdma_transport *rtransport, count++; + assert(rdma_req->num_outstanding_data_wr == 0); /* Try to process other queued requests */ spdk_nvmf_rdma_qpair_process_pending(rtransport, rqpair, false); break; @@ -2871,6 +2875,7 @@ spdk_nvmf_rdma_poller_poll(struct spdk_nvmf_rdma_transport *rtransport, rdma_req = SPDK_CONTAINEROF(rdma_wr, struct spdk_nvmf_rdma_request, data.rdma_wr); rqpair = SPDK_CONTAINEROF(rdma_req->req.qpair, struct spdk_nvmf_rdma_qpair, qpair); rqpair->current_send_depth--; + rdma_req->num_outstanding_data_wr--; /* Try to process other queued requests */ spdk_nvmf_rdma_qpair_process_pending(rtransport, rqpair, false);