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: 8695f060a0 ("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: Luis Chamberlain <mcgrof@kernel.org>
Reviewed-by: John Garry <john.g.garry@oracle.com>
Tested-by: Yi Zhang <yi.zhang@redhat.com>
This commit is contained in:
Christoph Hellwig 2025-06-11 06:54:56 +02:00
parent b2e607feca
commit f46d273449
2 changed files with 12 additions and 24 deletions

View File

@ -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)
@ -3219,6 +3199,7 @@ static int nvme_init_subsystem(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
memcpy(subsys->model, id->mn, sizeof(subsys->model));
subsys->vendor_id = le16_to_cpu(id->vid);
subsys->cmic = id->cmic;
subsys->awupf = le16_to_cpu(id->awupf);
/* Versions prior to 1.4 don't necessarily report a valid type */
if (id->cntrltype == NVME_CTRL_DISC ||
@ -3556,6 +3537,15 @@ static int nvme_init_identify(struct nvme_ctrl *ctrl)
if (ret)
goto out_free;
}
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 +3641,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;

View File

@ -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;
};
/*