Re: [PATCH V5 1/2] nvme: enable char device per namespace

[Christoph Hellwig <hch@lst.de>]

> +static inline bool nvme_dev_is_generic(struct device *dev)
> +{
> +	return dev->class == nvme_ns_class;
> +}
> +
> +static inline bool nvme_ns_is_generic(struct nvme_ns *ns)
> +{
> +	return !!ns->minor;
> +}

What does is_generic mean here?  In doubt add a few comments..

>  	/* some standard values */
> @@ -2241,6 +2270,13 @@ static int nvme_update_ns_info(struct nvme_ns *ns, struct nvme_id_ns *id)
>  	return 0;
>  
>  out_unfreeze:
> +	/*
> +	 * When the device does not support any of the features required by the
> +	 * kernel (or viceversa), hide the block device. We can still rely on
> +	 * the namespace char device for submitting IOCTLs
> +	 */
> +	ns->disk->flags |= GENHD_FL_HIDDEN;
> +

The out_unfreeze case also handles all kinds of real error, so this needs
to move into a better spot, and probably check a specific error code
or even explicit indicator.

> +	struct nvme_ns *ns = container_of(inode->i_cdev, struct nvme_ns, cdev);
> +{
> +	struct nvme_ns *ns = container_of(inode->i_cdev, struct nvme_ns, cdev);

> +	struct nvme_ns *ns = container_of(file->f_inode->i_cdev,
> +				struct nvme_ns, cdev);

Maybe add a little cdev_to_ns() helper?

> -	if (nvme_update_ns_info(ns, id))
> -		goto out_put_disk;
> +	nvme_update_ns_info(ns, id);

I don't think we can simplify ignore all errors here.

> +static inline struct nvme_ns *nvme_get_ns_from_cdev(struct device *dev)
> +{
> +	return dev_get_drvdata(dev);
> +}

I think we can keep this in core.c.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme