[PATCH 02/19] nvme: introduce a namespace count in the ns head structure

John Garry posted 19 patches 1 month, 1 week ago
[PATCH 02/19] nvme: introduce a namespace count in the ns head structure
Posted by John Garry 1 month, 1 week ago
For switching to use libmultipath, the per-namespace sibling list entry in
nvme_ns.sibling will be replaced with multipath_device.sibling list
pointer.

For when CONFIG_LIBMULTIPATH is disabled, that list of namespaces would no
longer be maintained.

However the core code checks in many places whether there is any
namespace in the head list, like in nvme_ns_remove().

Introduce a separate count of the number of namespaces for the namespace
head and use that count for the places where the per-namespace head list
of namespaces is checked to be empty.

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 drivers/nvme/host/core.c      | 10 +++++++---
 drivers/nvme/host/multipath.c |  4 ++--
 drivers/nvme/host/nvme.h      |  1 +
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 37e30caff4149..76249871dd7c2 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -4024,7 +4024,7 @@ static int nvme_init_ns_head(struct nvme_ns *ns, struct nvme_ns_info *info)
 	} else {
 		ret = -EINVAL;
 		if ((!info->is_shared || !head->shared) &&
-		    !list_empty(&head->list)) {
+		    head->ns_count) {
 			dev_err(ctrl->device,
 				"Duplicate unshared namespace %d\n",
 				info->nsid);
@@ -4047,6 +4047,7 @@ static int nvme_init_ns_head(struct nvme_ns *ns, struct nvme_ns_info *info)
 	}
 
 	list_add_tail_rcu(&ns->siblings, &head->list);
+	head->ns_count++;
 	ns->head = head;
 	mutex_unlock(&ctrl->subsys->lock);
 
@@ -4192,7 +4193,8 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
  out_unlink_ns:
 	mutex_lock(&ctrl->subsys->lock);
 	list_del_rcu(&ns->siblings);
-	if (list_empty(&ns->head->list)) {
+	ns->head->ns_count--;
+	if (!ns->head->ns_count) {
 		list_del_init(&ns->head->entry);
 		/*
 		 * If multipath is not configured, we still create a namespace
@@ -4217,6 +4219,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
 
 static void nvme_ns_remove(struct nvme_ns *ns)
 {
+	struct nvme_ns_head *head = ns->head;
 	bool last_path = false;
 
 	if (test_and_set_bit(NVME_NS_REMOVING, &ns->flags))
@@ -4238,7 +4241,8 @@ static void nvme_ns_remove(struct nvme_ns *ns)
 
 	mutex_lock(&ns->ctrl->subsys->lock);
 	list_del_rcu(&ns->siblings);
-	if (list_empty(&ns->head->list)) {
+	head->ns_count--;
+	if (!head->ns_count) {
 		if (!nvme_mpath_queue_if_no_path(ns->head))
 			list_del_init(&ns->head->entry);
 		last_path = true;
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index c70fff58b5698..0d540749b16ee 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -703,7 +703,7 @@ static void nvme_remove_head_work(struct work_struct *work)
 	bool remove = false;
 
 	mutex_lock(&head->subsys->lock);
-	if (list_empty(&head->list)) {
+	if (!head->ns_count) {
 		list_del_init(&head->entry);
 		remove = true;
 	}
@@ -1307,7 +1307,7 @@ void nvme_mpath_remove_disk(struct nvme_ns_head *head)
 	 * head->list here. If it is no longer empty then we skip enqueuing the
 	 * delayed head removal work.
 	 */
-	if (!list_empty(&head->list))
+	if (head->ns_count)
 		goto out;
 
 	if (head->delayed_removal_secs) {
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 057d051ef925d..397e8685f6c38 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -539,6 +539,7 @@ struct nvme_ns_head {
 	struct nvme_effects_log *effects;
 	u64			nuse;
 	unsigned		ns_id;
+	int			ns_count;
 	int			instance;
 #ifdef CONFIG_BLK_DEV_ZONED
 	u64			zsze;
-- 
2.43.5
Re: [PATCH 02/19] nvme: introduce a namespace count in the ns head structure
Posted by Nilay Shroff 1 month ago
On 2/25/26 9:09 PM, John Garry wrote:
> For switching to use libmultipath, the per-namespace sibling list entry in
> nvme_ns.sibling will be replaced with multipath_device.sibling list
> pointer.
> 
> For when CONFIG_LIBMULTIPATH is disabled, that list of namespaces would no
> longer be maintained.
> 
> However the core code checks in many places whether there is any
> namespace in the head list, like in nvme_ns_remove().
> 
> Introduce a separate count of the number of namespaces for the namespace
> head and use that count for the places where the per-namespace head list
> of namespaces is checked to be empty.
> 
> Signed-off-by: John Garry<john.g.garry@oracle.com>
> ---
>   drivers/nvme/host/core.c      | 10 +++++++---
>   drivers/nvme/host/multipath.c |  4 ++--
>   drivers/nvme/host/nvme.h      |  1 +
>   3 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 37e30caff4149..76249871dd7c2 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -4024,7 +4024,7 @@ static int nvme_init_ns_head(struct nvme_ns *ns, struct nvme_ns_info *info)
>   	} else {
>   		ret = -EINVAL;
>   		if ((!info->is_shared || !head->shared) &&
> -		    !list_empty(&head->list)) {
> +		    head->ns_count) {
>   			dev_err(ctrl->device,
>   				"Duplicate unshared namespace %d\n",
>   				info->nsid);
> @@ -4047,6 +4047,7 @@ static int nvme_init_ns_head(struct nvme_ns *ns, struct nvme_ns_info *info)
>   	}
>   
>   	list_add_tail_rcu(&ns->siblings, &head->list);
> +	head->ns_count++;
>   	ns->head = head;
>   	mutex_unlock(&ctrl->subsys->lock);
>   

I think we could still access head->mpath_disk->mpath_head->dev_list.
So in that case do we really need to have ->ns_count? Moreover, if
we could maintain a pointer to struct mpath_head from struct 
nvme_ns_head then we may avoid one dereference. What do you think?

Thanks,
--Nilay
Re: [PATCH 02/19] nvme: introduce a namespace count in the ns head structure
Posted by John Garry 1 month ago
On 02/03/2026 12:46, Nilay Shroff wrote:
>>
>> Signed-off-by: John Garry<john.g.garry@oracle.com>
>> ---
>>   drivers/nvme/host/core.c      | 10 +++++++---
>>   drivers/nvme/host/multipath.c |  4 ++--
>>   drivers/nvme/host/nvme.h      |  1 +
>>   3 files changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index 37e30caff4149..76249871dd7c2 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -4024,7 +4024,7 @@ static int nvme_init_ns_head(struct nvme_ns *ns, 
>> struct nvme_ns_info *info)
>>       } else {
>>           ret = -EINVAL;
>>           if ((!info->is_shared || !head->shared) &&
>> -            !list_empty(&head->list)) {
>> +            head->ns_count) {
>>               dev_err(ctrl->device,
>>                   "Duplicate unshared namespace %d\n",
>>                   info->nsid);
>> @@ -4047,6 +4047,7 @@ static int nvme_init_ns_head(struct nvme_ns *ns, 
>> struct nvme_ns_info *info)
>>       }
>>       list_add_tail_rcu(&ns->siblings, &head->list);
>> +    head->ns_count++;
>>       ns->head = head;
>>       mutex_unlock(&ctrl->subsys->lock);
> 
> I think we could still access head->mpath_disk->mpath_head->dev_list.
> So in that case do we really need to have ->ns_count? 

As mentioned, if CONFIG_NVME_MULTIPATH is disabled, mpath_head->dev_list 
is not maintained. So we need another method to set NS count in the core 
code.

> Moreover, if
> we could maintain a pointer to struct mpath_head from struct 
> nvme_ns_head then we may avoid one dereference. What do you think?

I think that it should be ok. I was just trying to reduce pointer 
declaration (as they need to be maintained).

Thanks!