> -----Original Message-----
> From: Klaus Jensen <its@xxxxxxxxxxxxx>
> Sent: Thursday, 18 June 2020 21.36
> To: Keith Busch <kbusch@xxxxxxxxxx>
> Cc: linux-nvme@xxxxxxxxxxxxxxxxxxx; linux-block@xxxxxxxxxxxxxxx; hch@xxxxxx;
> sagi@xxxxxxxxxxx; axboe@xxxxxxxxx; Niklas Cassel <Niklas.Cassel@xxxxxxx>;
> Damien Le Moal <Damien.LeMoal@xxxxxxx>; Ajay Joshi
> <Ajay.Joshi@xxxxxxx>; Keith Busch <Keith.Busch@xxxxxxx>; Martin K .
> Petersen <martin.petersen@xxxxxxxxxx>; Dmitry Fomichev
> <Dmitry.Fomichev@xxxxxxx>; Aravind Ramesh <Aravind.Ramesh@xxxxxxx>;
> Hans Holmberg <Hans.Holmberg@xxxxxxx>; Matias Bjorling
> <Matias.Bjorling@xxxxxxx>
> Subject: Re: [PATCHv2 5/5] nvme: support for zoned namespaces
>
> 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 ;)
I've stumpled upon this a couple of times where namespaces did not show up for some reason. Instead of adding it here, would it make sense to write an error higher up the chain? For example around the nvme_revalidate_disk function.
