On 6/21/21 8:38 AM, Christoph Hellwig wrote:

On Wed, Jun 09, 2021 at 05:01:18PM +0200, Hannes Reinecke wrote:

quoted

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 177cae44b612..6d7c2958b3e2 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c

@@ -566,6 +566,9 @@ static void nvme_free_ns_head(struct kref *ref)
  	struct nvme_ns_head *head =
  		container_of(ref, struct nvme_ns_head, ref);
  
+	mutex_lock(&head->subsys->lock);
+	list_del_init(&head->entry);
+	mutex_unlock(&head->subsys->lock);
  	nvme_mpath_remove_disk(head);
  	ida_simple_remove(&head->subsys->ns_ida, head->instance);
  	cleanup_srcu_struct(&head->srcu);

@@ -3806,8 +3809,6 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
   out_unlink_ns:
  	mutex_lock(&ctrl->subsys->lock);
  	list_del_rcu(&ns->siblings);
-	if (list_empty(&ns->head->list))
-		list_del_init(&ns->head->entry);
  	mutex_unlock(&ctrl->subsys->lock);
  	nvme_put_ns_head(ns->head);
   out_free_queue:

@@ -3828,8 +3829,6 @@ 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))
-		list_del_init(&ns->head->entry);
  	mutex_unlock(&ns->ctrl->subsys->lock);
  
  	synchronize_rcu(); /* guarantee not available in head->list */

@@ -3849,7 +3848,7 @@ static void nvme_ns_remove(struct nvme_ns *ns)
  	list_del_init(&ns->list);
  	up_write(&ns->ctrl->namespaces_rwsem);
  
-	nvme_mpath_check_last_path(ns);
+	nvme_mpath_check_last_path(ns->head);
  	nvme_put_ns(ns);
  }
  

diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 23573fe3fc7d..31153f6ec582 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c

@@ -266,6 +266,8 @@ inline struct nvme_ns *nvme_find_path(struct nvme_ns_head *head)
  	int node = numa_node_id();
  	struct nvme_ns *ns;
  
+	if (!(head->disk->flags & GENHD_FL_UP))
+		return NULL;
  	ns = srcu_dereference(head->current_path[node], &head->srcu);
  	if (unlikely(!ns))
  		return __nvme_find_path(head, node);

@@ -281,6 +283,8 @@ static bool nvme_available_path(struct nvme_ns_head *head)
  {
  	struct nvme_ns *ns;
  
+	if (!(head->disk->flags & GENHD_FL_UP))
+		return false;
  	list_for_each_entry_rcu(ns, &head->list, siblings) {
  		if (test_bit(NVME_CTRL_FAILFAST_EXPIRED, &ns->ctrl->flags))
  			continue;

@@ -771,20 +775,36 @@ void nvme_mpath_add_disk(struct nvme_ns *ns, struct nvme_id_ns *id)
  #endif
  }
  
-void nvme_mpath_remove_disk(struct nvme_ns_head *head)
+void nvme_mpath_check_last_path(struct nvme_ns_head *head)
  {
+	bool last_path = false;
  	if (!head->disk)
  		return;
+
+	/* Synchronize with nvme_init_ns_head() */
+	mutex_lock(&head->subsys->lock);
+	if (list_empty(&head->list))
+		last_path = true;
+	mutex_unlock(&head->subsys->lock);
+	if (last_path) {
+		kblockd_schedule_work(&head->requeue_work);
+		if (head->disk->flags & GENHD_FL_UP) {
+			nvme_cdev_del(&head->cdev, &head->cdev_device);
+			del_gendisk(head->disk);
+		}
  	}
+}
+
+void nvme_mpath_remove_disk(struct nvme_ns_head *head)
+{
+	if (!head->disk)
+		return;
  	blk_set_queue_dying(head->disk->queue);
  	/* make sure all pending bios are cleaned up */
  	kblockd_schedule_work(&head->requeue_work);
  	flush_work(&head->requeue_work);
  	blk_cleanup_queue(head->disk->queue);
-	if (!test_bit(NVME_NSHEAD_DISK_LIVE, &head->flags)) {
+	if (!test_and_clear_bit(NVME_NSHEAD_DISK_LIVE, &head->flags)) {
  		/*
  		 * if device_add_disk wasn't called, prevent
  		 * disk release to put a bogus reference on the

So if a nvme_mpath_set_live comes in between nvme_mpath_check_last_path
and nvme_mpath_remove_disk we'll end up without a gendisk still, don't we?

I can't see how we can end up there.
If we call nvme_mpath_set_live() we will have to have a reference on 
ns_head, consequently nvme_mpath_remove_disk() will not be called for 
that ns_head as that function is called from the kref callback once all 
references are dropped.

Am I missing something?

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

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme