* [PATCH V2] nvme: report capacity 0 for non supported ZNS SSDs @ 2020-11-02 13:22 Javier González 2020-11-02 15:44 ` Keith Busch 0 siblings, 1 reply; 11+ messages in thread From: Javier González @ 2020-11-02 13:22 UTC (permalink / raw) To: linux-nvme Cc: axboe, Niklas.Cassel, sagi, joshi.k, k.jensen, linux-block, kbusch, Javier González, hch Allow ZNS SSDs to be presented to the host even when they implement features that are not supported by the kernel zoned block device. Instead of rejecting the SSD at the NVMe driver level, deal with this in the block layer by setting capacity to 0, as we do with other things such as unsupported PI configurations. This allows to use standard management tools such as nvme-cli to choose a different format or firmware slot that is compatible with the Linux zoned block device. Changes since V1: - Apply feedback from Niklas: - Use IS_ENABLED() for checking config option - Use local variable - Use different variable names Signed-off-by: Javier González <javier.gonz@samsung.com> --- drivers/nvme/host/core.c | 3 +++ drivers/nvme/host/nvme.h | 1 + drivers/nvme/host/zns.c | 26 ++++++++++++++------------ 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index c190c56bf702..638997b6f5cd 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -2026,6 +2026,9 @@ static void nvme_update_disk_info(struct gendisk *disk, capacity = 0; } + if (IS_ENABLED(CONFIG_BLK_DEV_ZONED) && !ns->zoned_ns_supp) + capacity = 0; + set_capacity_revalidate_and_notify(disk, capacity, false); nvme_config_discard(disk, ns); diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h index 87737fa32360..ff4fe645ab9b 100644 --- a/drivers/nvme/host/nvme.h +++ b/drivers/nvme/host/nvme.h @@ -443,6 +443,7 @@ struct nvme_ns { u8 pi_type; #ifdef CONFIG_BLK_DEV_ZONED u64 zsze; + bool zoned_ns_supp; #endif unsigned long features; unsigned long flags; diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c index 57cfd78731fb..1ae039f9c20c 100644 --- a/drivers/nvme/host/zns.c +++ b/drivers/nvme/host/zns.c @@ -42,22 +42,25 @@ int nvme_update_zone_info(struct gendisk *disk, struct nvme_ns *ns, struct request_queue *q = disk->queue; struct nvme_command c = { }; struct nvme_id_ns_zns *id; + bool zoned_ns_supp = true; int status; /* Driver requires zone append support */ if (!(le32_to_cpu(log->iocs[nvme_cmd_zone_append]) & NVME_CMD_EFFECTS_CSUPP)) { + zoned_ns_supp = false; dev_warn(ns->ctrl->device, "append not supported for zoned namespace:%d\n", ns->head->ns_id); - return -EINVAL; - } - - /* 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; + } else { + /* 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); @@ -78,23 +81,22 @@ int nvme_update_zone_info(struct gendisk *disk, struct nvme_ns *ns, * operation characteristics. */ if (id->zoc) { + zoned_ns_supp = false; dev_warn(ns->ctrl->device, "zone operations:%x not supported for namespace:%u\n", le16_to_cpu(id->zoc), ns->head->ns_id); - status = -EINVAL; - goto free_data; } ns->zsze = nvme_lba_to_sect(ns, le64_to_cpu(id->lbafe[lbaf].zsze)); if (!is_power_of_2(ns->zsze)) { + zoned_ns_supp = false; dev_warn(ns->ctrl->device, "invalid zone size:%llu for namespace:%u\n", ns->zsze, ns->head->ns_id); - status = -EINVAL; - goto free_data; } q->limits.zoned = BLK_ZONED_HM; + ns->zoned_ns_supp = zoned_ns_supp; blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q); blk_queue_max_open_zones(q, le32_to_cpu(id->mor) + 1); blk_queue_max_active_zones(q, le32_to_cpu(id->mar) + 1); -- 2.17.1 _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH V2] nvme: report capacity 0 for non supported ZNS SSDs 2020-11-02 13:22 [PATCH V2] nvme: report capacity 0 for non supported ZNS SSDs Javier González @ 2020-11-02 15:44 ` Keith Busch 0 siblings, 0 replies; 11+ messages in thread From: Keith Busch @ 2020-11-02 15:44 UTC (permalink / raw) To: Javier González Cc: axboe, Niklas.Cassel, sagi, joshi.k, k.jensen, linux-nvme, linux-block, Javier González, hch On Mon, Nov 02, 2020 at 02:22:14PM +0100, Javier González wrote: > Changes since V1: > - Apply feedback from Niklas: > - Use IS_ENABLED() for checking config option Your v1 was correct. Using IS_ENBALED() attempts to use an undefined symbol when the CONFIG is not enabled: drivers/nvme/host/core.c: In function ‘nvme_update_disk_info’: drivers/nvme/host/core.c:2056:45: error: ‘struct nvme_ns’ has no member named ‘zoned_ns_supp’ 2056 | if (IS_ENABLED(CONFIG_BLK_DEV_ZONED) && !ns->zoned_ns_supp) | ^~ That said, I don't mind the concept, though I recall Christoph had concerns about the existing 0-capacity namespace used for invalid formats, so I'd like to hear more on that if he has some spare time to comment. _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <CGME20201102161505eucas1p19415e34eb0b14c7eca5a2c648569cf1e@eucas1p1.samsung.com>]
[parent not found: <0916865d50c640e3aa95dc542f3986b9@CAMSVWEXC01.scsc.local>]
* Re: [PATCH V2] nvme: report capacity 0 for non supported ZNS SSDs [not found] ` <0916865d50c640e3aa95dc542f3986b9@CAMSVWEXC01.scsc.local> @ 2020-11-02 16:30 ` Niklas Cassel 2020-11-02 18:08 ` hch 1 sibling, 0 replies; 11+ messages in thread From: Niklas Cassel @ 2020-11-02 16:30 UTC (permalink / raw) To: Javier Gonzalez Cc: axboe@kernel.dk, javier@javigon.com, sagi@grimberg.me, joshi.k@samsung.com, Klaus B. Jensen, linux-nvme@lists.infradead.org, linux-block@vger.kernel.org, kbusch@kernel.org, hch@lst.de On Mon, Nov 02, 2020 at 04:15:01PM +0000, Javier Gonzalez wrote: > > From: Keith Busch <kbusch@kernel.org> > Sent: Nov 2, 2020 16:45 > To: Javier González <javier@javigon.com> > Cc: linux-nvme@lists.infradead.org; linux-block@vger.kernel.org; hch@lst.de; sagi@grimberg.me; axboe@kernel.dk; joshi.k@samsung.com; "Klaus B. Jensen" <k.jensen@samsung.com>; Niklas.Cassel@wdc.com; Javier Gonzalez <javier.gonz@samsung.com> > Subject: Re: [PATCH V2] nvme: report capacity 0 for non supported ZNS SSDs > > On Mon, Nov 02, 2020 at 02:22:14PM +0100, Javier González wrote: > > Changes since V1: > > - Apply feedback from Niklas: > > - Use IS_ENABLED() for checking config option > > Your v1 was correct. Using IS_ENBALED() attempts to use an undefined > symbol when the CONFIG is not enabled: > > Oh. Ok. Will return to that. Keith is correct, sorry for that. https://www.kernel.org/doc/html/latest/process/coding-style.html#conditional-compilation "Thus, you still have to use an #ifdef if the code inside the block references symbols that will not exist if the condition is not met." Kind regards, Niklas > > drivers/nvme/host/core.c: In function ‘nvme_update_disk_info’: > drivers/nvme/host/core.c:2056:45: error: ‘struct nvme_ns’ has no member named ‘zoned_ns_supp’ > 2056 | if (IS_ENABLED(CONFIG_BLK_DEV_ZONED) && !ns->zoned_ns_supp) > | ^~ > > That said, I don't mind the concept, though I recall Christoph had > concerns about the existing 0-capacity namespace used for invalid > formats, so I'd like to hear more on that if he has some spare time to > comment. > > Sounds good. I'll respin a V3 in the meantime. Assuming not supported in the beginning gives problems with non zone namespaces either way. So I'll fix that too. > _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2] nvme: report capacity 0 for non supported ZNS SSDs [not found] ` <0916865d50c640e3aa95dc542f3986b9@CAMSVWEXC01.scsc.local> 2020-11-02 16:30 ` Niklas Cassel @ 2020-11-02 18:08 ` hch 2020-11-02 18:33 ` Keith Busch 1 sibling, 1 reply; 11+ messages in thread From: hch @ 2020-11-02 18:08 UTC (permalink / raw) To: Javier Gonzalez Cc: axboe@kernel.dk, Niklas.Cassel@wdc.com, javier@javigon.com, sagi@grimberg.me, joshi.k@samsung.com, Klaus B. Jensen, linux-nvme@lists.infradead.org, linux-block@vger.kernel.org, kbusch@kernel.org, hch@lst.de On Mon, Nov 02, 2020 at 04:15:01PM +0000, Javier Gonzalez wrote: > That said, I don't mind the concept, though I recall Christoph had > concerns about the existing 0-capacity namespace used for invalid > formats, so I'd like to hear more on that if he has some spare time to > comment. Yes, I really don't think 0 sized block devices are the right interface for namespaces we can't access. I think the proper fix is to ensure that we have a character device that allows submitting I/O commands for each namespaces including those where we don't understand the I/O command set or parts of it. That should also really help to have a proper access model for the KV command set. Initially we only need NVME_IOCTL_IO64_CMD, but eventually we'll need some support for async submissions, be that through io_uring or other ways. _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2] nvme: report capacity 0 for non supported ZNS SSDs 2020-11-02 18:08 ` hch @ 2020-11-02 18:33 ` Keith Busch 2020-11-02 18:58 ` hch 0 siblings, 1 reply; 11+ messages in thread From: Keith Busch @ 2020-11-02 18:33 UTC (permalink / raw) To: hch@lst.de Cc: axboe@kernel.dk, Niklas.Cassel@wdc.com, javier@javigon.com, sagi@grimberg.me, joshi.k@samsung.com, Klaus B. Jensen, linux-nvme@lists.infradead.org, linux-block@vger.kernel.org, Javier Gonzalez On Mon, Nov 02, 2020 at 07:08:37PM +0100, hch@lst.de wrote: > On Mon, Nov 02, 2020 at 04:15:01PM +0000, Javier Gonzalez wrote: > > That said, I don't mind the concept, though I recall Christoph had > > concerns about the existing 0-capacity namespace used for invalid > > formats, so I'd like to hear more on that if he has some spare time to > > comment. > > Yes, I really don't think 0 sized block devices are the right interface > for namespaces we can't access. I think the proper fix is to ensure that > we have a character device that allows submitting I/O commands for each > namespaces including those where we don't understand the I/O command set > or parts of it. That should also really help to have a proper access > model for the KV command set. Initially we only need NVME_IOCTL_IO64_CMD, > but eventually we'll need some support for async submissions, be > that through io_uring or other ways. I can see this going one of two ways: a) Set up the existing controller character device with a generic disk-less request_queue to the IO queues accepting IO commands to arbitrary NSIDs. b) Each namespace that can't be supported gets their own character device. I'm leaning toward option "a". While it doesn't create handles to unique namespaces, it has more resilience to potentially future changes. And I recall the target side had a potential use for that, too. _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2] nvme: report capacity 0 for non supported ZNS SSDs 2020-11-02 18:33 ` Keith Busch @ 2020-11-02 18:58 ` hch 2020-11-04 14:26 ` Hannes Reinecke 0 siblings, 1 reply; 11+ messages in thread From: hch @ 2020-11-02 18:58 UTC (permalink / raw) To: Keith Busch Cc: axboe@kernel.dk, Niklas.Cassel@wdc.com, javier@javigon.com, sagi@grimberg.me, joshi.k@samsung.com, Klaus B. Jensen, linux-nvme@lists.infradead.org, linux-block@vger.kernel.org, Javier Gonzalez, hch@lst.de On Mon, Nov 02, 2020 at 10:33:55AM -0800, Keith Busch wrote: > I can see this going one of two ways: > > a) Set up the existing controller character device with a generic > disk-less request_queue to the IO queues accepting IO commands to > arbitrary NSIDs. > > b) Each namespace that can't be supported gets their own character > device. > > I'm leaning toward option "a". While it doesn't create handles to unique > namespaces, it has more resilience to potentially future changes. And I > recall the target side had a potential use for that, too. The problem with a) is that it can't be used to give users or groups access to just one namespaces, so it causes a real access control nightmare. The problem with b) is that now applications will break when we add support for new command sets or features. I think c) Each namespace gets its own character device, period. is the only sensible option. _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2] nvme: report capacity 0 for non supported ZNS SSDs 2020-11-02 18:58 ` hch @ 2020-11-04 14:26 ` Hannes Reinecke 2020-11-04 14:29 ` hch 0 siblings, 1 reply; 11+ messages in thread From: Hannes Reinecke @ 2020-11-04 14:26 UTC (permalink / raw) To: hch@lst.de, Keith Busch Cc: axboe@kernel.dk, Niklas.Cassel@wdc.com, javier@javigon.com, sagi@grimberg.me, joshi.k@samsung.com, Klaus B. Jensen, linux-nvme@lists.infradead.org, linux-block@vger.kernel.org, Javier Gonzalez On 11/2/20 7:58 PM, hch@lst.de wrote: > On Mon, Nov 02, 2020 at 10:33:55AM -0800, Keith Busch wrote: >> I can see this going one of two ways: >> >> a) Set up the existing controller character device with a generic >> disk-less request_queue to the IO queues accepting IO commands to >> arbitrary NSIDs. >> >> b) Each namespace that can't be supported gets their own character >> device. >> >> I'm leaning toward option "a". While it doesn't create handles to unique >> namespaces, it has more resilience to potentially future changes. And I >> recall the target side had a potential use for that, too. > > The problem with a) is that it can't be used to give users or groups > access to just one namespaces, so it causes a real access control > nightmare. The problem with b) is that now applications will break > when we add support for new command sets or features. I think > > c) Each namespace gets its own character device, period. > > is the only sensible option. > I hardly dare to mention bsg here; but the is pretty similar to what it set out to do ... Or yet another interface? Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect hare@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2] nvme: report capacity 0 for non supported ZNS SSDs 2020-11-04 14:26 ` Hannes Reinecke @ 2020-11-04 14:29 ` hch 2020-11-04 14:46 ` Hannes Reinecke 0 siblings, 1 reply; 11+ messages in thread From: hch @ 2020-11-04 14:29 UTC (permalink / raw) To: Hannes Reinecke Cc: axboe@kernel.dk, Niklas.Cassel@wdc.com, javier@javigon.com, sagi@grimberg.me, joshi.k@samsung.com, Klaus B. Jensen, linux-nvme@lists.infradead.org, linux-block@vger.kernel.org, Keith Busch, Javier Gonzalez, hch@lst.de On Wed, Nov 04, 2020 at 03:26:46PM +0100, Hannes Reinecke wrote: > I hardly dare to mention bsg here; but the is pretty similar to what it set > out to do ... Except that: a) we created a complete mess with bsg by overloading the scsi ioctls with some of the transport stuff. b) bsg would not work with existing tools. A character device that accepts the same ioctl will just work. _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2] nvme: report capacity 0 for non supported ZNS SSDs 2020-11-04 14:29 ` hch @ 2020-11-04 14:46 ` Hannes Reinecke 2020-11-05 7:37 ` hch 0 siblings, 1 reply; 11+ messages in thread From: Hannes Reinecke @ 2020-11-04 14:46 UTC (permalink / raw) To: hch@lst.de Cc: axboe@kernel.dk, Niklas.Cassel@wdc.com, javier@javigon.com, sagi@grimberg.me, joshi.k@samsung.com, Klaus B. Jensen, linux-nvme@lists.infradead.org, linux-block@vger.kernel.org, Keith Busch, Javier Gonzalez On 11/4/20 3:29 PM, hch@lst.de wrote: > On Wed, Nov 04, 2020 at 03:26:46PM +0100, Hannes Reinecke wrote: >> I hardly dare to mention bsg here; but the is pretty similar to what it set >> out to do ... > > Except that: > > a) we created a complete mess with bsg by overloading the scsi ioctls > with some of the transport stuff. > b) bsg would not work with existing tools. A character device that > accepts the same ioctl will just work. > ... as would a bsg device which could accept said ioctl ... Plus it feels a bit weird, having a generic command passthrough character device which is then avoided in favour of a protocol-specific command passthrough device. Which we had been arguing for years with IHVs for _not_ doing it. So what is different here? Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect hare@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2] nvme: report capacity 0 for non supported ZNS SSDs 2020-11-04 14:46 ` Hannes Reinecke @ 2020-11-05 7:37 ` hch 2020-11-05 7:42 ` Hannes Reinecke 0 siblings, 1 reply; 11+ messages in thread From: hch @ 2020-11-05 7:37 UTC (permalink / raw) To: Hannes Reinecke Cc: axboe@kernel.dk, Niklas.Cassel@wdc.com, javier@javigon.com, sagi@grimberg.me, joshi.k@samsung.com, Klaus B. Jensen, linux-nvme@lists.infradead.org, linux-block@vger.kernel.org, Keith Busch, Javier Gonzalez, hch@lst.de On Wed, Nov 04, 2020 at 03:46:02PM +0100, Hannes Reinecke wrote: > ... as would a bsg device which could accept said ioctl ... Sure we could. So we'd have to add more code to almost 1000 lines of code in bsg that are not useful to the nvme use case to make it useful for that use case. Or we could just add about 50 lines of code to NVMe to do the right thing. _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2] nvme: report capacity 0 for non supported ZNS SSDs 2020-11-05 7:37 ` hch @ 2020-11-05 7:42 ` Hannes Reinecke 0 siblings, 0 replies; 11+ messages in thread From: Hannes Reinecke @ 2020-11-05 7:42 UTC (permalink / raw) To: hch@lst.de Cc: axboe@kernel.dk, Niklas.Cassel@wdc.com, javier@javigon.com, sagi@grimberg.me, joshi.k@samsung.com, Klaus B. Jensen, linux-nvme@lists.infradead.org, linux-block@vger.kernel.org, Keith Busch, Javier Gonzalez On 11/5/20 8:37 AM, hch@lst.de wrote: > On Wed, Nov 04, 2020 at 03:46:02PM +0100, Hannes Reinecke wrote: >> ... as would a bsg device which could accept said ioctl ... > > Sure we could. So we'd have to add more code to almost 1000 lines of > code in bsg that are not useful to the nvme use case to make it useful > for that use case. Or we could just add about 50 lines of code to NVMe > to do the right thing. > My point wasn't so much that it'd be easier to code. Just wanted to point out how we've argued in the past. But anyway: you are the maintainer, you get to decide. Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect hare@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-11-05 7:42 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-02 13:22 [PATCH V2] nvme: report capacity 0 for non supported ZNS SSDs Javier González
2020-11-02 15:44 ` Keith Busch
[not found] <CGME20201102161505eucas1p19415e34eb0b14c7eca5a2c648569cf1e@eucas1p1.samsung.com>
[not found] ` <0916865d50c640e3aa95dc542f3986b9@CAMSVWEXC01.scsc.local>
2020-11-02 16:30 ` Niklas Cassel
2020-11-02 18:08 ` hch
2020-11-02 18:33 ` Keith Busch
2020-11-02 18:58 ` hch
2020-11-04 14:26 ` Hannes Reinecke
2020-11-04 14:29 ` hch
2020-11-04 14:46 ` Hannes Reinecke
2020-11-05 7:37 ` hch
2020-11-05 7:42 ` Hannes Reinecke
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox