From 37ad7fd3b8e0c877669e164e756ee0ee684779ff 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 (master) Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/447462 Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Ben Walker --- 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 9ffbcb9b3..54a55b153 100644 --- a/lib/nvmf/rdma.c +++ b/lib/nvmf/rdma.c @@ -1468,6 +1468,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; @@ -2821,6 +2822,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: @@ -2843,10 +2846,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); } @@ -2904,6 +2907,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; @@ -2913,6 +2917,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);