* [PATCH v2 0/2] nvme: Enable generic interface (/dev/ngX) for unknown command sets [not found] <CGME20220607134309eucas1p1724cee80a5bc116c652e91422d474ccd@eucas1p1.samsung.com> @ 2022-06-07 13:40 ` Joel Granados 2022-06-07 13:40 ` [PATCH v2 1/2] nvme-multipath: refactor nvme_mpath_add_disk Joel Granados 2022-06-07 13:40 ` [PATCH v2 2/2] nvme: enable generic interface (/dev/ngXnY) for unknown command sets Joel Granados 0 siblings, 2 replies; 9+ messages in thread From: Joel Granados @ 2022-06-07 13:40 UTC (permalink / raw) To: hch, kbusch; +Cc: joshi.k, linux-nvme, gost.dev, k.jensen Currently block and char interface do not show up for any command set other than NVM and ZNS. This series enables char interface to come up for unknown command sets. Patch 1: Is prep. It allows the re-use of nvme_mpath_add_disk for supported as well as independent command sets. Patch 2: The following functions become aware of whether the command set is independent or not: nvme_alloc_ns and nvme_validate_ns. A boolean is introduced in each of them to call the relevant helper to identify and update the namespaces. nvme_update_ns_info_cs_indep is added to update command set independent devices. Changes since v1: - Use command-set independent id-ns to enable unknown command-sets Joel Granados (2): nvme-multipath: refactor nvme_mpath_add_disk nvme: enable generic interface (/dev/ngXnY) for unknown command sets drivers/nvme/host/core.c | 83 ++++++++++++++++++++++++++++++----- drivers/nvme/host/multipath.c | 7 +-- drivers/nvme/host/nvme.h | 5 +-- 3 files changed, 78 insertions(+), 17 deletions(-) -- 2.30.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] nvme-multipath: refactor nvme_mpath_add_disk 2022-06-07 13:40 ` [PATCH v2 0/2] nvme: Enable generic interface (/dev/ngX) for unknown command sets Joel Granados @ 2022-06-07 13:40 ` Joel Granados 2022-06-13 18:16 ` Christoph Hellwig 2022-06-07 13:40 ` [PATCH v2 2/2] nvme: enable generic interface (/dev/ngXnY) for unknown command sets Joel Granados 1 sibling, 1 reply; 9+ messages in thread From: Joel Granados @ 2022-06-07 13:40 UTC (permalink / raw) To: hch, kbusch; +Cc: joshi.k, linux-nvme, gost.dev, k.jensen Pass anagrpid as second argument. This is prep patch that allows reusing this function for supporting unknown command sets. Signed-off-by: Joel Granados <j.granados@samsung.com> Signed-off-by: Kanchan Joshi <joshi.k@samsung.com> --- drivers/nvme/host/core.c | 2 +- drivers/nvme/host/multipath.c | 7 ++++--- drivers/nvme/host/nvme.h | 5 ++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 24165daee3c8..06b9718b0540 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -4027,7 +4027,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid, if (!nvme_ns_head_multipath(ns->head)) nvme_add_ns_cdev(ns); - nvme_mpath_add_disk(ns, id); + nvme_mpath_add_disk(ns, id->anagrpid); nvme_fault_inject_init(&ns->fault_inject, ns->disk->disk_name); kfree(id); diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c index d3e2440d8abb..6ae7c042b1dc 100644 --- a/drivers/nvme/host/multipath.c +++ b/drivers/nvme/host/multipath.c @@ -3,6 +3,7 @@ * Copyright (c) 2017-2018 Christoph Hellwig. */ +#include "linux/types.h" #include <linux/backing-dev.h> #include <linux/moduleparam.h> #include <linux/vmalloc.h> @@ -800,16 +801,16 @@ static int nvme_lookup_ana_group_desc(struct nvme_ctrl *ctrl, return -ENXIO; /* just break out of the loop */ } -void nvme_mpath_add_disk(struct nvme_ns *ns, struct nvme_id_ns *id) +void nvme_mpath_add_disk(struct nvme_ns *ns, __le32 anagrpid) { if (nvme_ctrl_use_ana(ns->ctrl)) { struct nvme_ana_group_desc desc = { - .grpid = id->anagrpid, + .grpid = anagrpid, .state = 0, }; mutex_lock(&ns->ctrl->ana_lock); - ns->ana_grpid = le32_to_cpu(id->anagrpid); + ns->ana_grpid = le32_to_cpu(anagrpid); nvme_parse_ana_log(ns->ctrl, &desc, nvme_lookup_ana_group_desc); mutex_unlock(&ns->ctrl->ana_lock); if (desc.state) { diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h index 9b72b6ecf33c..14b097dfa7fa 100644 --- a/drivers/nvme/host/nvme.h +++ b/drivers/nvme/host/nvme.h @@ -808,7 +808,7 @@ void nvme_mpath_default_iopolicy(struct nvme_subsystem *subsys); void nvme_failover_req(struct request *req); void nvme_kick_requeue_lists(struct nvme_ctrl *ctrl); int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl,struct nvme_ns_head *head); -void nvme_mpath_add_disk(struct nvme_ns *ns, struct nvme_id_ns *id); +void nvme_mpath_add_disk(struct nvme_ns *ns, __le32 anagrpid); void nvme_mpath_remove_disk(struct nvme_ns_head *head); int nvme_mpath_init_identify(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id); void nvme_mpath_init_ctrl(struct nvme_ctrl *ctrl); @@ -850,8 +850,7 @@ static inline int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, { return 0; } -static inline void nvme_mpath_add_disk(struct nvme_ns *ns, - struct nvme_id_ns *id) +static inline void nvme_mpath_add_disk(struct nvme_ns *ns, __le32 anagrpid) { } static inline void nvme_mpath_remove_disk(struct nvme_ns_head *head) -- 2.30.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] nvme-multipath: refactor nvme_mpath_add_disk 2022-06-07 13:40 ` [PATCH v2 1/2] nvme-multipath: refactor nvme_mpath_add_disk Joel Granados @ 2022-06-13 18:16 ` Christoph Hellwig 2022-06-15 7:50 ` Joel Granados 0 siblings, 1 reply; 9+ messages in thread From: Christoph Hellwig @ 2022-06-13 18:16 UTC (permalink / raw) To: Joel Granados; +Cc: hch, kbusch, joshi.k, linux-nvme, gost.dev, k.jensen > +++ b/drivers/nvme/host/multipath.c > @@ -3,6 +3,7 @@ > * Copyright (c) 2017-2018 Christoph Hellwig. > */ > > +#include "linux/types.h" Please always use <> style includes for system headers. But I don't think we even need this include. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] nvme-multipath: refactor nvme_mpath_add_disk 2022-06-13 18:16 ` Christoph Hellwig @ 2022-06-15 7:50 ` Joel Granados 0 siblings, 0 replies; 9+ messages in thread From: Joel Granados @ 2022-06-15 7:50 UTC (permalink / raw) To: Christoph Hellwig; +Cc: kbusch, joshi.k, linux-nvme, gost.dev, k.jensen [-- Attachment #1: Type: text/plain, Size: 407 bytes --] On Mon, Jun 13, 2022 at 08:16:20PM +0200, Christoph Hellwig wrote: > > +++ b/drivers/nvme/host/multipath.c > > @@ -3,6 +3,7 @@ > > * Copyright (c) 2017-2018 Christoph Hellwig. > > */ > > > > +#include "linux/types.h" > > Please always use <> style includes for system headers. But I don't > think we even need this include. I'll just remove this line for the next version. Thx. Joel [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] nvme: enable generic interface (/dev/ngXnY) for unknown command sets 2022-06-07 13:40 ` [PATCH v2 0/2] nvme: Enable generic interface (/dev/ngX) for unknown command sets Joel Granados 2022-06-07 13:40 ` [PATCH v2 1/2] nvme-multipath: refactor nvme_mpath_add_disk Joel Granados @ 2022-06-07 13:40 ` Joel Granados 2022-06-13 18:24 ` Christoph Hellwig 1 sibling, 1 reply; 9+ messages in thread From: Joel Granados @ 2022-06-07 13:40 UTC (permalink / raw) To: hch, kbusch; +Cc: joshi.k, linux-nvme, gost.dev, k.jensen Extend nvme_alloc_ns() and nvme_validate_ns() for unknown command-set as well. Both are made to use a new helper (nvme_update_ns_info_cs_indep) which is similar to nvme_update_ns_info but performs fewer operations to get the generic interface up. Suggested-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Joel Granados <j.granados@samsung.com> Signed-off-by: Kanchan Joshi <joshi.k@samsung.com> --- drivers/nvme/host/core.c | 83 ++++++++++++++++++++++++++++++++++------ 1 file changed, 72 insertions(+), 11 deletions(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 06b9718b0540..c7eb92480fad 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -1912,6 +1912,33 @@ static void nvme_set_chunk_sectors(struct nvme_ns *ns, struct nvme_id_ns *id) blk_queue_chunk_sectors(ns->queue, iob); } +static int nvme_update_ns_info_cs_indep(struct nvme_ns *ns, + struct nvme_id_ns_cs_indep *id) +{ + blk_mq_freeze_queue(ns->disk->queue); + nvme_set_queue_limits(ns->ctrl, ns->queue); + set_disk_ro(ns->disk, (id->nsattr & NVME_NS_ATTR_RO) || + test_bit(NVME_NS_FORCE_RO, &ns->flags)); + blk_mq_unfreeze_queue(ns->disk->queue); + + if (nvme_ns_head_multipath(ns->head)) { + blk_mq_freeze_queue(ns->head->disk->queue); + set_disk_ro(ns->head->disk, + (id->nsattr & NVME_NS_ATTR_RO) || + test_bit(NVME_NS_FORCE_RO, &ns->flags)); + nvme_mpath_revalidate_paths(ns); + blk_stack_limits(&ns->head->disk->queue->limits, + &ns->queue->limits, 0); + blk_mq_unfreeze_queue(ns->head->disk->queue); + } + + /* Hide the block-interface for these devices */ + ns->disk->flags |= GENHD_FL_HIDDEN; + set_bit(NVME_NS_READY, &ns->flags); + + return 0; +} + static int nvme_update_ns_info(struct nvme_ns *ns, struct nvme_id_ns *id) { unsigned lbaf = nvme_lbaf_index(id->flbas); @@ -3953,15 +3980,27 @@ static void nvme_ns_add_to_ctrl_list(struct nvme_ns *ns) list_add(&ns->list, &ns->ctrl->namespaces); } +static inline bool nvme_ns_indep(struct nvme_ns_ids *ids) +{ + return ids->csi != NVME_CSI_NVM && ids->csi != NVME_CSI_ZNS; +} + static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid, struct nvme_ns_ids *ids) { + int ret = 0; struct nvme_ns *ns; struct gendisk *disk; struct nvme_id_ns *id; + struct nvme_id_ns_cs_indep *indep_id; int node = ctrl->numa_node; + bool cs_indep = nvme_ns_indep(ids); - if (nvme_identify_ns(ctrl, nsid, ids, &id)) + if (cs_indep) + ret = nvme_identify_ns_cs_indep(ctrl, nsid, &indep_id); + else + ret = nvme_identify_ns(ctrl, nsid, ids, &id); + if (ret) return; ns = kzalloc_node(sizeof(*ns), GFP_KERNEL, node); @@ -3987,7 +4026,8 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid, ns->ctrl = ctrl; kref_init(&ns->kref); - if (nvme_init_ns_head(ns, nsid, ids, id->nmic & NVME_NS_NMIC_SHARED)) + if (nvme_init_ns_head(ns, nsid, ids, + (cs_indep ? indep_id->nmic : id->nmic) & NVME_NS_NMIC_SHARED)) goto out_cleanup_disk; /* @@ -4013,7 +4053,11 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid, ns->head->instance); } - if (nvme_update_ns_info(ns, id)) + if (cs_indep) + ret = nvme_update_ns_info_cs_indep(ns, indep_id); + else + ret = nvme_update_ns_info(ns, id); + if (ret) goto out_unlink_ns; down_write(&ctrl->namespaces_rwsem); @@ -4027,9 +4071,12 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid, if (!nvme_ns_head_multipath(ns->head)) nvme_add_ns_cdev(ns); - nvme_mpath_add_disk(ns, id->anagrpid); + nvme_mpath_add_disk(ns, cs_indep ? indep_id->anagrpid : id->anagrpid); nvme_fault_inject_init(&ns->fault_inject, ns->disk->disk_name); - kfree(id); + if (cs_indep) + kfree(indep_id); + else + kfree(id); return; @@ -4050,7 +4097,10 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid, out_free_ns: kfree(ns); out_free_id: - kfree(id); + if (cs_indep) + kfree(indep_id); + else + kfree(id); } static void nvme_ns_remove(struct nvme_ns *ns) @@ -4112,12 +4162,17 @@ static void nvme_ns_remove_by_nsid(struct nvme_ctrl *ctrl, u32 nsid) static void nvme_validate_ns(struct nvme_ns *ns, struct nvme_ns_ids *ids) { struct nvme_id_ns *id; + struct nvme_id_ns_cs_indep *indep_id; int ret = NVME_SC_INVALID_NS | NVME_SC_DNR; + bool cs_indep = nvme_ns_indep(ids); if (test_bit(NVME_NS_DEAD, &ns->flags)) goto out; - ret = nvme_identify_ns(ns->ctrl, ns->head->ns_id, ids, &id); + if (cs_indep) + ret = nvme_identify_ns_cs_indep(ns->ctrl, ns->head->ns_id, &indep_id); + else + ret = nvme_identify_ns(ns->ctrl, ns->head->ns_id, ids, &id); if (ret) goto out; @@ -4128,10 +4183,16 @@ static void nvme_validate_ns(struct nvme_ns *ns, struct nvme_ns_ids *ids) goto out_free_id; } - ret = nvme_update_ns_info(ns, id); + if (cs_indep) + ret = nvme_update_ns_info_cs_indep(ns, indep_id); + else + ret = nvme_update_ns_info(ns, id); out_free_id: - kfree(id); + if (cs_indep) + kfree(indep_id); + else + kfree(id); out: /* * Only remove the namespace if we got a fatal error back from the @@ -4193,8 +4254,8 @@ static void nvme_validate_or_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid) nvme_alloc_ns(ctrl, nsid, &ids); break; default: - dev_warn(ctrl->device, "unknown csi %u for nsid %u\n", - ids.csi, nsid); + /* required to enable char-interface for unknown command sets*/ + nvme_alloc_ns(ctrl, nsid, &ids); break; } } -- 2.30.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] nvme: enable generic interface (/dev/ngXnY) for unknown command sets 2022-06-07 13:40 ` [PATCH v2 2/2] nvme: enable generic interface (/dev/ngXnY) for unknown command sets Joel Granados @ 2022-06-13 18:24 ` Christoph Hellwig 2022-06-15 11:20 ` Joel Granados 0 siblings, 1 reply; 9+ messages in thread From: Christoph Hellwig @ 2022-06-13 18:24 UTC (permalink / raw) To: Joel Granados; +Cc: hch, kbusch, joshi.k, linux-nvme, gost.dev, k.jensen On Tue, Jun 07, 2022 at 03:40:55PM +0200, Joel Granados wrote: > static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid, > struct nvme_ns_ids *ids) > { > + int ret = 0; > struct nvme_ns *ns; > struct gendisk *disk; > struct nvme_id_ns *id; > + struct nvme_id_ns_cs_indep *indep_id; > int node = ctrl->numa_node; > + bool cs_indep = nvme_ns_indep(ids); > > - if (nvme_identify_ns(ctrl, nsid, ids, &id)) > + if (cs_indep) > + ret = nvme_identify_ns_cs_indep(ctrl, nsid, &indep_id); > + else > + ret = nvme_identify_ns(ctrl, nsid, ids, &id); nvme_validate_or_alloc_ns already does the nvme_identify_ns_cs_indep, so we should be able to reuse it or move it here. > - if (nvme_init_ns_head(ns, nsid, ids, id->nmic & NVME_NS_NMIC_SHARED)) > + if (nvme_init_ns_head(ns, nsid, ids, > + (cs_indep ? indep_id->nmic : id->nmic) & NVME_NS_NMIC_SHARED)) This is pretty ugly. I think if there is no command set idepdent structure supported, we should probably just fake one up based on the legacy Identify Namespace. > + if (cs_indep) > + ret = nvme_update_ns_info_cs_indep(ns, indep_id); > + else > + ret = nvme_update_ns_info(ns, id); And here we should do a switch on ids->csi ala: switch (ids.csi) { case NVME_CSI_NVM: case NVME_CSI_ZNS: ret = nvme_update_ns_info_block(ns, id); break default: nvme_update_ns_info_generic(ns, iid); break; } ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] nvme: enable generic interface (/dev/ngXnY) for unknown command sets 2022-06-13 18:24 ` Christoph Hellwig @ 2022-06-15 11:20 ` Joel Granados 2022-06-15 14:50 ` Joel Granados 2022-06-22 14:11 ` Joel Granados 0 siblings, 2 replies; 9+ messages in thread From: Joel Granados @ 2022-06-15 11:20 UTC (permalink / raw) To: Christoph Hellwig; +Cc: kbusch, joshi.k, linux-nvme, gost.dev, k.jensen [-- Attachment #1: Type: text/plain, Size: 1954 bytes --] On Mon, Jun 13, 2022 at 08:24:39PM +0200, Christoph Hellwig wrote: > On Tue, Jun 07, 2022 at 03:40:55PM +0200, Joel Granados wrote: > > > static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid, > > struct nvme_ns_ids *ids) > > { > > + int ret = 0; > > struct nvme_ns *ns; > > struct gendisk *disk; > > struct nvme_id_ns *id; > > + struct nvme_id_ns_cs_indep *indep_id; > > int node = ctrl->numa_node; > > + bool cs_indep = nvme_ns_indep(ids); > > > > - if (nvme_identify_ns(ctrl, nsid, ids, &id)) > > + if (cs_indep) > > + ret = nvme_identify_ns_cs_indep(ctrl, nsid, &indep_id); > > + else > > + ret = nvme_identify_ns(ctrl, nsid, ids, &id); > > nvme_validate_or_alloc_ns already does the nvme_identify_ns_cs_indep, so > we should be able to reuse it or move it here. One way we can do this is to take the nvme_id_ns_cs_indep id that we got from calling nvme_identify_ns_cs_indep and pass it along to nvme_alloc_ns. Let me adjust this for V3. > > > - if (nvme_init_ns_head(ns, nsid, ids, id->nmic & NVME_NS_NMIC_SHARED)) > > + if (nvme_init_ns_head(ns, nsid, ids, > > + (cs_indep ? indep_id->nmic : id->nmic) & NVME_NS_NMIC_SHARED)) > > This is pretty ugly. I think if there is no command set idepdent > structure supported, we should probably just fake one up based on the > legacy Identify Namespace. Do you mean that ns->head should be "faked" with a call different than nvme_init_ns_head which is based on the legacy Identify Namespace? > > > + if (cs_indep) > > + ret = nvme_update_ns_info_cs_indep(ns, indep_id); > > + else > > + ret = nvme_update_ns_info(ns, id); > > And here we should do a switch on ids->csi ala: > > switch (ids.csi) { > case NVME_CSI_NVM: > case NVME_CSI_ZNS: > ret = nvme_update_ns_info_block(ns, id); > break > default: > nvme_update_ns_info_generic(ns, iid); > break; > } > Will adjust for V3 Joel [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] nvme: enable generic interface (/dev/ngXnY) for unknown command sets 2022-06-15 11:20 ` Joel Granados @ 2022-06-15 14:50 ` Joel Granados 2022-06-22 14:11 ` Joel Granados 1 sibling, 0 replies; 9+ messages in thread From: Joel Granados @ 2022-06-15 14:50 UTC (permalink / raw) To: Christoph Hellwig; +Cc: kbusch, joshi.k, linux-nvme, gost.dev, k.jensen [-- Attachment #1: Type: text/plain, Size: 2310 bytes --] On Wed, Jun 15, 2022 at 01:20:19PM +0200, Joel Granados wrote: > On Mon, Jun 13, 2022 at 08:24:39PM +0200, Christoph Hellwig wrote: > > On Tue, Jun 07, 2022 at 03:40:55PM +0200, Joel Granados wrote: > > > > > static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid, > > > struct nvme_ns_ids *ids) > > > { > > > + int ret = 0; > > > struct nvme_ns *ns; > > > struct gendisk *disk; > > > struct nvme_id_ns *id; > > > + struct nvme_id_ns_cs_indep *indep_id; > > > int node = ctrl->numa_node; > > > + bool cs_indep = nvme_ns_indep(ids); > > > > > > - if (nvme_identify_ns(ctrl, nsid, ids, &id)) > > > + if (cs_indep) > > > + ret = nvme_identify_ns_cs_indep(ctrl, nsid, &indep_id); > > > + else > > > + ret = nvme_identify_ns(ctrl, nsid, ids, &id); > > > > nvme_validate_or_alloc_ns already does the nvme_identify_ns_cs_indep, so > > we should be able to reuse it or move it here. > One way we can do this is to take the nvme_id_ns_cs_indep id that we got > from calling nvme_identify_ns_cs_indep and pass it along to > nvme_alloc_ns. Let me adjust this for V3. > > > > > > - if (nvme_init_ns_head(ns, nsid, ids, id->nmic & NVME_NS_NMIC_SHARED)) > > > + if (nvme_init_ns_head(ns, nsid, ids, > > > + (cs_indep ? indep_id->nmic : id->nmic) & NVME_NS_NMIC_SHARED)) > > > > This is pretty ugly. I think if there is no command set idepdent > > structure supported, we should probably just fake one up based on the > > legacy Identify Namespace. > > Do you mean that ns->head should be "faked" with a call different > than nvme_init_ns_head which is based on the legacy Identify Namespace? > > > > > > + if (cs_indep) > > > + ret = nvme_update_ns_info_cs_indep(ns, indep_id); > > > + else > > > + ret = nvme_update_ns_info(ns, id); > > > > And here we should do a switch on ids->csi ala: > > > > switch (ids.csi) { > > case NVME_CSI_NVM: > > case NVME_CSI_ZNS: > > ret = nvme_update_ns_info_block(ns, id); > > break > > default: > > nvme_update_ns_info_generic(ns, iid); > > break; > > } > > > Will adjust for V3 I see the point of making the command sets more explicit and for consistency I will also change the other conditionals. Its a bit more lines, but we gain clarity. > > Joel [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] nvme: enable generic interface (/dev/ngXnY) for unknown command sets 2022-06-15 11:20 ` Joel Granados 2022-06-15 14:50 ` Joel Granados @ 2022-06-22 14:11 ` Joel Granados 1 sibling, 0 replies; 9+ messages in thread From: Joel Granados @ 2022-06-22 14:11 UTC (permalink / raw) To: Christoph Hellwig; +Cc: kbusch, joshi.k, linux-nvme, gost.dev, k.jensen [-- Attachment #1: Type: text/plain, Size: 2427 bytes --] On Wed, Jun 15, 2022 at 01:20:19PM +0200, Joel Granados wrote: > On Mon, Jun 13, 2022 at 08:24:39PM +0200, Christoph Hellwig wrote: > > On Tue, Jun 07, 2022 at 03:40:55PM +0200, Joel Granados wrote: > > > > > static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid, > > > struct nvme_ns_ids *ids) > > > { > > > + int ret = 0; > > > struct nvme_ns *ns; > > > struct gendisk *disk; > > > struct nvme_id_ns *id; > > > + struct nvme_id_ns_cs_indep *indep_id; > > > int node = ctrl->numa_node; > > > + bool cs_indep = nvme_ns_indep(ids); > > > > > > - if (nvme_identify_ns(ctrl, nsid, ids, &id)) > > > + if (cs_indep) > > > + ret = nvme_identify_ns_cs_indep(ctrl, nsid, &indep_id); > > > + else > > > + ret = nvme_identify_ns(ctrl, nsid, ids, &id); > > > > nvme_validate_or_alloc_ns already does the nvme_identify_ns_cs_indep, so > > we should be able to reuse it or move it here. > One way we can do this is to take the nvme_id_ns_cs_indep id that we got > from calling nvme_identify_ns_cs_indep and pass it along to > nvme_alloc_ns. Let me adjust this for V3. > > > > > > - if (nvme_init_ns_head(ns, nsid, ids, id->nmic & NVME_NS_NMIC_SHARED)) > > > + if (nvme_init_ns_head(ns, nsid, ids, > > > + (cs_indep ? indep_id->nmic : id->nmic) & NVME_NS_NMIC_SHARED)) > > > > This is pretty ugly. I think if there is no command set idepdent > > structure supported, we should probably just fake one up based on the > > legacy Identify Namespace. > > Do you mean that ns->head should be "faked" with a call different > than nvme_init_ns_head which is based on the legacy Identify Namespace? I have posted V3 for this patch (https://lore.kernel.org/all/20220622135556.2185991-1-j.granados@samsung.com/), but am still unsure if I have addressed this concern. We can continue discussing in the V3 thread if you still see that something needs to be changed Best Joel > > > > > > + if (cs_indep) > > > + ret = nvme_update_ns_info_cs_indep(ns, indep_id); > > > + else > > > + ret = nvme_update_ns_info(ns, id); > > > > And here we should do a switch on ids->csi ala: > > > > switch (ids.csi) { > > case NVME_CSI_NVM: > > case NVME_CSI_ZNS: > > ret = nvme_update_ns_info_block(ns, id); > > break > > default: > > nvme_update_ns_info_generic(ns, iid); > > break; > > } > > > Will adjust for V3 > > Joel [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-06-22 14:11 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20220607134309eucas1p1724cee80a5bc116c652e91422d474ccd@eucas1p1.samsung.com>
2022-06-07 13:40 ` [PATCH v2 0/2] nvme: Enable generic interface (/dev/ngX) for unknown command sets Joel Granados
2022-06-07 13:40 ` [PATCH v2 1/2] nvme-multipath: refactor nvme_mpath_add_disk Joel Granados
2022-06-13 18:16 ` Christoph Hellwig
2022-06-15 7:50 ` Joel Granados
2022-06-07 13:40 ` [PATCH v2 2/2] nvme: enable generic interface (/dev/ngXnY) for unknown command sets Joel Granados
2022-06-13 18:24 ` Christoph Hellwig
2022-06-15 11:20 ` Joel Granados
2022-06-15 14:50 ` Joel Granados
2022-06-22 14:11 ` Joel Granados
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox