From a50a70ecdf3da2ab1aec62c3f6bbf695e2c302ce Mon Sep 17 00:00:00 2001 From: Konrad Sztyber Date: Fri, 17 Dec 2021 15:04:21 +0100 Subject: [PATCH] nvmf: abort outstanding zcopy reqs in qpair disconnect Zero-copy requests are kept on the outstanding queue for the whole duration of the request - from the initial zcopy_start submission to the completion of zcopy_end. This means, that there's a period in which a request doesn't wait for a completion from the bdev layer, but is still on the oustanding queue (after zcopy_start callback, before zcopy_end submit). If a qpair gets disconnected while a request is in this state, we need to manually force its completion, as otherwise it might hang indefinitely (e.g. waiting for host data). Signed-off-by: Konrad Sztyber Change-Id: I53731b8e363b725efa564ca3c7d89b46f5fb2a24 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10793 Reviewed-by: Jim Harris Reviewed-by: Aleksey Marchuk Reviewed-by: Ben Walker Tested-by: SPDK CI Jenkins Community-CI: Broadcom CI --- lib/nvmf/ctrlr.c | 20 ++++++++++++++++++++ lib/nvmf/nvmf.c | 1 + lib/nvmf/nvmf_internal.h | 8 ++++++++ test/unit/lib/nvmf/fc.c/fc_ut.c | 1 + test/unit/lib/nvmf/nvmf.c/nvmf_ut.c | 1 + 5 files changed, 31 insertions(+) diff --git a/lib/nvmf/ctrlr.c b/lib/nvmf/ctrlr.c index 2cf813431..359ffc032 100644 --- a/lib/nvmf/ctrlr.c +++ b/lib/nvmf/ctrlr.c @@ -2777,6 +2777,26 @@ nvmf_qpair_abort_aer(struct spdk_nvmf_qpair *qpair, uint16_t cid) return false; } +void +nvmf_qpair_abort_pending_zcopy_reqs(struct spdk_nvmf_qpair *qpair) +{ + struct spdk_nvmf_request *req, *tmp; + + TAILQ_FOREACH_SAFE(req, &qpair->outstanding, link, tmp) { + if (req->zcopy_phase == NVMF_ZCOPY_PHASE_EXECUTE) { + /* Zero-copy requests are kept on the outstanding queue from the moment + * zcopy_start is sent until a zcopy_end callback is received. Therefore, + * we can't remove them from the outstanding queue here, but need to rely on + * the transport to do a zcopy_end to release their buffers and, in turn, + * remove them from the queue. + */ + req->rsp->nvme_cpl.status.sct = SPDK_NVME_SCT_GENERIC; + req->rsp->nvme_cpl.status.sc = SPDK_NVME_SC_ABORTED_BY_REQUEST; + nvmf_transport_req_free(req); + } + } +} + static void nvmf_qpair_abort_request(struct spdk_nvmf_qpair *qpair, struct spdk_nvmf_request *req) { diff --git a/lib/nvmf/nvmf.c b/lib/nvmf/nvmf.c index 3626bccff..a3a1d491b 100644 --- a/lib/nvmf/nvmf.c +++ b/lib/nvmf/nvmf.c @@ -1110,6 +1110,7 @@ spdk_nvmf_qpair_disconnect(struct spdk_nvmf_qpair *qpair, nvmf_qpair_disconnect_ SPDK_DTRACE_PROBE2(nvmf_poll_group_drain_qpair, qpair, spdk_thread_get_id(group->thread)); qpair->state_cb = _nvmf_qpair_destroy; qpair->state_cb_arg = qpair_ctx; + nvmf_qpair_abort_pending_zcopy_reqs(qpair); nvmf_qpair_free_aer(qpair); return 0; } diff --git a/lib/nvmf/nvmf_internal.h b/lib/nvmf/nvmf_internal.h index dcc03db4b..2fd89dbc2 100644 --- a/lib/nvmf/nvmf_internal.h +++ b/lib/nvmf/nvmf_internal.h @@ -448,6 +448,14 @@ void nvmf_ctrlr_reservation_notice_log(struct spdk_nvmf_ctrlr *ctrlr, */ void nvmf_ctrlr_abort_aer(struct spdk_nvmf_ctrlr *ctrlr); +/* + * Abort zero-copy requests that already got the buffer (received zcopy_start cb), but haven't + * started zcopy_end. These requests are kept on the outstanding queue, but are not waiting for a + * completion from the bdev layer, so, when a qpair is being disconnected, we need to kick them to + * force their completion. + */ +void nvmf_qpair_abort_pending_zcopy_reqs(struct spdk_nvmf_qpair *qpair); + /* * Free aer simply frees the rdma resources for the aer without informing the host. * This function should be called when deleting a qpair when one wants to make sure diff --git a/test/unit/lib/nvmf/fc.c/fc_ut.c b/test/unit/lib/nvmf/fc.c/fc_ut.c index 5bea41f09..c63667380 100644 --- a/test/unit/lib/nvmf/fc.c/fc_ut.c +++ b/test/unit/lib/nvmf/fc.c/fc_ut.c @@ -98,6 +98,7 @@ DEFINE_STUB(spdk_nvme_transport_id_compare, int, DEFINE_STUB(spdk_bdev_get_name, const char *, (const struct spdk_bdev *bdev), "fc_ut_test"); DEFINE_STUB_V(nvmf_ctrlr_destruct, (struct spdk_nvmf_ctrlr *ctrlr)); DEFINE_STUB_V(nvmf_qpair_free_aer, (struct spdk_nvmf_qpair *qpair)); +DEFINE_STUB_V(nvmf_qpair_abort_pending_zcopy_reqs, (struct spdk_nvmf_qpair *qpair)); DEFINE_STUB(spdk_bdev_get_io_channel, struct spdk_io_channel *, (struct spdk_bdev_desc *desc), NULL); DEFINE_STUB_V(spdk_nvmf_request_exec, (struct spdk_nvmf_request *req)); diff --git a/test/unit/lib/nvmf/nvmf.c/nvmf_ut.c b/test/unit/lib/nvmf/nvmf.c/nvmf_ut.c index f360b67e2..05fa5f0b1 100644 --- a/test/unit/lib/nvmf/nvmf.c/nvmf_ut.c +++ b/test/unit/lib/nvmf/nvmf.c/nvmf_ut.c @@ -43,6 +43,7 @@ DEFINE_STUB_V(nvmf_transport_qpair_fini, (struct spdk_nvmf_qpair *qpair, spdk_nvmf_transport_qpair_fini_cb cb_fn, void *cb_arg)); DEFINE_STUB_V(nvmf_qpair_free_aer, (struct spdk_nvmf_qpair *qpair)); +DEFINE_STUB_V(nvmf_qpair_abort_pending_zcopy_reqs, (struct spdk_nvmf_qpair *qpair)); DEFINE_STUB(nvmf_transport_poll_group_create, struct spdk_nvmf_transport_poll_group *, (struct spdk_nvmf_transport *transport), NULL); DEFINE_STUB(spdk_bdev_get_io_channel, struct spdk_io_channel *, (struct spdk_bdev_desc *desc),