[RFC PATCH] nvme: use NS head lock to serialize access to NS list

[John Garry]

Currently when accessing the NS head list of sibling NSes, we lock
using the subsystem lock.

Change to serialize by locking with the NS head lock, which is more
appropriate.

Signed-off-by: John Garry <john.g.garry at oracle.com>
---
I'm setting this as an RFC as I think that other NS head members - like
delayed sec removal - could also use NS head lock to serialise, but
I'm waiting for feedback on this change first. I also have not tested
heavily, either.

About using subsystem lock for accessing the NS list, I assume that it
was done this way as the NS head lock was later introduced (after
subsys lock).

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 82446b9d30941..a1054b3e06106 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -4019,13 +4019,16 @@ static int nvme_init_ns_head(struct nvme_ns *ns, struct nvme_ns_info *info)
 		}
 	} else {
 		ret = -EINVAL;
+		mutex_lock(&head->lock);
 		if ((!info->is_shared || !head->shared) &&
 		    !list_empty(&head->list)) {
+			mutex_unlock(&head->lock);
 			dev_err(ctrl->device,
 				"Duplicate unshared namespace %d\n",
 				info->nsid);
 			goto out_put_ns_head;
 		}
+		mutex_unlock(&head->lock);
 		if (!nvme_ns_ids_equal(&head->ids, &info->ids)) {
 			dev_err(ctrl->device,
 				"IDs don't match for shared namespace %d\n",
@@ -4041,10 +4044,12 @@ static int nvme_init_ns_head(struct nvme_ns *ns, struct nvme_ns_info *info)
 				"Shared namespace support requires core_nvme.multipath=Y.\n");
 		}
 	}
+	mutex_unlock(&ctrl->subsys->lock);
 
+	mutex_lock(&head->lock);
 	list_add_tail_rcu(&ns->siblings, &head->list);
 	ns->head = head;
-	mutex_unlock(&ctrl->subsys->lock);
+	mutex_unlock(&head->lock);
 
 #ifdef CONFIG_NVME_MULTIPATH
 	cancel_delayed_work(&head->remove_work);
@@ -4193,7 +4198,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
 	mutex_unlock(&ctrl->namespaces_lock);
 	synchronize_srcu(&ctrl->srcu);
  out_unlink_ns:
-	mutex_lock(&ctrl->subsys->lock);
+	mutex_lock(&ns->head->lock);
 	list_del_rcu(&ns->siblings);
 	if (list_empty(&ns->head->list)) {
 		list_del_init(&ns->head->entry);
@@ -4208,7 +4213,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
 		if (ns->head->disk)
 			last_path = true;
 	}
-	mutex_unlock(&ctrl->subsys->lock);
+	mutex_unlock(&ns->head->lock);
 	if (last_path)
 		nvme_put_ns_head(ns->head);
 	nvme_put_ns_head(ns->head);
@@ -4239,14 +4244,14 @@ static void nvme_ns_remove(struct nvme_ns *ns)
 	if (nvme_mpath_clear_current_path(ns))
 		synchronize_srcu(&ns->head->srcu);
 
-	mutex_lock(&ns->ctrl->subsys->lock);
+	mutex_lock(&ns->head->lock);
 	list_del_rcu(&ns->siblings);
 	if (list_empty(&ns->head->list)) {
 		if (!nvme_mpath_queue_if_no_path(ns->head))
 			list_del_init(&ns->head->entry);
 		last_path = true;
 	}
-	mutex_unlock(&ns->ctrl->subsys->lock);
+	mutex_unlock(&ns->head->lock);
 
 	/* guarantee not available in head->list */
 	synchronize_srcu(&ns->head->srcu);
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 174027d1cc198..4b2b8fc5638dd 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -702,12 +702,12 @@ static void nvme_remove_head_work(struct work_struct *work)
 			struct nvme_ns_head, remove_work);
 	bool remove = false;
 
-	mutex_lock(&head->subsys->lock);
+	mutex_lock(&head->lock);
 	if (list_empty(&head->list)) {
 		list_del_init(&head->entry);
 		remove = true;
 	}
-	mutex_unlock(&head->subsys->lock);
+	mutex_unlock(&head->lock);
 	if (remove)
 		nvme_remove_head(head);
 
@@ -1297,7 +1297,7 @@ void nvme_mpath_remove_disk(struct nvme_ns_head *head)
 	if (!head->disk)
 		return;
 
-	mutex_lock(&head->subsys->lock);
+	mutex_lock(&head->lock);
 	/*
 	 * We are called when all paths have been removed, and at that point
 	 * head->list is expected to be empty. However, nvme_remove_ns() and
@@ -1324,7 +1324,7 @@ void nvme_mpath_remove_disk(struct nvme_ns_head *head)
 		remove = true;
 	}
 out:
-	mutex_unlock(&head->subsys->lock);
+	mutex_unlock(&head->lock);
 	if (remove)
 		nvme_remove_head(head);
 }
-- 
2.43.5