From: Sagi Grimberg <sagi@grimberg.me> To: Keith Busch <kbusch@kernel.org> Cc: "Niklas Cassel" <Niklas.Cassel@wdc.com>, "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>, "hch@lst.de" <hch@lst.de>, "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>, "axboe@kernel.dk" <axboe@kernel.dk>, "Javier González" <javier.gonz@samsung.com>, "Martin K . Petersen" <martin.petersen@oracle.com>, "Johannes Thumshirn" <Johannes.Thumshirn@wdc.com>, "Matias Bjorling" <Matias.Bjorling@wdc.com>, "Daniel Wagner" <dwagner@suse.de> Subject: Re: [PATCHv3 3/5] nvme: implement I/O Command Sets Command Set support Date: Wed, 24 Jun 2020 11:33:25 -0700 [thread overview] Message-ID: <76966a48-0588-3f3c-0ec1-842cd2ac413d@grimberg.me> (raw) In-Reply-To: <1ddbf614-f5a7-a265-b1a2-7c5ed0922f18@grimberg.me> >> If the controller does not support the CNS value, it may return Invalid >> Field with DNR set. That error currently gets propogated back to >> nvme_init_ns_head(), which then abandons the namespace. Just as the code >> coments say, we had been historically been clearing such errors because >> we have other ways to identify the namespace, but now we're not clearing >> that error. > > I don't understand what you are saying Keith. > > My comment was for this block: > -- > if (!status && nvme_multi_css(ctrl) && !csi_seen) { > dev_warn(ctrl->device, "Command set not reported for nsid:%d\n", > nsid); > status = -EINVAL; > } > -- > > I was saying that !status doesn't necessarily mean success, but it can > also mean that its an retry-able error status (due to transport or > controller). If we see a retry-able error we should still clear the > status so we don't abandon the namespace. > > This for example would achieve that: Sorry, meant this: -- diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index ba512c45b40f..3187cf768d08 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -1130,8 +1130,14 @@ static int nvme_identify_ns_descs(struct nvme_ctrl *ctrl, unsigned nsid, * Don't treat an error as fatal, as we potentially already * have a NGUID or EUI-64. */ - if (status > 0 && !(status & NVME_SC_DNR)) + if (status > 0 && !(status & NVME_SC_DNR)) { status = 0; + } else if (status == 0 && nvme_multi_css(ctrl) && !csi_seen) { + dev_warn(ctrl->device, + "Command set not reported for nsid:%d\n", + nsid); + status = -EINVAL; + } goto free_data; } --
WARNING: multiple messages have this Message-ID (diff)
From: Sagi Grimberg <sagi@grimberg.me> To: Keith Busch <kbusch@kernel.org> Cc: "axboe@kernel.dk" <axboe@kernel.dk>, "Niklas Cassel" <Niklas.Cassel@wdc.com>, "Daniel Wagner" <dwagner@suse.de>, "Martin K . Petersen" <martin.petersen@oracle.com>, "Johannes Thumshirn" <Johannes.Thumshirn@wdc.com>, "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>, "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>, "Javier González" <javier.gonz@samsung.com>, "hch@lst.de" <hch@lst.de>, "Matias Bjorling" <Matias.Bjorling@wdc.com> Subject: Re: [PATCHv3 3/5] nvme: implement I/O Command Sets Command Set support Date: Wed, 24 Jun 2020 11:33:25 -0700 [thread overview] Message-ID: <76966a48-0588-3f3c-0ec1-842cd2ac413d@grimberg.me> (raw) In-Reply-To: <1ddbf614-f5a7-a265-b1a2-7c5ed0922f18@grimberg.me> >> If the controller does not support the CNS value, it may return Invalid >> Field with DNR set. That error currently gets propogated back to >> nvme_init_ns_head(), which then abandons the namespace. Just as the code >> coments say, we had been historically been clearing such errors because >> we have other ways to identify the namespace, but now we're not clearing >> that error. > > I don't understand what you are saying Keith. > > My comment was for this block: > -- > if (!status && nvme_multi_css(ctrl) && !csi_seen) { > dev_warn(ctrl->device, "Command set not reported for nsid:%d\n", > nsid); > status = -EINVAL; > } > -- > > I was saying that !status doesn't necessarily mean success, but it can > also mean that its an retry-able error status (due to transport or > controller). If we see a retry-able error we should still clear the > status so we don't abandon the namespace. > > This for example would achieve that: Sorry, meant this: -- diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index ba512c45b40f..3187cf768d08 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -1130,8 +1130,14 @@ static int nvme_identify_ns_descs(struct nvme_ctrl *ctrl, unsigned nsid, * Don't treat an error as fatal, as we potentially already * have a NGUID or EUI-64. */ - if (status > 0 && !(status & NVME_SC_DNR)) + if (status > 0 && !(status & NVME_SC_DNR)) { status = 0; + } else if (status == 0 && nvme_multi_css(ctrl) && !csi_seen) { + dev_warn(ctrl->device, + "Command set not reported for nsid:%d\n", + nsid); + status = -EINVAL; + } goto free_data; } -- _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme
next prev parent reply other threads:[~2020-06-24 18:33 UTC|newest] Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-06-22 16:25 [PATCHv3 0/5] nvme support for zoned namespace command set Keith Busch 2020-06-22 16:25 ` [PATCHv3 1/5] block: add capacity field to zone descriptors Keith Busch 2020-06-23 6:15 ` Hannes Reinecke 2020-06-23 8:44 ` Sagi Grimberg 2020-06-26 12:17 ` Jens Axboe 2020-06-26 12:17 ` Jens Axboe 2020-06-22 16:25 ` [PATCHv3 2/5] null_blk: introduce zone capacity for zoned device Keith Busch 2020-06-23 6:16 ` Hannes Reinecke 2020-06-23 8:45 ` Sagi Grimberg 2020-06-22 16:25 ` [PATCHv3 3/5] nvme: implement I/O Command Sets Command Set support Keith Busch 2020-06-23 6:20 ` Hannes Reinecke 2020-06-23 9:20 ` Niklas Cassel 2020-06-23 14:25 ` Keith Busch 2020-06-23 8:53 ` Sagi Grimberg 2020-06-23 11:25 ` Niklas Cassel 2020-06-23 14:59 ` Keith Busch 2020-06-23 14:59 ` Keith Busch 2020-06-23 22:10 ` Keith Busch 2020-06-23 22:10 ` Keith Busch 2020-06-23 23:17 ` Sagi Grimberg 2020-06-23 23:17 ` Sagi Grimberg 2020-06-24 17:25 ` Keith Busch 2020-06-24 17:25 ` Keith Busch 2020-06-24 17:46 ` Sagi Grimberg 2020-06-24 17:46 ` Sagi Grimberg 2020-06-24 18:03 ` Keith Busch 2020-06-24 18:03 ` Keith Busch 2020-06-24 18:28 ` Sagi Grimberg 2020-06-24 18:28 ` Sagi Grimberg 2020-06-24 18:33 ` Sagi Grimberg [this message] 2020-06-24 18:33 ` Sagi Grimberg 2020-06-24 18:40 ` Keith Busch 2020-06-24 18:40 ` Keith Busch 2020-06-24 19:03 ` Sagi Grimberg 2020-06-24 19:03 ` Sagi Grimberg 2020-06-24 21:49 ` Keith Busch 2020-06-24 21:49 ` Keith Busch 2020-06-24 22:54 ` Sagi Grimberg 2020-06-24 22:54 ` Sagi Grimberg 2020-06-24 23:54 ` Keith Busch 2020-06-24 23:54 ` Keith Busch 2020-06-23 23:20 ` Sagi Grimberg 2020-06-23 23:20 ` Sagi Grimberg 2020-06-26 8:54 ` Christoph Hellwig 2020-06-26 8:54 ` Christoph Hellwig 2020-06-22 16:25 ` [PATCHv3 4/5] nvme: support for multi-command set effects Keith Busch 2020-06-23 6:21 ` Hannes Reinecke 2020-06-23 17:43 ` Sagi Grimberg 2020-06-23 17:43 ` Sagi Grimberg 2020-06-22 16:25 ` [PATCHv3 5/5] nvme: support for zoned namespaces Keith Busch 2020-06-22 16:48 ` Johannes Thumshirn 2020-06-23 6:23 ` Hannes Reinecke 2020-06-23 17:45 ` Sagi Grimberg 2020-06-23 17:45 ` Sagi Grimberg 2020-06-24 9:11 ` Javier González 2020-06-24 9:11 ` Javier González 2020-06-29 13:53 ` Johannes Thumshirn 2020-06-29 13:53 ` Johannes Thumshirn
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=76966a48-0588-3f3c-0ec1-842cd2ac413d@grimberg.me \ --to=sagi@grimberg.me \ --cc=Johannes.Thumshirn@wdc.com \ --cc=Matias.Bjorling@wdc.com \ --cc=Niklas.Cassel@wdc.com \ --cc=axboe@kernel.dk \ --cc=dwagner@suse.de \ --cc=hch@lst.de \ --cc=javier.gonz@samsung.com \ --cc=kbusch@kernel.org \ --cc=linux-block@vger.kernel.org \ --cc=linux-nvme@lists.infradead.org \ --cc=martin.petersen@oracle.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.