nvme: add namespace paths links

PATCH In review Hannes Reinecke 4 participants 9 messages Apr 1, 2022 – Jul 29, 2022

Subsystems (2)

NVMe Util-Linux package

lore

Share & tools

lore

Hannes Reinecke

author

2022-04-01 09:29 UTC #0 lore raw

PATCH 1/1 nvme: add namespace paths links

It is really annoying to always have to loop through the entire /sys/block directory just to find the namespace path links for a single namespace in libnvme/nvme-cli.
So provide links to the namespace paths for efficient lookup.

Signed-off-by Hannes Reinecke <[email protected]>

---
 drivers/nvme/host/core.c      | 3 +++
 drivers/nvme/host/multipath.c | 8 ++++++++
 2 files changed, 11 insertions(+)

drivers/nvme/host/core.c +3 -0

index f204c6f78b5b..df4d89aa061f 100644
@@ -4054,6 +4054,9 @@ static void nvme_ns_remove(struct nvme_ns *ns)
 
 	if (!nvme_ns_head_multipath(ns->head))
 		nvme_cdev_del(&ns->cdev, &ns->cdev_device);
+	else
+		sysfs_remove_link(&disk_to_dev(ns->head->disk)->kobj,
+				  ns->disk->disk_name);
 	del_gendisk(ns->disk);
 	blk_cleanup_queue(ns->queue);

drivers/nvme/host/multipath.c +8 -0

index 7fc58e1f6b09..dc9a4018f90d 100644
@@ -801,6 +801,8 @@ static int nvme_lookup_ana_group_desc(struct nvme_ctrl *ctrl,
 
 void nvme_mpath_add_disk(struct nvme_ns *ns, struct nvme_id_ns *id)
 {
+	int ret;
+
 	if (nvme_ctrl_use_ana(ns->ctrl)) {
 		struct nvme_ana_group_desc desc = {
 			.grpid = id->anagrpid,

@@ -831,6 +833,12 @@ void nvme_mpath_add_disk(struct nvme_ns *ns, struct nvme_id_ns *id)
 	if (blk_queue_is_zoned(ns->queue) && ns->head->disk)
 		ns->head->disk->queue->nr_zones = ns->queue->nr_zones;
 #endif
+	ret = sysfs_create_link(&disk_to_dev(ns->head->disk)->kobj,
+				&disk_to_dev(ns->disk)->kobj,
+				ns->disk->disk_name);
+	if (ret)
+		dev_warn(ns->ctrl->device,
+			 "failed to create namespace path link\n");
 }
 
 void nvme_mpath_shutdown_disk(struct nvme_ns_head *head)

Signature · 1 line

2.29.2

Christoph Hellwig

maintainer

2022-04-05 05:42 UTC #1 lore raw

Quoted 4 lines · Hannes Reinecke

It is really annoying to always have to loop through the entire /sys/block directory just to find the namespace path links for a single namespace in libnvme/nvme-cli.
So provide links to the namespace paths for efficient lookup.

I think having some form of links would be useful. Quite a while ago Thadeu looked into adding some form of relationship for lsblk and friends. Maybe it would be good to sync up and make sure whatever links we are adding would be useful for all users?

Quoted 49 lines

Signed-off-by Hannes Reinecke <[email protected]>

---
 drivers/nvme/host/core.c      | 3 +++
 drivers/nvme/host/multipath.c | 8 ++++++++
 2 files changed, 11 insertions(+)

drivers/nvme/host/core.c +3 -0

index f204c6f78b5b..df4d89aa061f 100644
@@ -4054,6 +4054,9 @@ static void nvme_ns_remove(struct nvme_ns *ns)
 
 	if (!nvme_ns_head_multipath(ns->head))
 		nvme_cdev_del(&ns->cdev, &ns->cdev_device);
+	else
+		sysfs_remove_link(&disk_to_dev(ns->head->disk)->kobj,
+				  ns->disk->disk_name);
 	del_gendisk(ns->disk);
 	blk_cleanup_queue(ns->queue);

drivers/nvme/host/multipath.c +8 -0

index 7fc58e1f6b09..dc9a4018f90d 100644
@@ -801,6 +801,8 @@ static int nvme_lookup_ana_group_desc(struct nvme_ctrl *ctrl,
 
 void nvme_mpath_add_disk(struct nvme_ns *ns, struct nvme_id_ns *id)
 {
+	int ret;
+
 	if (nvme_ctrl_use_ana(ns->ctrl)) {
 		struct nvme_ana_group_desc desc = {
 			.grpid = id->anagrpid,

@@ -831,6 +833,12 @@ void nvme_mpath_add_disk(struct nvme_ns *ns, struct nvme_id_ns *id)
 	if (blk_queue_is_zoned(ns->queue) && ns->head->disk)
 		ns->head->disk->queue->nr_zones = ns->queue->nr_zones;
 #endif
+	ret = sysfs_create_link(&disk_to_dev(ns->head->disk)->kobj,
+				&disk_to_dev(ns->disk)->kobj,
+				ns->disk->disk_name);
+	if (ret)
+		dev_warn(ns->ctrl->device,
+			 "failed to create namespace path link\n");
 }
 
 void nvme_mpath_shutdown_disk(struct nvme_ns_head *head)

Signature · 1 line

2.29.2

---end quoted text---

Hannes Reinecke

author

2022-04-05 05:46 UTC #2 lore raw

Quoted 11 lines · Christoph Hellwig

Quoted 4 lines · Hannes Reinecke
It is really annoying to always have to loop through the entire /sys/block directory just to find the namespace path links for a single namespace in libnvme/nvme-cli.
So provide links to the namespace paths for efficient lookup.
I think having some form of links would be useful. Quite a while ago Thadeu looked into adding some form of relationship for lsblk and friends. Maybe it would be good to sync up and make sure whatever links we are adding would be useful for all users?

Care to elaborate?
Using the block device name is the most straightforward way here; we don't have a good enumeration to leverage as the paths are essentially just a linked list.

We sure can have a subdirectory 'paths', and stick the links in there. If that helps with lsblk ...

Cheers,

Hannes

Signature · 4 lines

Dr. Hannes Reinecke                Kernel Storage Architect
[email protected]                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

Christoph Hellwig

maintainer

2022-04-05 05:48 UTC #3 lore raw

Quoted 12 lines · Hannes Reinecke

Quoted 11 lines · Christoph Hellwig
Quoted 4 lines · Hannes Reinecke
It is really annoying to always have to loop through the entire /sys/block directory just to find the namespace path links for a single namespace in libnvme/nvme-cli.
So provide links to the namespace paths for efficient lookup.
I think having some form of links would be useful. Quite a while ago Thadeu looked into adding some form of relationship for lsblk and friends. Maybe it would be good to sync up and make sure whatever links we are adding would be useful for all users?

Quoted 7 lines

Care to elaborate?
Using the block device name is the most straightforward way here; we don't have a good enumeration to leverage as the paths are essentially just a linked list.
We sure can have a subdirectory 'paths', and stick the links in there. If that helps with lsblk ...

I personally don't care. But I've added Thadeu and the util-linux list to get everyone on board.

Sagi Grimberg

maintainer

2022-04-05 08:34 UTC #4 lore raw

Quoted 16 lines

Quoted 16 lines
Quoted 11 lines
Quoted 5 lines
It is really annoying to always have to loop through the entire /sys/block directory just to find the namespace path links for a single namespace in libnvme/nvme-cli.
So provide links to the namespace paths for efficient lookup.
I think having some form of links would be useful. Quite a while ago Thadeu looked into adding some form of relationship for lsblk and friends. Maybe it would be good to sync up and make sure whatever links we are adding would be useful for all users?
Care to elaborate?
Using the block device name is the most straightforward way here; we don't have a good enumeration to leverage as the paths are essentially just a linked list.

Quoted 5 lines

Quoted 2 lines
We sure can have a subdirectory 'paths', and stick the links in there. If that helps with lsblk ...
I personally don't care. But I've added Thadeu and the util-linux list to get everyone on board.

Why not stick to slaves/ like the device mapper?

Hannes Reinecke

author

2022-04-05 09:51 UTC #5 lore raw

Quoted 16 lines · Sagi Grimberg

Quoted 16 lines
Quoted 16 lines
Quoted 11 lines
Quoted 5 lines
It is really annoying to always have to loop through the entire /sys/block directory just to find the namespace path links for a single namespace in libnvme/nvme-cli.
So provide links to the namespace paths for efficient lookup.
I think having some form of links would be useful. Quite a while ago Thadeu looked into adding some form of relationship for lsblk and friends. Maybe it would be good to sync up and make sure whatever links we are adding would be useful for all users?
Care to elaborate?
Using the block device name is the most straightforward way here; we don't have a good enumeration to leverage as the paths are essentially just a linked list.

Quoted 7 lines

Quoted 5 lines
Quoted 2 lines
We sure can have a subdirectory 'paths', and stick the links in there. If that helps with lsblk ...
I personally don't care. But I've added Thadeu and the util-linux list to get everyone on board.
Why not stick to slaves/ like the device mapper?

Because:

/**
* bd_link_disk_holder - create symlinks between holding disk and slave
bdev
* @bdev: the claimed slave bdev
* @disk: the holding disk
*
* DON'T USE THIS UNLESS YOU'RE ALREADY USING IT.
*

Not sure _why_ it's there, but it is there.

Cheers,

Hannes

Signature · 4 lines

Dr. Hannes Reinecke		           Kernel Storage Architect
[email protected]			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer

Sagi Grimberg

maintainer

2022-04-05 10:22 UTC #6 lore raw

Quoted 14 lines

Quoted 2 lines
Why not stick to slaves/ like the device mapper?
Because:
/**
* bd_link_disk_holder - create symlinks between holding disk and slave
bdev
* @bdev: the claimed slave bdev
* @disk: the holding disk
*
* DON'T USE THIS UNLESS YOU'RE ALREADY USING IT.
*

Quoted 1 line

Not sure _why_ it's there, but it is there.

Fair enough

Karel Zak

maintainer

2022-04-25 13:54 UTC #7 lore raw

In reply to Christoph Hellwig

Quoted 18 lines · Christoph Hellwig

Quoted 17 lines · Hannes Reinecke
Quoted 11 lines · Christoph Hellwig
Quoted 4 lines · Hannes Reinecke
It is really annoying to always have to loop through the entire /sys/block directory just to find the namespace path links for a single namespace in libnvme/nvme-cli.
So provide links to the namespace paths for efficient lookup.
I think having some form of links would be useful. Quite a while ago Thadeu looked into adding some form of relationship for lsblk and friends. Maybe it would be good to sync up and make sure whatever links we are adding would be useful for all users?
Care to elaborate?
Using the block device name is the most straightforward way here; we don't have a good enumeration to leverage as the paths are essentially just a linked list.

Quoted 5 lines

Quoted 2 lines
We sure can have a subdirectory 'paths', and stick the links in there. If that helps with lsblk ...
I personally don't care. But I've added Thadeu and the util-linux list to get everyone on board.

lsblk internally can accept arbitrary relations between devices (and libsmartcols can display it). If you introduce something new or extend the current holders/slaves, it will probably be accessible for lsblk to use it.

Karel

Signature · 2 lines

 Karel Zak  <[email protected]>
 http://karelzak.blogspot.com

Hannes Reinecke

author

2022-07-29 11:43 UTC #8 lore raw

Quoted 22 lines · Karel Zak

Quoted 21 lines · Christoph Hellwig
Quoted 19 lines · Hannes Reinecke
Quoted 11 lines · Christoph Hellwig
Quoted 4 lines · Hannes Reinecke
It is really annoying to always have to loop through the entire /sys/block directory just to find the namespace path links for a single namespace in libnvme/nvme-cli.
So provide links to the namespace paths for efficient lookup.
I think having some form of links would be useful. Quite a while ago Thadeu looked into adding some form of relationship for lsblk and friends. Maybe it would be good to sync up and make sure whatever links we are adding would be useful for all users?
Care to elaborate?
Using the block device name is the most straightforward way here; we don't have a good enumeration to leverage as the paths are essentially just a linked list.
We sure can have a subdirectory 'paths', and stick the links in there. If that helps with lsblk ...

Quoted 8 lines

Quoted 2 lines
I personally don't care. But I've added Thadeu and the util-linux list to get everyone on board.
lsblk internally can accept arbitrary relations between devices (and libsmartcols can display it). If you introduce something new or extend the current holders/slaves, it will probably be accessible for lsblk to use it.

Christoph, ping?

Shall I resend the patch or can you pick it up directly?

Cheers,

Hannes

Signature · 4 lines

Dr. Hannes Reinecke		           Kernel Storage Architect
[email protected]			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer