* [PATCH] nvme: add namespace paths links
@ 2022-04-01 9:28 Hannes Reinecke
2022-04-05 5:42 ` Christoph Hellwig
0 siblings, 1 reply; 18+ messages in thread
From: Hannes Reinecke @ 2022-04-01 9:28 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Sagi Grimberg, Keith Busch, linux-nvme, 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.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/nvme/host/core.c | 3 +++
drivers/nvme/host/multipath.c | 8 ++++++++
2 files changed, 11 insertions(+)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f204c6f78b5b..df4d89aa061f 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -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);
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 7fc58e1f6b09..dc9a4018f90d 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -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)
--
2.29.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] nvme: add namespace paths links
2022-04-01 9:28 [PATCH] nvme: add namespace paths links Hannes Reinecke
@ 2022-04-05 5:42 ` Christoph Hellwig
2022-04-05 5:46 ` Hannes Reinecke
0 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2022-04-05 5:42 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme,
util-linux, Thadeu Lima de Souza Cascardo
On Fri, Apr 01, 2022 at 11:28:55AM +0200, Hannes Reinecke wrote:
> 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?
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
> drivers/nvme/host/core.c | 3 +++
> drivers/nvme/host/multipath.c | 8 ++++++++
> 2 files changed, 11 insertions(+)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index f204c6f78b5b..df4d89aa061f 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -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);
>
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index 7fc58e1f6b09..dc9a4018f90d 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -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)
> --
> 2.29.2
---end quoted text---
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] nvme: add namespace paths links
2022-04-05 5:42 ` Christoph Hellwig
@ 2022-04-05 5:46 ` Hannes Reinecke
2022-04-05 5:48 ` Christoph Hellwig
0 siblings, 1 reply; 18+ messages in thread
From: Hannes Reinecke @ 2022-04-05 5:46 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Sagi Grimberg, Keith Busch, linux-nvme, util-linux,
Thadeu Lima de Souza Cascardo
On 4/5/22 07:42, Christoph Hellwig wrote:
> On Fri, Apr 01, 2022 at 11:28:55AM +0200, Hannes Reinecke wrote:
>> 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
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] nvme: add namespace paths links
2022-04-05 5:46 ` Hannes Reinecke
@ 2022-04-05 5:48 ` Christoph Hellwig
2022-04-05 8:34 ` Sagi Grimberg
2022-04-25 13:54 ` Karel Zak
0 siblings, 2 replies; 18+ messages in thread
From: Christoph Hellwig @ 2022-04-05 5:48 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme,
util-linux, Thadeu Lima de Souza Cascardo
On Tue, Apr 05, 2022 at 07:46:40AM +0200, Hannes Reinecke wrote:
> On 4/5/22 07:42, Christoph Hellwig wrote:
>> On Fri, Apr 01, 2022 at 11:28:55AM +0200, Hannes Reinecke wrote:
>>> 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 ...
I personally don't care. But I've added Thadeu and the util-linux list
to get everyone on board.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] nvme: add namespace paths links
2022-04-05 5:48 ` Christoph Hellwig
@ 2022-04-05 8:34 ` Sagi Grimberg
2022-04-05 9:51 ` Hannes Reinecke
2022-04-25 13:54 ` Karel Zak
1 sibling, 1 reply; 18+ messages in thread
From: Sagi Grimberg @ 2022-04-05 8:34 UTC (permalink / raw)
To: Christoph Hellwig, Hannes Reinecke
Cc: Keith Busch, linux-nvme, util-linux, Thadeu Lima de Souza Cascardo
>>>> 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 ...
>
> 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?
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] nvme: add namespace paths links
2022-04-05 8:34 ` Sagi Grimberg
@ 2022-04-05 9:51 ` Hannes Reinecke
2022-04-05 10:22 ` Sagi Grimberg
0 siblings, 1 reply; 18+ messages in thread
From: Hannes Reinecke @ 2022-04-05 9:51 UTC (permalink / raw)
To: Sagi Grimberg, Christoph Hellwig
Cc: Keith Busch, linux-nvme, util-linux, Thadeu Lima de Souza Cascardo
On 4/5/22 10:34, Sagi Grimberg wrote:
>
>>>>> 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 ...
>>
>> 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
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] nvme: add namespace paths links
2022-04-05 9:51 ` Hannes Reinecke
@ 2022-04-05 10:22 ` Sagi Grimberg
0 siblings, 0 replies; 18+ messages in thread
From: Sagi Grimberg @ 2022-04-05 10:22 UTC (permalink / raw)
To: Hannes Reinecke, Christoph Hellwig
Cc: Keith Busch, linux-nvme, util-linux, Thadeu Lima de Souza Cascardo
>> 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.
Fair enough
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] nvme: add namespace paths links
2022-04-05 5:48 ` Christoph Hellwig
2022-04-05 8:34 ` Sagi Grimberg
@ 2022-04-25 13:54 ` Karel Zak
2022-07-29 11:43 ` Hannes Reinecke
1 sibling, 1 reply; 18+ messages in thread
From: Karel Zak @ 2022-04-25 13:54 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Hannes Reinecke, Sagi Grimberg, Keith Busch, linux-nvme,
util-linux, Thadeu Lima de Souza Cascardo
On Tue, Apr 05, 2022 at 07:48:36AM +0200, Christoph Hellwig wrote:
> On Tue, Apr 05, 2022 at 07:46:40AM +0200, Hannes Reinecke wrote:
> > On 4/5/22 07:42, Christoph Hellwig wrote:
> >> On Fri, Apr 01, 2022 at 11:28:55AM +0200, Hannes Reinecke wrote:
> >>> 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 ...
>
> 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
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] nvme: add namespace paths links
2022-04-25 13:54 ` Karel Zak
@ 2022-07-29 11:43 ` Hannes Reinecke
0 siblings, 0 replies; 18+ messages in thread
From: Hannes Reinecke @ 2022-07-29 11:43 UTC (permalink / raw)
To: Karel Zak, Christoph Hellwig
Cc: Sagi Grimberg, Keith Busch, linux-nvme, util-linux,
Thadeu Lima de Souza Cascardo
On 4/25/22 15:54, Karel Zak wrote:
> On Tue, Apr 05, 2022 at 07:48:36AM +0200, Christoph Hellwig wrote:
>> On Tue, Apr 05, 2022 at 07:46:40AM +0200, Hannes Reinecke wrote:
>>> On 4/5/22 07:42, Christoph Hellwig wrote:
>>>> On Fri, Apr 01, 2022 at 11:28:55AM +0200, Hannes Reinecke wrote:
>>>>> 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 ...
>>
>> 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
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] nvme: add namespace paths links
2023-12-04 11:58 ` Sagi Grimberg
@ 2023-12-08 15:34 ` Daniel Wagner
0 siblings, 0 replies; 18+ messages in thread
From: Daniel Wagner @ 2023-12-08 15:34 UTC (permalink / raw)
To: Sagi Grimberg; +Cc: Hannes Reinecke, Christoph Hellwig, Keith Busch, linux-nvme
On Mon, Dec 04, 2023 at 01:58:47PM +0200, Sagi Grimberg wrote:
> Sadly, no. Everyone said "Yeah, good idea, but let's wait for the
> > others." I _dearly_ would love to have it as it will simplify the
> > logic in nvme-cli/libnvme drastically.
> > Shall I repost?
>
> Yes, and I personally don't mind having it, so if no one voices
> any concern, I don't see why we should prevent it.
I'd like to get the sysfs link too, so no concerns from my side.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] nvme: add namespace paths links
2023-12-04 11:19 ` Hannes Reinecke
@ 2023-12-04 11:58 ` Sagi Grimberg
2023-12-08 15:34 ` Daniel Wagner
0 siblings, 1 reply; 18+ messages in thread
From: Sagi Grimberg @ 2023-12-04 11:58 UTC (permalink / raw)
To: Hannes Reinecke, Christoph Hellwig; +Cc: Keith Busch, linux-nvme
>>>>>>> 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 <hare@suse.de>
>>>>>>> ---
>>>>>>> drivers/nvme/host/core.c | 3 +++
>>>>>>> drivers/nvme/host/multipath.c | 10 ++++++++++
>>>>>>> 2 files changed, 13 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>>>>>>> index 37b6fa746662..88933838e067 100644
>>>>>>> --- a/drivers/nvme/host/core.c
>>>>>>> +++ b/drivers/nvme/host/core.c
>>>>>>> @@ -3681,6 +3681,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);
>>>>>>> down_write(&ns->ctrl->namespaces_rwsem);
>>>>>>> diff --git a/drivers/nvme/host/multipath.c
>>>>>>> b/drivers/nvme/host/multipath.c
>>>>>>> index 0a88d7bdc5e3..9dd0be645dad 100644
>>>>>>> --- a/drivers/nvme/host/multipath.c
>>>>>>> +++ b/drivers/nvme/host/multipath.c
>>>>>>> @@ -875,6 +875,16 @@ void nvme_mpath_add_disk(struct nvme_ns *ns,
>>>>>>> __le32 anagrpid)
>>>>>>> if (blk_queue_is_zoned(ns->queue) && ns->head->disk)
>>>>>>> ns->head->disk->nr_zones = ns->disk->nr_zones;
>>>>>>> #endif
>>>>>>> + if (ns->head->disk) {
>>>>>>
>>>>>> Why are the two conditions not identical?
>>>>>>
>>>>> Which two?
>>>>
>>>> ns->head->disk and nvme_ns_head_multipath()
>>>>
>>> How can we have one without the other? I thought that both are
>>> mutually dependent .. but yeah, we should be using
>>> 'nvme_ns_head_multipath()' here.
>>>
>>>>>
>>>>>>> + int ret;
>>>>>>> +
>>>>>>> + 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)
>>>>>>
>>>>>> What will these links be used for?
>>>>>
>>>>> Mapping for nvme-cli, as explained in the description.
>>>>> Clearly it doesn't do it _now_, but once the patch is in we can
>>>>> simplify nvme-cli to take advantage of it.
>>>>
>>>> you mean for nvme list -v ?
>>>>
>>>> Don't really mind having it, but would want to hear from others as
>>>> well.
>>>
>>> Ok.
>>
>> Hey Hannes,
>> Has this gone anywhere?
>
> Sadly, no. Everyone said "Yeah, good idea, but let's wait for the
> others." I _dearly_ would love to have it as it will simplify the
> logic in nvme-cli/libnvme drastically.
> Shall I repost?
Yes, and I personally don't mind having it, so if no one voices
any concern, I don't see why we should prevent it.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] nvme: add namespace paths links
2023-12-04 10:25 ` Sagi Grimberg
@ 2023-12-04 11:19 ` Hannes Reinecke
2023-12-04 11:58 ` Sagi Grimberg
0 siblings, 1 reply; 18+ messages in thread
From: Hannes Reinecke @ 2023-12-04 11:19 UTC (permalink / raw)
To: Sagi Grimberg, Christoph Hellwig; +Cc: Keith Busch, linux-nvme
On 12/4/23 11:25, Sagi Grimberg wrote:
>
>
> On 9/12/23 17:04, Hannes Reinecke wrote:
>> On 9/12/23 14:55, Sagi Grimberg wrote:
>>>
>>>>>> 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 <hare@suse.de>
>>>>>> ---
>>>>>> drivers/nvme/host/core.c | 3 +++
>>>>>> drivers/nvme/host/multipath.c | 10 ++++++++++
>>>>>> 2 files changed, 13 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>>>>>> index 37b6fa746662..88933838e067 100644
>>>>>> --- a/drivers/nvme/host/core.c
>>>>>> +++ b/drivers/nvme/host/core.c
>>>>>> @@ -3681,6 +3681,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);
>>>>>> down_write(&ns->ctrl->namespaces_rwsem);
>>>>>> diff --git a/drivers/nvme/host/multipath.c
>>>>>> b/drivers/nvme/host/multipath.c
>>>>>> index 0a88d7bdc5e3..9dd0be645dad 100644
>>>>>> --- a/drivers/nvme/host/multipath.c
>>>>>> +++ b/drivers/nvme/host/multipath.c
>>>>>> @@ -875,6 +875,16 @@ void nvme_mpath_add_disk(struct nvme_ns *ns,
>>>>>> __le32 anagrpid)
>>>>>> if (blk_queue_is_zoned(ns->queue) && ns->head->disk)
>>>>>> ns->head->disk->nr_zones = ns->disk->nr_zones;
>>>>>> #endif
>>>>>> + if (ns->head->disk) {
>>>>>
>>>>> Why are the two conditions not identical?
>>>>>
>>>> Which two?
>>>
>>> ns->head->disk and nvme_ns_head_multipath()
>>>
>> How can we have one without the other? I thought that both are
>> mutually dependent .. but yeah, we should be using
>> 'nvme_ns_head_multipath()' here.
>>
>>>>
>>>>>> + int ret;
>>>>>> +
>>>>>> + 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)
>>>>>
>>>>> What will these links be used for?
>>>>
>>>> Mapping for nvme-cli, as explained in the description.
>>>> Clearly it doesn't do it _now_, but once the patch is in we can
>>>> simplify nvme-cli to take advantage of it.
>>>
>>> you mean for nvme list -v ?
>>>
>>> Don't really mind having it, but would want to hear from others as
>>> well.
>>
>> Ok.
>
> Hey Hannes,
> Has this gone anywhere?
Sadly, no. Everyone said "Yeah, good idea, but let's wait for the
others." I _dearly_ would love to have it as it will simplify the
logic in nvme-cli/libnvme drastically.
Shall I repost?
Cheers,
Hannes
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] nvme: add namespace paths links
2023-09-12 14:04 ` Hannes Reinecke
@ 2023-12-04 10:25 ` Sagi Grimberg
2023-12-04 11:19 ` Hannes Reinecke
0 siblings, 1 reply; 18+ messages in thread
From: Sagi Grimberg @ 2023-12-04 10:25 UTC (permalink / raw)
To: Hannes Reinecke, Christoph Hellwig; +Cc: Keith Busch, linux-nvme
On 9/12/23 17:04, Hannes Reinecke wrote:
> On 9/12/23 14:55, Sagi Grimberg wrote:
>>
>>>>> 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 <hare@suse.de>
>>>>> ---
>>>>> drivers/nvme/host/core.c | 3 +++
>>>>> drivers/nvme/host/multipath.c | 10 ++++++++++
>>>>> 2 files changed, 13 insertions(+)
>>>>>
>>>>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>>>>> index 37b6fa746662..88933838e067 100644
>>>>> --- a/drivers/nvme/host/core.c
>>>>> +++ b/drivers/nvme/host/core.c
>>>>> @@ -3681,6 +3681,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);
>>>>> down_write(&ns->ctrl->namespaces_rwsem);
>>>>> diff --git a/drivers/nvme/host/multipath.c
>>>>> b/drivers/nvme/host/multipath.c
>>>>> index 0a88d7bdc5e3..9dd0be645dad 100644
>>>>> --- a/drivers/nvme/host/multipath.c
>>>>> +++ b/drivers/nvme/host/multipath.c
>>>>> @@ -875,6 +875,16 @@ void nvme_mpath_add_disk(struct nvme_ns *ns,
>>>>> __le32 anagrpid)
>>>>> if (blk_queue_is_zoned(ns->queue) && ns->head->disk)
>>>>> ns->head->disk->nr_zones = ns->disk->nr_zones;
>>>>> #endif
>>>>> + if (ns->head->disk) {
>>>>
>>>> Why are the two conditions not identical?
>>>>
>>> Which two?
>>
>> ns->head->disk and nvme_ns_head_multipath()
>>
> How can we have one without the other? I thought that both are mutually
> dependent .. but yeah, we should be using 'nvme_ns_head_multipath()' here.
>
>>>
>>>>> + int ret;
>>>>> +
>>>>> + 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)
>>>>
>>>> What will these links be used for?
>>>
>>> Mapping for nvme-cli, as explained in the description.
>>> Clearly it doesn't do it _now_, but once the patch is in we can
>>> simplify nvme-cli to take advantage of it.
>>
>> you mean for nvme list -v ?
>>
>> Don't really mind having it, but would want to hear from others as
>> well.
>
> Ok.
Hey Hannes,
Has this gone anywhere?
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] nvme: add namespace paths links
2023-09-12 12:55 ` Sagi Grimberg
@ 2023-09-12 14:04 ` Hannes Reinecke
2023-12-04 10:25 ` Sagi Grimberg
0 siblings, 1 reply; 18+ messages in thread
From: Hannes Reinecke @ 2023-09-12 14:04 UTC (permalink / raw)
To: Sagi Grimberg, Christoph Hellwig; +Cc: Keith Busch, linux-nvme
On 9/12/23 14:55, Sagi Grimberg wrote:
>
>>>> 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 <hare@suse.de>
>>>> ---
>>>> drivers/nvme/host/core.c | 3 +++
>>>> drivers/nvme/host/multipath.c | 10 ++++++++++
>>>> 2 files changed, 13 insertions(+)
>>>>
>>>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>>>> index 37b6fa746662..88933838e067 100644
>>>> --- a/drivers/nvme/host/core.c
>>>> +++ b/drivers/nvme/host/core.c
>>>> @@ -3681,6 +3681,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);
>>>> down_write(&ns->ctrl->namespaces_rwsem);
>>>> diff --git a/drivers/nvme/host/multipath.c
>>>> b/drivers/nvme/host/multipath.c
>>>> index 0a88d7bdc5e3..9dd0be645dad 100644
>>>> --- a/drivers/nvme/host/multipath.c
>>>> +++ b/drivers/nvme/host/multipath.c
>>>> @@ -875,6 +875,16 @@ void nvme_mpath_add_disk(struct nvme_ns *ns,
>>>> __le32 anagrpid)
>>>> if (blk_queue_is_zoned(ns->queue) && ns->head->disk)
>>>> ns->head->disk->nr_zones = ns->disk->nr_zones;
>>>> #endif
>>>> + if (ns->head->disk) {
>>>
>>> Why are the two conditions not identical?
>>>
>> Which two?
>
> ns->head->disk and nvme_ns_head_multipath()
>
How can we have one without the other? I thought that both are mutually
dependent .. but yeah, we should be using 'nvme_ns_head_multipath()' here.
>>
>>>> + int ret;
>>>> +
>>>> + 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)
>>>
>>> What will these links be used for?
>>
>> Mapping for nvme-cli, as explained in the description.
>> Clearly it doesn't do it _now_, but once the patch is in we can
>> simplify nvme-cli to take advantage of it.
>
> you mean for nvme list -v ?
>
> Don't really mind having it, but would want to hear from others as
> well.
Ok.
Cheers,
Hannes
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] nvme: add namespace paths links
2023-09-12 11:51 ` Hannes Reinecke
@ 2023-09-12 12:55 ` Sagi Grimberg
2023-09-12 14:04 ` Hannes Reinecke
0 siblings, 1 reply; 18+ messages in thread
From: Sagi Grimberg @ 2023-09-12 12:55 UTC (permalink / raw)
To: Hannes Reinecke, Christoph Hellwig; +Cc: Keith Busch, linux-nvme
>>> 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 <hare@suse.de>
>>> ---
>>> drivers/nvme/host/core.c | 3 +++
>>> drivers/nvme/host/multipath.c | 10 ++++++++++
>>> 2 files changed, 13 insertions(+)
>>>
>>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>>> index 37b6fa746662..88933838e067 100644
>>> --- a/drivers/nvme/host/core.c
>>> +++ b/drivers/nvme/host/core.c
>>> @@ -3681,6 +3681,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);
>>> down_write(&ns->ctrl->namespaces_rwsem);
>>> diff --git a/drivers/nvme/host/multipath.c
>>> b/drivers/nvme/host/multipath.c
>>> index 0a88d7bdc5e3..9dd0be645dad 100644
>>> --- a/drivers/nvme/host/multipath.c
>>> +++ b/drivers/nvme/host/multipath.c
>>> @@ -875,6 +875,16 @@ void nvme_mpath_add_disk(struct nvme_ns *ns,
>>> __le32 anagrpid)
>>> if (blk_queue_is_zoned(ns->queue) && ns->head->disk)
>>> ns->head->disk->nr_zones = ns->disk->nr_zones;
>>> #endif
>>> + if (ns->head->disk) {
>>
>> Why are the two conditions not identical?
>>
> Which two?
ns->head->disk and nvme_ns_head_multipath()
>
>>> + int ret;
>>> +
>>> + 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)
>>
>> What will these links be used for?
>
> Mapping for nvme-cli, as explained in the description.
> Clearly it doesn't do it _now_, but once the patch is in we can
> simplify nvme-cli to take advantage of it.
you mean for nvme list -v ?
Don't really mind having it, but would want to hear from others as
well.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] nvme: add namespace paths links
2023-09-12 11:43 ` Sagi Grimberg
@ 2023-09-12 11:51 ` Hannes Reinecke
2023-09-12 12:55 ` Sagi Grimberg
0 siblings, 1 reply; 18+ messages in thread
From: Hannes Reinecke @ 2023-09-12 11:51 UTC (permalink / raw)
To: Sagi Grimberg, Christoph Hellwig; +Cc: Keith Busch, linux-nvme
On 9/12/23 13:43, Sagi Grimberg wrote:
>
>> 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 <hare@suse.de>
>> ---
>> drivers/nvme/host/core.c | 3 +++
>> drivers/nvme/host/multipath.c | 10 ++++++++++
>> 2 files changed, 13 insertions(+)
>>
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index 37b6fa746662..88933838e067 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -3681,6 +3681,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);
>> down_write(&ns->ctrl->namespaces_rwsem);
>> diff --git a/drivers/nvme/host/multipath.c
>> b/drivers/nvme/host/multipath.c
>> index 0a88d7bdc5e3..9dd0be645dad 100644
>> --- a/drivers/nvme/host/multipath.c
>> +++ b/drivers/nvme/host/multipath.c
>> @@ -875,6 +875,16 @@ void nvme_mpath_add_disk(struct nvme_ns *ns,
>> __le32 anagrpid)
>> if (blk_queue_is_zoned(ns->queue) && ns->head->disk)
>> ns->head->disk->nr_zones = ns->disk->nr_zones;
>> #endif
>> + if (ns->head->disk) {
>
> Why are the two conditions not identical?
>
Which two?
>> + int ret;
>> +
>> + 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)
>
> What will these links be used for?
Mapping for nvme-cli, as explained in the description.
Clearly it doesn't do it _now_, but once the patch is in we can
simplify nvme-cli to take advantage of it.
Cheers,
Hannes
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] nvme: add namespace paths links
2023-09-08 6:57 Hannes Reinecke
@ 2023-09-12 11:43 ` Sagi Grimberg
2023-09-12 11:51 ` Hannes Reinecke
0 siblings, 1 reply; 18+ messages in thread
From: Sagi Grimberg @ 2023-09-12 11:43 UTC (permalink / raw)
To: Hannes Reinecke, Christoph Hellwig; +Cc: Keith Busch, linux-nvme
> 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 <hare@suse.de>
> ---
> drivers/nvme/host/core.c | 3 +++
> drivers/nvme/host/multipath.c | 10 ++++++++++
> 2 files changed, 13 insertions(+)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 37b6fa746662..88933838e067 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -3681,6 +3681,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);
>
> down_write(&ns->ctrl->namespaces_rwsem);
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index 0a88d7bdc5e3..9dd0be645dad 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -875,6 +875,16 @@ void nvme_mpath_add_disk(struct nvme_ns *ns, __le32 anagrpid)
> if (blk_queue_is_zoned(ns->queue) && ns->head->disk)
> ns->head->disk->nr_zones = ns->disk->nr_zones;
> #endif
> + if (ns->head->disk) {
Why are the two conditions not identical?
> + int ret;
> +
> + 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)
What will these links be used for?
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] nvme: add namespace paths links
@ 2023-09-08 6:57 Hannes Reinecke
2023-09-12 11:43 ` Sagi Grimberg
0 siblings, 1 reply; 18+ messages in thread
From: Hannes Reinecke @ 2023-09-08 6:57 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Sagi Grimberg, Keith Busch, linux-nvme, 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.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
drivers/nvme/host/core.c | 3 +++
drivers/nvme/host/multipath.c | 10 ++++++++++
2 files changed, 13 insertions(+)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 37b6fa746662..88933838e067 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3681,6 +3681,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);
down_write(&ns->ctrl->namespaces_rwsem);
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 0a88d7bdc5e3..9dd0be645dad 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -875,6 +875,16 @@ void nvme_mpath_add_disk(struct nvme_ns *ns, __le32 anagrpid)
if (blk_queue_is_zoned(ns->queue) && ns->head->disk)
ns->head->disk->nr_zones = ns->disk->nr_zones;
#endif
+ if (ns->head->disk) {
+ int ret;
+
+ 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)
--
2.35.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2023-12-08 15:35 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-01 9:28 [PATCH] nvme: add namespace paths links Hannes Reinecke
2022-04-05 5:42 ` Christoph Hellwig
2022-04-05 5:46 ` Hannes Reinecke
2022-04-05 5:48 ` Christoph Hellwig
2022-04-05 8:34 ` Sagi Grimberg
2022-04-05 9:51 ` Hannes Reinecke
2022-04-05 10:22 ` Sagi Grimberg
2022-04-25 13:54 ` Karel Zak
2022-07-29 11:43 ` Hannes Reinecke
2023-09-08 6:57 Hannes Reinecke
2023-09-12 11:43 ` Sagi Grimberg
2023-09-12 11:51 ` Hannes Reinecke
2023-09-12 12:55 ` Sagi Grimberg
2023-09-12 14:04 ` Hannes Reinecke
2023-12-04 10:25 ` Sagi Grimberg
2023-12-04 11:19 ` Hannes Reinecke
2023-12-04 11:58 ` Sagi Grimberg
2023-12-08 15:34 ` Daniel Wagner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.