From b1deebd18cbcd054369085c3901d7b486fbd1711 Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Mon, 27 Jul 2020 09:11:28 -0700 Subject: [PATCH] nvme: do not abort reqs in multi-process cleanup path When a process cleans up IO qpairs from another crashed process in a multi-process environment, we must not try to abort reqs for that IO qpair. Any reqs will contain callbacks for the crashed process which we must not try to execute in a different process. Fixes issue #1509. Signed-off-by: Jim Harris Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3536 (master) (cherry picked from commit 751e2812bc4365dbf699df0a8e91e01c23d2d6de) Change-Id: I5e58cce7bdb86e3feb4084733815c086901f867e Signed-off-by: Tomasz Zawadzki Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3548 Reviewed-by: Aleksey Marchuk Reviewed-by: Shuhei Matsumoto Reviewed-by: Ben Walker Tested-by: SPDK CI Jenkins --- lib/nvme/nvme_ctrlr.c | 11 ++++++++++- lib/nvme/nvme_transport.c | 9 ++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/lib/nvme/nvme_ctrlr.c b/lib/nvme/nvme_ctrlr.c index 5d8b519e6..ced02e9bb 100644 --- a/lib/nvme/nvme_ctrlr.c +++ b/lib/nvme/nvme_ctrlr.c @@ -563,7 +563,16 @@ spdk_nvme_ctrlr_free_io_qpair(struct spdk_nvme_qpair *qpair) /* Do not retry. */ nvme_qpair_set_state(qpair, NVME_QPAIR_DESTROYING); - nvme_qpair_abort_reqs(qpair, 1); + + /* In the multi-process case, a process may call this function on a foreign + * I/O qpair (i.e. one that this process did not create) when that qpairs process + * exits unexpectedly. In that case, we must not try to abort any reqs associated + * with that qpair, since the callbacks will also be foreign to this process. + */ + if (qpair->active_proc == nvme_ctrlr_get_current_process(ctrlr)) { + nvme_qpair_abort_reqs(qpair, 1); + } + nvme_robust_mutex_lock(&ctrlr->ctrlr_lock); nvme_ctrlr_proc_remove_io_qpair(qpair); diff --git a/lib/nvme/nvme_transport.c b/lib/nvme/nvme_transport.c index 75ff85484..1f47a025b 100644 --- a/lib/nvme/nvme_transport.c +++ b/lib/nvme/nvme_transport.c @@ -278,7 +278,14 @@ nvme_transport_ctrlr_create_io_qpair(struct spdk_nvme_ctrlr *ctrlr, uint16_t qid int nvme_transport_ctrlr_delete_io_qpair(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_qpair *qpair) { - return qpair->transport->ops.ctrlr_delete_io_qpair(ctrlr, qpair); + const struct spdk_nvme_transport *transport = nvme_get_transport(ctrlr->trid.trstring); + + /* Do not rely on qpair->transport. For multi-process cases, a foreign process may delete + * the IO qpair, in which case the transport object would be invalid (each process has their + * own unique transport objects since they contain function pointers). So we look up the + * transport object in the delete_io_qpair case. + */ + return transport->ops.ctrlr_delete_io_qpair(ctrlr, qpair); } int