nvme-multipath: fix double initialization of ANA state

nvme_init_identify and thus nvme_mpath_init can be called multiple
times and thus must not overwrite potentially initialized or in-use
fields.  Split out a helper for the basic initialization when the
controller is initialized and make sure the init_identify path does
not blindly change in-use data structures.

Fixes: 0d0b660f214d ("nvme: add ANA support")
Reported-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Keith Busch <kbusch@kernel.org>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Reviewed-by: Hannes Reinecke <hare@suse.de>

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 522c9b2..762125f 100644 (file)
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2901,7 +2901,7 @@ static int nvme_init_identify(struct nvme_ctrl *ctrl)
                ctrl->hmmaxd = le16_to_cpu(id->hmmaxd);
        }
 
-       ret = nvme_mpath_init(ctrl, id);
+       ret = nvme_mpath_init_identify(ctrl, id);
        if (ret < 0)
                goto out_free;
 
@@ -4364,6 +4364,7 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
                min(default_ps_max_latency_us, (unsigned long)S32_MAX));
 
        nvme_fault_inject_init(&ctrl->fault_inject, dev_name(ctrl->device));
+       nvme_mpath_init_ctrl(ctrl);
 
        return 0;
 out_free_name:

diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 0551796..deb1456 100644 (file)
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -781,9 +781,18 @@ void nvme_mpath_remove_disk(struct nvme_ns_head *head)
        put_disk(head->disk);
 }
 
-int nvme_mpath_init(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
+void nvme_mpath_init_ctrl(struct nvme_ctrl *ctrl)
 {
-       int error;
+       mutex_init(&ctrl->ana_lock);
+       timer_setup(&ctrl->anatt_timer, nvme_anatt_timeout, 0);
+       INIT_WORK(&ctrl->ana_work, nvme_ana_work);
+}
+
+int nvme_mpath_init_identify(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
+{
+       size_t max_transfer_size = ctrl->max_hw_sectors << SECTOR_SHIFT;
+       size_t ana_log_size;
+       int error = 0;
 
        /* check if multipath is enabled and we have the capability */
        if (!multipath || !ctrl->subsys ||
@@ -795,37 +804,31 @@ int nvme_mpath_init(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
        ctrl->nanagrpid = le32_to_cpu(id->nanagrpid);
        ctrl->anagrpmax = le32_to_cpu(id->anagrpmax);
 
-       mutex_init(&ctrl->ana_lock);
-       timer_setup(&ctrl->anatt_timer, nvme_anatt_timeout, 0);
-       ctrl->ana_log_size = sizeof(struct nvme_ana_rsp_hdr) +
-               ctrl->nanagrpid * sizeof(struct nvme_ana_group_desc);
-       ctrl->ana_log_size += ctrl->max_namespaces * sizeof(__le32);
-
-       if (ctrl->ana_log_size > ctrl->max_hw_sectors << SECTOR_SHIFT) {
+       ana_log_size = sizeof(struct nvme_ana_rsp_hdr) +
+               ctrl->nanagrpid * sizeof(struct nvme_ana_group_desc) +
+               ctrl->max_namespaces * sizeof(__le32);
+       if (ana_log_size > max_transfer_size) {
                dev_err(ctrl->device,
-                       "ANA log page size (%zd) larger than MDTS (%d).\n",
-                       ctrl->ana_log_size,
-                       ctrl->max_hw_sectors << SECTOR_SHIFT);
+                       "ANA log page size (%zd) larger than MDTS (%zd).\n",
+                       ana_log_size, max_transfer_size);
                dev_err(ctrl->device, "disabling ANA support.\n");
-               return 0;
+               goto out_uninit;
        }
-
-       INIT_WORK(&ctrl->ana_work, nvme_ana_work);
-       kfree(ctrl->ana_log_buf);
-       ctrl->ana_log_buf = kmalloc(ctrl->ana_log_size, GFP_KERNEL);
-       if (!ctrl->ana_log_buf) {
-               error = -ENOMEM;
-               goto out;
+       if (ana_log_size > ctrl->ana_log_size) {
+               nvme_mpath_stop(ctrl);
+               kfree(ctrl->ana_log_buf);
+               ctrl->ana_log_buf = kmalloc(ctrl->ana_log_size, GFP_KERNEL);
+               if (!ctrl->ana_log_buf)
+                       return -ENOMEM;
        }
-
+       ctrl->ana_log_size = ana_log_size;
        error = nvme_read_ana_log(ctrl);
        if (error)
-               goto out_free_ana_log_buf;
+               goto out_uninit;
        return 0;
-out_free_ana_log_buf:
-       kfree(ctrl->ana_log_buf);
-       ctrl->ana_log_buf = NULL;
-out:
+
+out_uninit:
+       nvme_mpath_uninit(ctrl);
        return error;
 }
 

diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 05f31a2..0015860 100644 (file)
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -712,7 +712,8 @@ void nvme_kick_requeue_lists(struct nvme_ctrl *ctrl);
 int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl,struct nvme_ns_head *head);
 void nvme_mpath_add_disk(struct nvme_ns *ns, struct nvme_id_ns *id);
 void nvme_mpath_remove_disk(struct nvme_ns_head *head);
-int nvme_mpath_init(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id);
+int nvme_mpath_init_identify(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id);
+void nvme_mpath_init_ctrl(struct nvme_ctrl *ctrl);
 void nvme_mpath_uninit(struct nvme_ctrl *ctrl);
 void nvme_mpath_stop(struct nvme_ctrl *ctrl);
 bool nvme_mpath_clear_current_path(struct nvme_ns *ns);
@@ -780,7 +781,10 @@ static inline void nvme_mpath_check_last_path(struct nvme_ns *ns)
 static inline void nvme_trace_bio_complete(struct request *req)
 {
 }
-static inline int nvme_mpath_init(struct nvme_ctrl *ctrl,
+static inline void nvme_mpath_init_ctrl(struct nvme_ctrl *ctrl)
+{
+}
+static inline int nvme_mpath_init_identify(struct nvme_ctrl *ctrl,
                struct nvme_id_ctrl *id)
 {
        if (ctrl->subsys->cmic & NVME_CTRL_CMIC_ANA)