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

[Javier González]

On 24.02.2021 17:44, Christoph Hellwig wrote:
>> +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..

Will do.

>
>>  	/* 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.

Ok.

>
>> +	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?

Ok.
>
>> -	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.

Sounds good.

>
>> +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.

Perfect.

Will send a new version.