From f38d2f6a8ef7413a7660eda1831c346587482304 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Tue, 14 Feb 2023 14:22:56 +0900 Subject: [PATCH] bdev/nvme: Delete alternate paths and an active path separately If alternate paths and an active path are treated separately in _bdev_nvme_delete(), it will be much easier to protect a path list from delete operation by mutex. Signed-off-by: Shuhei Matsumoto Change-Id: Ie3bed095fd92b80c0487ef7b136953ad03a174eb Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/16820 Reviewed-by: Jim Harris Reviewed-by: Aleksey Marchuk Tested-by: SPDK CI Jenkins --- module/bdev/nvme/bdev_nvme.c | 43 ++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/module/bdev/nvme/bdev_nvme.c b/module/bdev/nvme/bdev_nvme.c index 151f45c1d..3a0f37bf6 100644 --- a/module/bdev/nvme/bdev_nvme.c +++ b/module/bdev/nvme/bdev_nvme.c @@ -5365,30 +5365,35 @@ _bdev_nvme_delete(struct nvme_ctrlr *nvme_ctrlr, const struct nvme_path_id *path int rc = -ENXIO; TAILQ_FOREACH_REVERSE_SAFE(p, &nvme_ctrlr->trids, nvme_paths, link, t) { + if (p == TAILQ_FIRST(&nvme_ctrlr->trids)) { + break; + } + if (!nvme_path_should_delete(p, path_id)) { continue; } - /* If we made it here, then this path is a match! Now we need to remove it. */ - if (p == nvme_ctrlr->active_path_id) { - /* This is the active path in use right now. The active path is always the first in the list. */ - if (!TAILQ_NEXT(p, link)) { - /* The current path is the only path. */ - rc = bdev_nvme_delete_ctrlr(nvme_ctrlr, false); - } else { - /* There is an alternative path. */ - rc = bdev_nvme_failover(nvme_ctrlr, true); - } - } else { - /* We are not using the specified path. */ - TAILQ_REMOVE(&nvme_ctrlr->trids, p, link); - free(p); - rc = 0; - } + /* We are not using the specified path. */ + TAILQ_REMOVE(&nvme_ctrlr->trids, p, link); + free(p); + rc = 0; + } - if (rc < 0 && rc != -ENXIO) { - return rc; - } + if (p == NULL || !nvme_path_should_delete(p, path_id)) { + return rc; + } + + /* If we made it here, then this path is a match! Now we need to remove it. */ + + /* This is the active path in use right now. The active path is always the first in the list. */ + assert(p == nvme_ctrlr->active_path_id); + + if (!TAILQ_NEXT(p, link)) { + /* The current path is the only path. */ + rc = bdev_nvme_delete_ctrlr(nvme_ctrlr, false); + } else { + /* There is an alternative path. */ + rc = bdev_nvme_failover(nvme_ctrlr, true); } return rc;