Skip to contentSkip to conversation

NVMe: eliminate potential deadlock by nvme_get_ns_from_disk invoking nvme_free_ns

PATCH Superseded Wang Sheng-Hui 3 participants 4 messages May 3, 2016 NVMe

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Wang Sheng-Hui

author
#0 lore raw

PATCH 1/1 NVMe: eliminate potential deadlock by nvme_get_ns_from_disk invoking nvme_free_ns

Release dev_list_lock before enter nvme_free_ns from nvme_get_ns_from_disk to avoid potential deadlock.

Signed-off-by Wang Sheng-Hui <shhuiw at foxmail.com>
---
 drivers/nvme/host/core.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)
drivers/nvme/host/core.c +10 -11
index 49bc4c0..81e397b 100644
@@ -87,20 +87,19 @@ static struct nvme_ns *nvme_get_ns_from_disk(struct gendisk *disk)
 	spin_lock(&dev_list_lock);
 	ns = disk->private_data;
 	if (ns) {
-		if (!kref_get_unless_zero(&ns->kref))
-			goto fail;
-		if (!try_module_get(ns->ctrl->ops->module))
-			goto fail_put_ns;
+		if (!kref_get_unless_zero(&ns->kref)) {
+			ns = NULL;
+			goto out;
+		}
+		if (!try_module_get(ns->ctrl->ops->module)) {
+			spin_unlock(&dev_list_lock);
+			kref_put(&ns->kref, nvme_free_ns);
+			return NULL;
+		}
 	}
+out:
 	spin_unlock(&dev_list_lock);
-
 	return ns;
-
-fail_put_ns:
-	kref_put(&ns->kref, nvme_free_ns);
-fail:
-	spin_unlock(&dev_list_lock);
-	return NULL;
 }
 
 void nvme_requeue_req(struct request *req)
Signature · 1 line
2.7.4
maintainer
#1 lore raw

PATCH 1/1 NVMe: eliminate potential deadlock by nvme_get_ns_from_disk invoking nvme_free_ns

Quoted 4 lines · Wang Sheng-Hui

Release dev_list_lock before enter nvme_free_ns from nvme_get_ns_from_disk to avoid potential deadlock.

Signed-off-by Wang Sheng-Hui <shhuiw at foxmail.com>

Looks fine,

Reviewed-by Christoph Hellwig <hch at lst.de>
#2 lore raw

PATCH 1/1 NVMe: eliminate potential deadlock by nvme_get_ns_from_disk invoking nvme_free_ns

In reply to Wang Sheng-Hui
Quoted 26 lines

-----Original Message-----
From: Wang Sheng-Hui [mailto:shhuiw at foxmail.com]
Sent: Tuesday, May 03, 2016 2:26 AM
To: hch at lst.de; sagig at mellanox.com; Busch, Keith; axboe at fb.com; linux-nvme at lists.infradead.org
Subject: [PATCH] NVMe: eliminate potential deadlock by nvme_get_ns_from_disk invoking nvme_free_ns

Release dev_list_lock before enter nvme_free_ns from nvme_get_ns_from_disk to avoid potential deadlock.

Signed-off-by Wang Sheng-Hui <shhuiw at foxmail.com>
---
 drivers/nvme/host/core.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)
drivers/nvme/host/core.c +0 -4
index 49bc4c0..81e397b 100644
@@ -87,20 +87,19 @@ static struct nvme_ns *nvme_get_ns_from_disk(struct gendisk *disk)
 	spin_lock(&dev_list_lock);
 	ns = disk->private_data;
 	if (ns) {
-		if (!kref_get_unless_zero(&ns->kref))
-			goto fail;
-		if (!try_module_get(ns->ctrl->ops->module))
-			goto fail_put_ns;

drivers/nvme/host/core.c static struct nvme_ns *nvme_get_ns_from_disk(struct gendisk *disk) · line 99

Quoted 10 lines
drivers/nvme/host/core.c +9 -0
+		if (!kref_get_unless_zero(&ns->kref)) {
+			ns = NULL;
+			goto out;
+		}
+		if (!try_module_get(ns->ctrl->ops->module)) {
+			spin_unlock(&dev_list_lock);
+			kref_put(&ns->kref, nvme_free_ns);
+			return NULL;
+		}
 	}

Swapping the ns and module reference gets order would have been simpler.

maintainer
#3 lore raw

PATCH 1/1 NVMe: eliminate potential deadlock by nvme_get_ns_from_disk invoking nvme_free_ns

Quoted 1 line · Keith

Swapping the ns and module reference gets order would have been simpler.

That's fine with me, too.