From 388e31015077152c2205209d654e384d5852d757 Mon Sep 17 00:00:00 2001 From: Seth Howell Date: Wed, 18 Jul 2018 08:47:16 -0700 Subject: [PATCH] nvmf: add free_req function pointer. At times, it may be necessary to free requests without completing them. For example, when freeing a qpair, one needs to free the AER sent from the host before deleting the qpair. It is important not to send a completion for the AER because: 1. According to the spec, this will trigger the host to send another AER 2. No Asynchronous Events have occured, so we should not complete the AER. Change-Id: I92e163f0fed0ee2bc942569a647cb3c1967edec9 Signed-off-by: Seth Howell Reviewed-on: https://review.gerrithub.io/419732 Chandler-Test-Pool: SPDK Automated Test System Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Ben Walker --- lib/nvmf/nvmf_internal.h | 1 + lib/nvmf/rdma.c | 24 ++++++++++++++ lib/nvmf/request.c | 40 +++++++++++++++++------ lib/nvmf/transport.c | 6 ++++ lib/nvmf/transport.h | 8 +++++ test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c | 2 +- test/unit/lib/nvmf/request.c/request_ut.c | 6 ++++ 7 files changed, 76 insertions(+), 11 deletions(-) diff --git a/lib/nvmf/nvmf_internal.h b/lib/nvmf/nvmf_internal.h index 4559cb462..7a25f2604 100644 --- a/lib/nvmf/nvmf_internal.h +++ b/lib/nvmf/nvmf_internal.h @@ -268,6 +268,7 @@ int spdk_nvmf_poll_group_pause_subsystem(struct spdk_nvmf_poll_group *group, int spdk_nvmf_poll_group_resume_subsystem(struct spdk_nvmf_poll_group *group, struct spdk_nvmf_subsystem *subsystem); void spdk_nvmf_request_exec(struct spdk_nvmf_request *req); +int spdk_nvmf_request_free(struct spdk_nvmf_request *req); int spdk_nvmf_request_complete(struct spdk_nvmf_request *req); void spdk_nvmf_get_discovery_log_page(struct spdk_nvmf_tgt *tgt, diff --git a/lib/nvmf/rdma.c b/lib/nvmf/rdma.c index cd45471b0..4a534ed31 100644 --- a/lib/nvmf/rdma.c +++ b/lib/nvmf/rdma.c @@ -2390,6 +2390,29 @@ spdk_nvmf_rdma_poll_group_remove(struct spdk_nvmf_transport_poll_group *group, return 0; } +static int +spdk_nvmf_rdma_request_free(struct spdk_nvmf_request *req) +{ + struct spdk_nvmf_rdma_request *rdma_req = SPDK_CONTAINEROF(req, struct spdk_nvmf_rdma_request, req); + struct spdk_nvmf_rdma_transport *rtransport = SPDK_CONTAINEROF(req->qpair->transport, + struct spdk_nvmf_rdma_transport, transport); + + if (rdma_req->data_from_pool) { + /* Put the buffer/s back in the pool */ + for (uint32_t i = 0; i < rdma_req->req.iovcnt; i++) { + spdk_mempool_put(rtransport->data_buf_pool, rdma_req->data.buffers[i]); + rdma_req->req.iov[i].iov_base = NULL; + rdma_req->data.buffers[i] = NULL; + } + rdma_req->data_from_pool = false; + } + rdma_req->req.length = 0; + rdma_req->req.iovcnt = 0; + rdma_req->req.data = NULL; + spdk_nvmf_rdma_request_set_state(rdma_req, RDMA_REQUEST_STATE_FREE); + return 0; +} + static int spdk_nvmf_rdma_request_complete(struct spdk_nvmf_request *req) { @@ -2606,6 +2629,7 @@ const struct spdk_nvmf_transport_ops spdk_nvmf_transport_rdma = { .poll_group_remove = spdk_nvmf_rdma_poll_group_remove, .poll_group_poll = spdk_nvmf_rdma_poll_group_poll, + .req_free = spdk_nvmf_rdma_request_free, .req_complete = spdk_nvmf_rdma_request_complete, .qpair_fini = spdk_nvmf_rdma_close_qpair, diff --git a/lib/nvmf/request.c b/lib/nvmf/request.c index 734339424..88b6b9a95 100644 --- a/lib/nvmf/request.c +++ b/lib/nvmf/request.c @@ -45,6 +45,35 @@ #include "spdk_internal/assert.h" #include "spdk_internal/log.h" +static void +spdk_nvmf_qpair_request_cleanup(struct spdk_nvmf_qpair *qpair) +{ + if (qpair->state == SPDK_NVMF_QPAIR_DEACTIVATING) { + assert(qpair->state_cb != NULL); + + if (TAILQ_EMPTY(&qpair->outstanding)) { + qpair->state_cb(qpair->state_cb_arg, 0); + } + } else { + assert(qpair->state == SPDK_NVMF_QPAIR_ACTIVE); + } +} + +int +spdk_nvmf_request_free(struct spdk_nvmf_request *req) +{ + struct spdk_nvmf_qpair *qpair = req->qpair; + + TAILQ_REMOVE(&qpair->outstanding, req, link); + if (spdk_nvmf_transport_req_free(req)) { + SPDK_ERRLOG("Unable to free transport level request resources.\n"); + } + + spdk_nvmf_qpair_request_cleanup(qpair); + + return 0; +} + int spdk_nvmf_request_complete(struct spdk_nvmf_request *req) { @@ -67,16 +96,7 @@ spdk_nvmf_request_complete(struct spdk_nvmf_request *req) SPDK_ERRLOG("Transport request completion error!\n"); } - if (qpair->state == SPDK_NVMF_QPAIR_DEACTIVATING) { - assert(qpair->state_cb != NULL); - - if (TAILQ_EMPTY(&qpair->outstanding)) { - - qpair->state_cb(qpair->state_cb_arg, 0); - } - } else { - assert(qpair->state == SPDK_NVMF_QPAIR_ACTIVE); - } + spdk_nvmf_qpair_request_cleanup(qpair); return 0; } diff --git a/lib/nvmf/transport.c b/lib/nvmf/transport.c index 9e24f9fb7..6e9e5ae45 100644 --- a/lib/nvmf/transport.c +++ b/lib/nvmf/transport.c @@ -160,6 +160,12 @@ spdk_nvmf_transport_poll_group_poll(struct spdk_nvmf_transport_poll_group *group return group->transport->ops->poll_group_poll(group); } +int +spdk_nvmf_transport_req_free(struct spdk_nvmf_request *req) +{ + return req->qpair->transport->ops->req_free(req); +} + int spdk_nvmf_transport_req_complete(struct spdk_nvmf_request *req) { diff --git a/lib/nvmf/transport.h b/lib/nvmf/transport.h index f2e5158e1..beedba386 100644 --- a/lib/nvmf/transport.h +++ b/lib/nvmf/transport.h @@ -114,6 +114,12 @@ struct spdk_nvmf_transport_ops { */ int (*poll_group_poll)(struct spdk_nvmf_transport_poll_group *group); + /* + * Free the request without sending a response + * to the originator. Release memory tied to this request. + */ + int (*req_free)(struct spdk_nvmf_request *req); + /* * Signal request completion, which sends a response * to the originator. @@ -160,6 +166,8 @@ int spdk_nvmf_transport_poll_group_remove(struct spdk_nvmf_transport_poll_group int spdk_nvmf_transport_poll_group_poll(struct spdk_nvmf_transport_poll_group *group); +int spdk_nvmf_transport_req_free(struct spdk_nvmf_request *req); + int spdk_nvmf_transport_req_complete(struct spdk_nvmf_request *req); void spdk_nvmf_transport_qpair_fini(struct spdk_nvmf_qpair *qpair); diff --git a/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c b/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c index 2ba8ddb84..275b8811f 100644 --- a/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c +++ b/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c @@ -128,7 +128,7 @@ DEFINE_STUB(spdk_nvmf_request_complete, (struct spdk_nvmf_request *req), -1); -DEFINE_STUB(spdk_nvmf_request_abort, +DEFINE_STUB(spdk_nvmf_request_free, int, (struct spdk_nvmf_request *req), -1); diff --git a/test/unit/lib/nvmf/request.c/request_ut.c b/test/unit/lib/nvmf/request.c/request_ut.c index 14a9c34e0..c37e917b0 100644 --- a/test/unit/lib/nvmf/request.c/request_ut.c +++ b/test/unit/lib/nvmf/request.c/request_ut.c @@ -44,6 +44,12 @@ void spdk_trace_record(uint16_t tpoint_id, uint16_t poller_id, uint32_t size, { } +int +spdk_nvmf_transport_req_free(struct spdk_nvmf_request *req) +{ + return 0; +} + int spdk_nvmf_transport_req_complete(struct spdk_nvmf_request *req) {