From 4300c62167f7ea4597dda4587b4c6ecb6df095b7 Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Fri, 19 Aug 2022 14:19:24 +0000 Subject: [PATCH] nvme: add spdk_nvme_ctrlr_disable_read_changed_ns_list_log_page() Commit a119799b ("test/nvme/aer: remove duplicated changed NS list log") changed the nvme driver to read the CHANGED_NS_LIST log page before calling the application's AER callback (previously it would read it after). Commit b801af090 ("nvme: add disable_read_changed_ns_list_log_page") added a new ctrlr_opts member to allow the application to tell the driver to not read this log page, and will read the log page itself instead to clear the AEN. But we cannot add this option to the 22.01 LTS branch since it breaks the ABI. So adding this API here, which can then be backported manually to the 22.01 branch for LTS users that require it. Restoring the old behavior is not correct for applications that want to consume the CHANGED_NS_LIST log page contents itself to know which namespaces have changed. Even if the driver reads the log page after the application, that read could happen during a small window between when a namespace change event has occurred and the AEN has been sent to the host. The only safe way for the application to consume ChANGED_NS_LIST log page contents itself is to make sure the driver never issues such a log page request itself. Fixes issue #2647. Signed-off-by: Jim Harris Change-Id: Iaeffe23dc7817c0c94441a36ed4d6f64a1f15a4e Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/14134 Reviewed-by: Michael Haeuptle Reviewed-by: Tomasz Zawadzki Reviewed-by: Dong Yi Reviewed-by: Shuhei Matsumoto Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins --- CHANGELOG.md | 5 +++++ include/spdk/nvme.h | 16 ++++++++++++++++ lib/nvme/nvme_ctrlr.c | 6 ++++++ lib/nvme/spdk_nvme.map | 1 + 4 files changed, 28 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d4fa0179..7d3073262 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,11 @@ fabric transport. SPDK_NVME_TRANSPORT_CUSTOM was intended to be non-fabric custo Added a new function `spdk_nvme_ns_cmd_verify` to submit a Verify Command to a Namespace. +Added `spdk_nvme_ctrlr_disable_read_changed_ns_list_log_page` to allow an application to +tell the driver to not read the CHANGED_NS_LIST log page in response to a NS_ATTR_CHANGED +AEN. When called the application is required to read this log page instead to clear the +AEN. + ## v22.05 ### sock diff --git a/include/spdk/nvme.h b/include/spdk/nvme.h index 4fae63241..27d00e118 100644 --- a/include/spdk/nvme.h +++ b/include/spdk/nvme.h @@ -1397,6 +1397,22 @@ void spdk_nvme_ctrlr_register_aer_callback(struct spdk_nvme_ctrlr *ctrlr, spdk_nvme_aer_cb aer_cb_fn, void *aer_cb_arg); +/** + * Disable reading the CHANGED_NS_LIST log page for the specified controller. + * + * Applications that register an AER callback may wish to read the CHANGED_NS_LIST + * log page itself, rather than relying on the driver to do it. Calling this + * function will ensure that the driver does not read this log page if the + * controller returns a NS_ATTR_CHANGED AEN. + * + * Reading of this log page can alternatively be disabled by setting the + * disable_read_changed_ns_list_log_page flag in the spdk_nvme_ctrlr_opts + * when attaching the controller. + * + * \param ctrlr NVMe controller on which to disable the log page read. + */ +void spdk_nvme_ctrlr_disable_read_changed_ns_list_log_page(struct spdk_nvme_ctrlr *ctrlr); + /** * Opaque handle to a queue pair. * diff --git a/lib/nvme/nvme_ctrlr.c b/lib/nvme/nvme_ctrlr.c index f9f0ca63b..1cb242f35 100644 --- a/lib/nvme/nvme_ctrlr.c +++ b/lib/nvme/nvme_ctrlr.c @@ -4522,6 +4522,12 @@ spdk_nvme_ctrlr_register_aer_callback(struct spdk_nvme_ctrlr *ctrlr, nvme_robust_mutex_unlock(&ctrlr->ctrlr_lock); } +void +spdk_nvme_ctrlr_disable_read_changed_ns_list_log_page(struct spdk_nvme_ctrlr *ctrlr) +{ + ctrlr->opts.disable_read_changed_ns_list_log_page = true; +} + void spdk_nvme_ctrlr_register_timeout_callback(struct spdk_nvme_ctrlr *ctrlr, uint64_t timeout_io_us, uint64_t timeout_admin_us, diff --git a/lib/nvme/spdk_nvme.map b/lib/nvme/spdk_nvme.map index b69a2967b..c4bc133bb 100644 --- a/lib/nvme/spdk_nvme.map +++ b/lib/nvme/spdk_nvme.map @@ -63,6 +63,7 @@ spdk_nvme_ctrlr_is_log_page_supported; spdk_nvme_ctrlr_is_feature_supported; spdk_nvme_ctrlr_register_aer_callback; + spdk_nvme_ctrlr_disable_read_changed_ns_list_log_page; spdk_nvme_ctrlr_register_timeout_callback; spdk_nvme_ctrlr_get_default_io_qpair_opts; spdk_nvme_ctrlr_alloc_io_qpair;