From 963cb0038ece89463cd28e1b7801b5d93c97693d Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Wed, 30 Mar 2022 06:02:38 +0900 Subject: [PATCH] bdev/nvme: disconnected_qpair_cb gets NOTICELOG only when disconnection was unexpected After the change that the NVMe bdev module disconnects qpair asynchronously, disconnected_qpair_cb() got NOTICELOG always when a qpair was disconnected and freed. This was very noisy. We have three cases that disconnected_qpair_cb() is called now, 1) qpair was destroyed in a full ctrlr reset sequence, 2) the upper layer closed I/O channel, and 3) qpair detected error, and was disconnected and freed. Get NOTICELOG for 3) but get DEBUGLOG for 1) and 2) with some rewording. Additionally, to improve readability, change if-else ordering. Signed-off-by: Shuhei Matsumoto Change-Id: Ib63bcfd4b72a82a13d3cda208c71cdb40a42fd6b Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/12085 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Ben Walker Reviewed-by: Jim Harris Reviewed-by: Aleksey Marchuk --- module/bdev/nvme/bdev_nvme.c | 50 +++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/module/bdev/nvme/bdev_nvme.c b/module/bdev/nvme/bdev_nvme.c index 430b00fd6..7b70c8dda 100644 --- a/module/bdev/nvme/bdev_nvme.c +++ b/module/bdev/nvme/bdev_nvme.c @@ -1147,36 +1147,38 @@ bdev_nvme_disconnected_qpair_cb(struct spdk_nvme_qpair *qpair, void *poll_group_ struct nvme_qpair *nvme_qpair; struct nvme_ctrlr_channel *ctrlr_ch; - SPDK_NOTICELOG("qpair %p is disconnected, free the qpair and reset controller.\n", qpair); - - /* - * Free the I/O qpair and reset the nvme_ctrlr. - */ nvme_qpair = nvme_poll_group_get_qpair(group, qpair); - if (nvme_qpair != NULL) { - if (nvme_qpair->qpair != NULL) { - spdk_nvme_ctrlr_free_io_qpair(nvme_qpair->qpair); - nvme_qpair->qpair = NULL; - } + if (nvme_qpair == NULL) { + return; + } - _bdev_nvme_clear_io_path_cache(nvme_qpair); + if (nvme_qpair->qpair != NULL) { + spdk_nvme_ctrlr_free_io_qpair(nvme_qpair->qpair); + nvme_qpair->qpair = NULL; + } - ctrlr_ch = nvme_qpair->ctrlr_ch; + _bdev_nvme_clear_io_path_cache(nvme_qpair); - if (ctrlr_ch != NULL) { - if (ctrlr_ch->reset_iter != NULL) { - /* If we are already in a full reset sequence, we do not have - * to restart it. Just move to the next ctrlr_channel. - */ - spdk_for_each_channel_continue(ctrlr_ch->reset_iter, 0); - ctrlr_ch->reset_iter = NULL; - } else { - bdev_nvme_reset(nvme_qpair->ctrlr); - } + ctrlr_ch = nvme_qpair->ctrlr_ch; + + if (ctrlr_ch != NULL) { + if (ctrlr_ch->reset_iter != NULL) { + /* If we are already in a full reset sequence, we do not have + * to restart it. Just move to the next ctrlr_channel. + */ + SPDK_DEBUGLOG(bdev_nvme, "qpair %p was disconnected and freed in a reset ctrlr sequence.\n", + qpair); + spdk_for_each_channel_continue(ctrlr_ch->reset_iter, 0); + ctrlr_ch->reset_iter = NULL; } else { - /* In this case, ctrlr_channel is already deleted. */ - nvme_qpair_delete(nvme_qpair); + /* qpair was disconnected unexpectedly. Reset controller for recovery. */ + SPDK_NOTICELOG("qpair %p was disconnected and freed. reset controller.\n", qpair); + bdev_nvme_reset(nvme_qpair->ctrlr); } + } else { + /* In this case, ctrlr_channel is already deleted. */ + SPDK_DEBUGLOG(bdev_nvme, "qpair %p was disconnected and freed. delete nvme_qpair.\n", qpair); + nvme_qpair_delete(nvme_qpair); } }