Commit f46d2734 authored by Christoph Hellwig's avatar Christoph Hellwig
Browse files

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: 8695f060 ("nvme: all namespaces in a subsystem must adhere to a common atomic write size")
Reported-by: default avatarYi Zhang <yi.zhang@redhat.com>
Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarLuis Chamberlain <mcgrof@kernel.org>
Reviewed-by: default avatarJohn Garry <john.g.garry@oracle.com>
Tested-by: default avatarYi Zhang <yi.zhang@redhat.com>
parent b2e607fe

drivers/nvme/host/core.c

+11 −22
Original line number Diff line number Diff line
@@ -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;

drivers/nvme/host/nvme.h

+1 −2
Original line number Diff line number Diff line
@@ -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;

};



/*