nvme: fix atomic write size validation

Don't mix the namespace and controller values, and validate the
per-controller limit when probing the controller.  This avoid spurious
failures for controllers with namespaces that have different namespaces
with different logical block sizes, or report the per-namespace values
only for some namespaces.

It also fixes a missing queue_limits_cancel_update in an error path by
removing that error path.

Fixes: 8695f060a029 ("nvme: all namespaces in a subsystem must adhere to a common atomic write size")
Reported-by: Yi Zhang <yi.zhang@redhat.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: John Garry <john.g.garry@oracle.com>
Tested-by: Yi Zhang <yi.zhang@redhat.com>

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 520fb5f1e2147b1079cea2e3cfe2360040ac7798..3da5ac71a9b07fec0f03e3c58dfd4bf741860294 100644 (file)
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2041,17 +2041,7 @@ static u32 nvme_configure_atomic_write(struct nvme_ns *ns,
                 * no clear language in the specification prohibiting different
                 * values for different controllers in the subsystem.
                 */
-               atomic_bs = (1 + ns->ctrl->awupf) * bs;
-       }
-
-       if (!ns->ctrl->subsys->atomic_bs) {
-               ns->ctrl->subsys->atomic_bs = atomic_bs;
-       } else if (ns->ctrl->subsys->atomic_bs != atomic_bs) {
-               dev_err_ratelimited(ns->ctrl->device,
-                       "%s: Inconsistent Atomic Write Size, Namespace will not be added: Subsystem=%d bytes, Controller/Namespace=%d bytes\n",
-                       ns->disk ? ns->disk->disk_name : "?",
-                       ns->ctrl->subsys->atomic_bs,
-                       atomic_bs);
+               atomic_bs = (1 + ns->ctrl->subsys->awupf) * bs;
        }
 
        lim->atomic_write_hw_max = atomic_bs;
@@ -2386,16 +2376,6 @@ static int nvme_update_ns_info_block(struct nvme_ns *ns,
        if (!nvme_update_disk_info(ns, id, &lim))
                capacity = 0;
 
-       /*
-        * Validate the max atomic write size fits within the subsystem's
-        * atomic write capabilities.
-        */
-       if (lim.atomic_write_hw_max > ns->ctrl->subsys->atomic_bs) {
-               blk_mq_unfreeze_queue(ns->disk->queue, memflags);
-               ret = -ENXIO;
-               goto out;
-       }
-
        nvme_config_discard(ns, &lim);
        if (IS_ENABLED(CONFIG_BLK_DEV_ZONED) &&
            ns->head->ids.csi == NVME_CSI_ZNS)
@@ -3555,7 +3535,18 @@ static int nvme_init_identify(struct nvme_ctrl *ctrl)
                ret = nvme_init_effects(ctrl, id);
                if (ret)
                        goto out_free;
+
+               ctrl->subsys->awupf = le16_to_cpu(id->awupf);
        }
+
+       if (le16_to_cpu(id->awupf) != ctrl->subsys->awupf) {
+               dev_err_ratelimited(ctrl->device,
+                       "inconsistent AWUPF, controller not added (%u/%u).\n",
+                       le16_to_cpu(id->awupf), ctrl->subsys->awupf);
+               ret = -EINVAL;
+               goto out_free;
+       }
+
        memcpy(ctrl->subsys->firmware_rev, id->fr,
               sizeof(ctrl->subsys->firmware_rev));
 
@@ -3651,7 +3642,6 @@ static int nvme_init_identify(struct nvme_ctrl *ctrl)
                dev_pm_qos_expose_latency_tolerance(ctrl->device);
        else if (!ctrl->apst_enabled && prev_apst_enabled)
                dev_pm_qos_hide_latency_tolerance(ctrl->device);
-       ctrl->awupf = le16_to_cpu(id->awupf);
 out_free:
        kfree(id);
        return ret;

diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index a468cdc5b5cb7b97346559b8610e86d8454d29b3..7df2ea21851f5bdb3ac8630bd99686ab97b2d50b 100644 (file)
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -410,7 +410,6 @@ struct nvme_ctrl {
 
        enum nvme_ctrl_type cntrltype;
        enum nvme_dctype dctype;
-       u16 awupf; /* 0's based value. */
 };
 
 static inline enum nvme_ctrl_state nvme_ctrl_state(struct nvme_ctrl *ctrl)
@@ -443,11 +442,11 @@ struct nvme_subsystem {
        u8                      cmic;
        enum nvme_subsys_type   subtype;
        u16                     vendor_id;
+       u16                     awupf; /* 0's based value. */
        struct ida              ns_ida;
 #ifdef CONFIG_NVME_MULTIPATH
        enum nvme_iopolicy      iopolicy;
 #endif
-       u32                     atomic_bs;
 };
 
 /*