NVMe: Expose namespace unique identifier to sysfs
PATCH 1/1 NVMe: Expose namespace unique identifier to sysfs
A controller namespace supporting 1.1 or 1.2 capabilities uniquely identifies itself with either the 64-bit EUI or 128-bit NGUID.
This patch determines which the device supports and reports the unique identifier in new sysfs binary attribute "uuid". The attribute group is added to the gendisk's kobject directory.
No fallback is provided for controllers not reporting a unique id.
--- drivers/nvme/host/core.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++-- drivers/nvme/host/nvme.h | 2 ++ 2 files changed, 55 insertions(+), 2 deletions(-)
drivers/nvme/host/core.c +53 -2
@@ -574,6 +574,21 @@ static int nvme_revalidate_disk(struct gendisk *disk) ns->type =NVME_NS_LIGHTNVM ; } +if (ns->ctrl->vs >=NVME_VS (1 ,1 )) { +u8 *identifier = id->eui64; +int len =sizeof (id->eui64); + +if (bitmap_empty ((void *)identifier, len *8 ) && + ns->ctrl->vs >=NVME_VS (1 ,2 )) { + identifier = id->nguid; + len =sizeof (id->nguid); + } +if (!bitmap_empty ((void *)identifier, len *8 )) { +memcpy (ns->uuid, identifier, len); + ns->uuid_len = len; + } + } + old_ms = ns->ms; lbaf = id->flbas &NVME_NS_FLBAS_LBA_MASK ; ns->lba_shift = id->lbaf[lbaf].ds;
@@ -994,6 +1009,34 @@ static const struct attribute_group *nvme_dev_attr_groups[] = {NULL }; +#define nvme_bin_attr (field) \ +static ssize_t field##_show(struct file *filp, struct kobject *kobj, \ + struct bin_attribute *bin_attr, \ + char *buf, loff_t off, size_t count) \ +{ \ + struct device *dev = container_of(kobj, struct device, kobj); \ + struct nvme_ns *ns = dev_to_disk(dev)->private_data; \ + return memory_read_from_buffer(buf, count, &off, \ + ns->field, \ + ns->field##_len); \ +} \ +static struct bin_attribute dev_attr_nvme_##field = { \ + .attr = {.name = #field, .mode = S_IRUGO }, \ + .size = 0, \ + .read = field##_show, \ +}; + +nvme_bin_attr (uuid); + +static struct bin_attribute *nvme_ns_bin_attrs[] = { + &dev_attr_nvme_uuid, +NULL , +}; + +static struct attribute_group nvme_ns_attrs_group = { + .bin_attrs = nvme_ns_bin_attrs, +}; +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);
@@ -1068,9 +1111,16 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid)list_add_tail (&ns->list, &ctrl->namespaces);kref_get (&ctrl->kref); -if (ns->type !=NVME_NS_LIGHTNVM ) -add_disk (ns->disk); +if (ns->type ==NVME_NS_LIGHTNVM ) +return ; +add_disk (ns->disk); +if (sysfs_create_group (&disk_to_dev (ns->disk)->kobj, + &nvme_ns_attrs_group)) { +del_gendisk (ns->disk); +nvme_put_ctrl (ctrl); + goto out_free_queue; + }return ; out_free_disk:kfree (disk);
@@ -1090,6 +1140,7 @@ static void nvme_ns_remove(struct nvme_ns *ns)if (ns->disk->flags &GENHD_FL_UP ) {if (blk_get_integrity (ns->disk))blk_integrity_unregister (ns->disk); +sysfs_remove_group (&disk_to_dev (ns->disk)->kobj, &nvme_ns_attrs_group);del_gendisk (ns->disk); }if (kill || !blk_queue_dying (ns->queue)) {
drivers/nvme/host/nvme.h +2 -0
@@ -102,6 +102,8 @@ struct nvme_ns {struct gendisk *disk;struct kref kref; +u8 uuid_len; +u8 uuid[16 ];/* Either EUI64 or NGUID, if provided */ unsigned ns_id;int lba_shift;u16 ms;
Signature · 1 line
2.6.2.307.g37023ba
PATCH 1/1 NVMe: Expose namespace unique identifier to sysfs
Quoted 6 lines · Keith Busch
A controller namespace supporting 1.1 or 1.2 capabilities uniquely identifies itself with either the 64-bit EUI or 128-bit NGUID.
This patch determines which the device supports and reports the unique identifier in new sysfs binary attribute "uuid". The attribute group is added to the gendisk's kobject directory.
I don't understand why we want to produce a binary attribute here instead of a text attribute? We already have nicely-formatted UUIDs in sysfs (see the %pU specifier to printk)
I don't think we should have one attribute that might be an eui64 or might be a uuid. Maybe we should produce either an eui64 or a uuid attribute, depending on which one the device reports?
drivers/nvme/host/core.c static int nvme_revalidate_disk(struct gendisk *disk) · line 585
Quoted 9 lines
drivers/nvme/host/core.c +9
-0+if (ns->ctrl->vs >=NVME_VS (1 ,1 )) { +u8 *identifier = id->eui64; +int len =sizeof (id->eui64); + +if (bitmap_empty ((void *)identifier, len *8 ) && + ns->ctrl->vs >=NVME_VS (1 ,2 )) { + identifier = id->nguid; + len =sizeof (id->nguid); + }
I'm a bit reluctant to use bitmap_empty here, because it's not actually a bitmap. We almost want the inverse of memchr ("find me the first byte that is non-zero"). Maybe somebody else knows a better functoin to call here?
drivers/nvme/host/core.c static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid) · line 1126
Quoted 10 lines
drivers/nvme/host/core.c +7
-0+add_disk (ns->disk); +if (sysfs_create_group (&disk_to_dev (ns->disk)->kobj, + &nvme_ns_attrs_group)) { +del_gendisk (ns->disk); +nvme_put_ctrl (ctrl); + goto out_free_queue; + }return ; out_free_disk:kfree (disk);
An interesting philosophical point ... if creating the group fails, should we really refuse to use the namespace? It seems to me that we can use it just fine.
PATCH 1/1 NVMe: Expose namespace unique identifier to sysfs
Quoted 8 lines · Matthew Wilcox
Quoted 3 lines · Keith Busch
This patch determines which the device supports and reports the unique identifier in new sysfs binary attribute "uuid". The attribute group is added to the gendisk's kobject directory.
I don't understand why we want to produce a binary attribute here instead of a text attribute? We already have nicely-formatted UUIDs in sysfs (see the %pU specifier to printk)
I don't have a strong opinion here. Just copying scsi vpd83 attribute, and thought it's easier for a program to consume as binary, and easy to read from the shell with hexdump.
Quoted 3 lines
I don't think we should have one attribute that might be an eui64 or might be a uuid. Maybe we should produce either an eui64 or a uuid attribute, depending on which one the device reports?
Sure, sounds reasonable.
These are supposed to be consumed by userspace to do something useful, but I'm not getting much direction (I don't know why they insist on these being available in sysfs instead of using existing ioctl).
I've no problem dynamically providing these based on device capabilities. Hopefully the user space is okay with looking for the presense of two different files.
Quoted 6 lines
drivers/nvme/host/core.c static int nvme_revalidate_disk(struct gendisk *disk) · line 581
Quoted 1 line
drivers/nvme/host/core.c +1
-0+ if (bitmap_empty((void *)identifier, len * 8) &&I'm a bit reluctant to use
bitmap_emptyhere, because it's not actually a bitmap. We almost want the inverse of memchr ("find me the first byte that is non-zero"). Maybe somebody else knows a better functoin to call here?
Better methods are learned all the time! It was nearly a year ago today you suggested bitmap_empty for checking the very same fields. :)
http://lists.infradead.org/pipermail/linux-nvme/2014-December/001349.html
PATCH 1/1 NVMe: Expose namespace unique identifier to sysfs
Quoted 8 lines
Quoted 4 lines
I'm a bit reluctant to use
bitmap_emptyhere, because it's not actually a bitmap. We almost want the inverse of memchr ("find me the first byte that is non-zero"). Maybe somebody else knows a better functoin to call here?Better methods are learned all the time! It was nearly a year ago today you suggested
bitmap_emptyfor checking the very same fields. :)
How about memchr_inv? I think Matthew was alluding to that one
PATCH 1/1 NVMe: Expose namespace unique identifier to sysfs
In reply to Keith BuschQuoted 7 lines · Keith Busch
Quoted 3 lines
I don't understand why we want to produce a binary attribute here instead of a text attribute? We already have nicely-formatted UUIDs in sysfs (see the %pU specifier to printk)
I don't have a strong opinion here. Just copying scsi vpd83 attribute, and thought it's easier for a program to consume as binary, and easy to read from the shell with hexdump.
SCSI is a bad example. James insisted we should not parse the EVPD pages in kernel space for reasons no one agreed to (and now we'll need to add that for in-kernel consumers anyway). So unless some userspace absolutely needs to do it the same way it might be better to offer a ASCII representation.
Quoted 9 lines
Quoted 3 lines
I don't think we should have one attribute that might be an eui64 or might be a uuid. Maybe we should produce either an eui64 or a uuid attribute, depending on which one the device reports?
Sure, sounds reasonable.
These are supposed to be consumed by userspace to do something useful, but I'm not getting much direction (I don't know why they insist on these being available in sysfs instead of using existing ioctl).
Using pass through ioctls from udev has been really painful from SCSI, mostly in multipath environments or with flakey USB devices where we'd need to extend a lot of magic done in the SCSI layer to those userspace callers to get everything right. NVMe is probably doing better in that respect at the moment, but I'm not going to take any bets that it's going to remain that way.
Quoted 3 lines
I've no problem dynamically providing these based on device capabilities. Hopefully the user space is okay with looking for the presense of two different files.
An alternative might be to show the type of ID if we're using a text format, but that sounds fairly ugly as well.
PATCH 1/1 NVMe: Expose namespace unique identifier to sysfs
In reply to Keith BuschQuoted 6 lines · Keith Busch
Quoted 5 lines · Matthew Wilcox
Quoted 3 lines · Keith Busch
This patch determines which the device supports and reports the unique identifier in new sysfs binary attribute "uuid". The attribute group is added to the gendisk's kobject directory.
Quoted 7 lines
Quoted 3 lines
I don't understand why we want to produce a binary attribute here instead of a text attribute? We already have nicely-formatted UUIDs in sysfs (see the %pU specifier to printk)
I don't have a strong opinion here. Just copying scsi vpd83 attribute, and thought it's easier for a program to consume as binary, and easy to read from the shell with hexdump.
Fwiw, the nvdimm subsystem dumps namespace uuids as text in sysfs, and libuuid makes it trivial to convert back and forth.