[PATCH v2 2/2] nvme: enable generic interface (/dev/ngXnY) for unknown command sets

[Joel Granados]

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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-nvme/attachments/20220615/5c3f4005/attachment.sig>