Re: [PATCHv2 5/5] nvme: support for zoned namespaces

From: Klaus Jensen <its@irrelevant.dk>
To: Keith Busch <kbusch@kernel.org>
Cc: linux-nvme@lists.infradead.org, linux-block@vger.kernel.org,
	hch@lst.de, sagi@grimberg.me, axboe@kernel.dk,
	"Niklas Cassel" <niklas.cassel@wdc.com>,
	"Damien Le Moal" <damien.lemoal@wdc.com>,
	"Ajay Joshi" <ajay.joshi@wdc.com>,
	"Keith Busch" <keith.busch@wdc.com>,
	"Martin K . Petersen" <martin.petersen@oracle.com>,
	"Dmitry Fomichev" <dmitry.fomichev@wdc.com>,
	"Aravind Ramesh" <aravind.ramesh@wdc.com>,
	"Hans Holmberg" <hans.holmberg@wdc.com>,
	"Matias Bjørling" <matias.bjorling@wdc.com>
Subject: Re: [PATCHv2 5/5] nvme: support for zoned namespaces
Date: Thu, 18 Jun 2020 21:35:56 +0200	[thread overview]
Message-ID: <20200618193556.3s2gbkjsfotomot7@apples.localdomain> (raw)
In-Reply-To: <20200618145354.1139350-6-kbusch@kernel.org>

On Jun 18 07:53, Keith Busch wrote:
> diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
> new file mode 100644
> index 000000000000..d57fbbfe15e8
> --- /dev/null
> +++ b/drivers/nvme/host/zns.c
> @@ -0,0 +1,238 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 Western Digital Corporation or its affiliates.
> + */
> +
> +#include <linux/blkdev.h>
> +#include <linux/vmalloc.h>
> +#include "nvme.h"
> +
> +static int nvme_set_max_append(struct nvme_ctrl *ctrl)
> +{
> +	struct nvme_command c = { };
> +	struct nvme_id_ctrl_zns *id;
> +	int status;
> +
> +	id = kzalloc(sizeof(*id), GFP_KERNEL);
> +	if (!id)
> +		return -ENOMEM;
> +
> +	c.identify.opcode = nvme_admin_identify;
> +	c.identify.cns = NVME_ID_CNS_CS_CTRL;
> +	c.identify.csi = NVME_CSI_ZNS;
> +
> +	status = nvme_submit_sync_cmd(ctrl->admin_q, &c, id, sizeof(*id));
> +	if (status) {
> +		kfree(id);
> +		return status;
> +	}
> +
> +	ctrl->max_zone_append = 1 << (id->zamds + 3);
> +	kfree(id);
> +	return 0;
> +}
> +
> +int nvme_update_zone_info(struct gendisk *disk, struct nvme_ns *ns,
> +			  unsigned lbaf)
> +{
> +	struct nvme_effects_log *log = ns->head->effects;
> +	struct request_queue *q = disk->queue;
> +	struct nvme_command c = { };
> +	struct nvme_id_ns_zns *id;
> +	int status;
> +
> +	/* Driver requires zone append support */
> +	if (!(log->iocs[nvme_cmd_zone_append] & NVME_CMD_EFFECTS_CSUPP))
> +		return -ENODEV;
> +
> +	/* Lazily query controller append limit for the first zoned namespace */
> +	if (!ns->ctrl->max_zone_append) {
> +		status = nvme_set_max_append(ns->ctrl);
> +		if (status)
> +			return status;
> +	}
> +
> +	id = kzalloc(sizeof(*id), GFP_KERNEL);
> +	if (!id)
> +		return -ENOMEM;
> +
> +	c.identify.opcode = nvme_admin_identify;
> +	c.identify.nsid = cpu_to_le32(ns->head->ns_id);
> +	c.identify.cns = NVME_ID_CNS_CS_NS;
> +	c.identify.csi = NVME_CSI_ZNS;
> +
> +	status = nvme_submit_sync_cmd(ns->ctrl->admin_q, &c, id, sizeof(*id));
> +	if (status)
> +		goto free_data;
> +
> +	/*
> +	 * We currently do not handle devices requiring any of the zoned
> +	 * operation characteristics.
> +	 */
> +	if (id->zoc) {
> +		status = -EINVAL;
> +		goto free_data;
> +	}

A dev_err() here would be nice. I had to dig a bit to track down why my
emulated device didn't show up ;)