[PATCH 2/2] nvme: Expose cntrltype and dctype through sysfs.

Sagi Grimberg sagi at grimberg.me
Thu Feb 3 05:08:11 PST 2022


>>>> From: Martin Belanger <martin.belanger at dell.com>
>>>>
>>>> TP8010 introduces the Discovery Controller Type (dctype) attribute.
>>>> The dctype is returned in the response to the Identify command. This
>>>> patch exposes the dctype through the sysfs. Since the dctype depends
>>>> on the Controller Type (cntrltype), another attribute of the Identify
>>>> response, the patch also exposes the cntrltype as well.
>>>>
>>>> Signed-off-by: Martin Belanger <martin.belanger at dell.com>
>>>> ---
>>>>    drivers/nvme/host/core.c | 35
>>> +++++++++++++++++++++++++++++++++++
>>>>    drivers/nvme/host/nvme.h |  3 +++
>>>>    include/linux/nvme.h     | 10 +++++++++-
>>>>    3 files changed, 47 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index
>>>> 872c389dc6d3..c65d44fb65c4 100644
>>>> --- a/drivers/nvme/host/core.c
>>>> +++ b/drivers/nvme/host/core.c
>>>> @@ -2992,6 +2992,9 @@ static int nvme_init_identify(struct nvme_ctrl
>>> *ctrl)
>>>>        ctrl->max_namespaces = le32_to_cpu(id->mnan);
>>>>        ctrl->ctratt = le32_to_cpu(id->ctratt);
>>>>
>>>> +    ctrl->cntrltype = id->cntrltype;
>>>> +    ctrl->dctype = id->dctype;
>>>> +
>>>>        if (id->rtd3e) {
>>>>            /* us -> s */
>>>>            u32 transition_time = le32_to_cpu(id->rtd3e) /
>>> USEC_PER_SEC; @@
>>>> -3525,6 +3528,34 @@ static ssize_t
>>> nvme_ctrl_fast_io_fail_tmo_store(struct device *dev,
>>>>    static DEVICE_ATTR(fast_io_fail_tmo, S_IRUGO | S_IWUSR,
>>>>        nvme_ctrl_fast_io_fail_tmo_show,
>>> nvme_ctrl_fast_io_fail_tmo_store);
>>>>
>>>> +static ssize_t nvme_ctrl_cntrltype_show(struct device *dev,
>>>> +                     struct device_attribute *attr, char *buf) {
>>>> +    struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
>>>> +
>>>> +    switch (ctrl->cntrltype) {
>>>> +    case NVME_CTRL_IO:    return sysfs_emit(buf, "io\n");
>>>> +    case NVME_CTRL_DISC:    return sysfs_emit(buf, "disc\n");
>>>
>>> 'discovery', please. 'disc' is misleading here.
>>>
>>>> +    case NVME_CTRL_ADMIN:    return sysfs_emit(buf, "admin\n");
>>>> +    default:        return sysfs_emit(buf, "reserved\n");
>>>> +    }
>>>> +}
>>>> +static DEVICE_ATTR(cntrltype, S_IRUGO, nvme_ctrl_cntrltype_show,
>>>> +NULL);
>>>> +
>>>> +static ssize_t nvme_ctrl_dctype_show(struct device *dev,
>>>> +                     struct device_attribute *attr, char *buf) {
>>>> +    struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
>>>> +
>>>> +    switch (ctrl->dctype) {
>>>> +    case NVME_DCTYPE_NOT_REPORTED:    return sysfs_emit(buf,
>>> "nr\n");
>>>
>>> Please use 'none' here; 'nr' is ambiguous ('Number'?)
>>
>> Thanks for the review. I will apply the changes.
>>
>> Question: Since this was part of a set of patches, do I need to resubmit
>> the whole set or can I just resubmit this one patch?
>>
> Whole series, please. And gather up the reviews when resubmitting.
> 
>> Also, Sagi mentioned that the dctype should only be exposed for
>> Discovery Controllers, which makes sense. However, I found that
>> when  nvme_dev_attrs_are_visible() is invoked the ctrl object is
>> not fully configured yet (i.e. the controller is not yet identified).
>> Is there any other way to determine the controller type prior to
>> receiving the response to the Identify command?
>>
> Nope, unfortunately.
> 
> But maybe we can lump both attributes together in to a single ctrltype;
> then we would have 'io', 'admin', 'discovery', 'discovery-cdc', and 
> 'discovery-ddc'.
> 
> Hmm?

We should really move the ctrl device node creation to
nvme_init_ctrl_finish. I think this came up before...



More information about the Linux-nvme mailing list