[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