Skip to contentSkip to conversation

NVMe: Export NVMe attributes to sysfs group

PATCH New Keith Busch 2 participants 4 messages Dec 7, 2015 – Dec 9, 2015 NVMe

lore
Share & tools
lore

Keith Busch

author
#0 lore raw

PATCH 1/1 NVMe: Export NVMe attributes to sysfs group

Adds all controller information to attribute list exposed to sysfs, and appends the reset_controller attribute to it. The nvme device is created with this attribute list, so driver no long manages its attributes.

Reported-by Sujith Pandel <sujithpshankar at gmail.com>
Cc Sujith Pandel <sujithpshankar@ gmail.com>
Cc David Milburn <dmilburn at redhat.com>
Signed-off-by Keith Busch <keith.busch at intel.com>

---
This is based on the patch from Sujith. It is merged up to Christoph's block tree, exposes all the saved nvme controller attributes, aligns coding conventions with scsi, and simplifies sysfs attribute management by providing it at device allocation time.

 drivers/nvme/host/core.c | 44 +++++++++++++++++++++++++++++++++-----------
 1 file changed, 33 insertions(+), 11 deletions(-)
drivers/nvme/host/core.c +33 -11
index 0a7a510..cd2a64f 100644
@@ -964,6 +964,36 @@ static ssize_t nvme_sysfs_reset(struct device *dev,
 }
 static DEVICE_ATTR(reset_controller, S_IWUSR, NULL, nvme_sysfs_reset);
 
+#define nvme_show_function(field)						\
+static ssize_t  field##_show(struct device *dev,				\
+			    struct device_attribute *attr, char *buf)		\
+{										\
+        struct nvme_ctrl *ctrl = dev_get_drvdata(dev);				\
+        return sprintf(buf, "%.*s\n", (int)sizeof(ctrl->field), ctrl->field);	\
+}										\
+static DEVICE_ATTR(field, S_IRUGO, field##_show, NULL);
+
+nvme_show_function(model);
+nvme_show_function(serial);
+nvme_show_function(firmware_rev);
+
+static struct attribute *nvme_dev_attrs[] = {
+	&dev_attr_reset_controller.attr,
+	&dev_attr_model.attr,
+	&dev_attr_serial.attr,
+	&dev_attr_firmware_rev.attr,
+	NULL
+};
+
+static struct attribute_group nvme_dev_attrs_group = {
+	.attrs = nvme_dev_attrs,
+};
+
+static const struct attribute_group *nvme_dev_attr_groups[] = {
+	&nvme_dev_attrs_group,
+	NULL
+};
+
 static int ns_cmp(void *priv, struct list_head *a, struct list_head *b)
 {
 	struct nvme_ns *nsa = container_of(a, struct nvme_ns, list);

@@ -1191,7 +1221,6 @@ static void nvme_release_instance(struct nvme_ctrl *ctrl)
 
 void nvme_uninit_ctrl(struct nvme_ctrl *ctrl)
  {
-	device_remove_file(ctrl->device, &dev_attr_reset_controller);
 	device_destroy(nvme_class, MKDEV(nvme_char_major, ctrl->instance));
 
 	spin_lock(&dev_list_lock);

@@ -1234,9 +1263,10 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
 	if (ret)
 		goto out;
 
-	ctrl->device = device_create(nvme_class, ctrl->dev,
+	ctrl->device = device_create_with_groups(nvme_class, ctrl->dev,
 				MKDEV(nvme_char_major, ctrl->instance),
-				dev, "nvme%d", ctrl->instance);
+				dev, nvme_dev_attr_groups,
+				"nvme%d", ctrl->instance);
 	if (IS_ERR(ctrl->device)) {
 		ret = PTR_ERR(ctrl->device);
 		goto out_release_instance;

@@ -1244,19 +1274,11 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
 	get_device(ctrl->device);
 	dev_set_drvdata(ctrl->device, ctrl);
 
-	ret = device_create_file(ctrl->device, &dev_attr_reset_controller);
-	if (ret)
-		goto out_put_device;
-
 	spin_lock(&dev_list_lock);
 	list_add_tail(&ctrl->node, &nvme_ctrl_list);
 	spin_unlock(&dev_list_lock);
 
 	return 0;
-
-out_put_device:
-	put_device(ctrl->device);
-	device_destroy(nvme_class, MKDEV(nvme_char_major, ctrl->instance));
 out_release_instance:
 	nvme_release_instance(ctrl);
 out:
Signature · 1 line
2.6.2.307.g37023ba
#1 lore raw

PATCH 1/1 NVMe: Export NVMe attributes to sysfs group

Quoted 8 lines · Keith Busch

Adds all controller information to attribute list exposed to sysfs, and appends the reset_controller attribute to it. The nvme device is created with this attribute list, so driver no long manages its attributes.

Reported-by Sujith Pandel <sujithpshankar at gmail.com>
Cc Sujith Pandel <sujithpshankar@ gmail.com>
Cc David Milburn <dmilburn at redhat.com>
Signed-off-by Keith Busch <keith.busch at intel.com>
Quoted 5 lines

---
This is based on the patch from Sujith. It is merged up to Christoph's block tree, exposes all the saved nvme controller attributes, aligns coding conventions with scsi, and simplifies sysfs attribute management by providing it at device allocation time.

This looks good, BUT:

We really need to expose the NGUID on the namespace at the same time. Otherwise people will use this information to incorrectly provide a serial number of the block devices in udev because not better information is available.

author
#2 lore raw

PATCH 1/1 NVMe: Export NVMe attributes to sysfs group

Quoted 4 lines · Christoph Hellwig

We really need to expose the NGUID on the namespace at the same time. Otherwise people will use this information to incorrectly provide a serial number of the block devices in udev because not better information is available.

I'm planning to tackle that next. NGUID can not be in the same attribute group. It needs to be in a new namespace group instead.

Right now all I got is the NGUID/EUI64. Any opinion on what to name the attribute? I was going with "id". The systemd developers didn't provide a very strong opinion, and I think they were confusing "serial" as a unique identifier.

While I'm in this area, are there other namespace attributes anyone is interested in exposing?

#3 lore raw

PATCH 1/1 NVMe: Export NVMe attributes to sysfs group

Quoted 2 lines · Keith Busch

While I'm in this area, are there other namespace attributes anyone is interested in exposing?

I think the nsid would be really useful!