[PATCH 0/6] Fix deadlock in ftl formating on mtd

[PATCH 0/6] Fix deadlock in ftl formating on mtd

[Zhihao Cheng]

There is an AA-deadlock problem while formating mtd device to generate
a ftl device. Fix it by reverting "mtd: allow to unload the mtdtrans
module if its block devices aren't open" recommended in [1].

[<0>] blktrans_open+0x47/0x340            LOCK(mtd_table_mutex)
[<0>] __blkdev_get+0x5b/0x3e0
[<0>] blkdev_get_by_dev+0x18f/0x370
[<0>] __device_add_disk+0x2db/0x700
[<0>] device_add_disk+0x17/0x20
[<0>] add_mtd_blktrans_dev+0x39e/0x6d0
[<0>] ftl_add_mtd+0x792/0x908 [ftl]
[<0>] register_mtd_blktrans+0xfb/0x170    LOCK(mtd_table_mutex)
[<0>] ftl_tr_init+0x18/0x1000 [ftl]
[<0>] do_one_initcall+0x71/0x330
[<0>] do_init_module+0xa6/0x350

[1] http://lists.infradead.org/pipermail/linux-mtd/2017-March/072899.html

Zhihao Cheng (6):
  Revert "mtd: blkdevs: fix potential deadlock + lockdep warnings"
  Revert "mtd: fix: avoid race condition when accessing mtd->usecount"
  Revert "mtd: mtd_blkdevs: don't increase 'open' count on error path"
  Revert "mtd: mtd_blkdevs: fix error path in blktrans_open"
  Revert "mtd: Remove redundant mutex from mtd_blkdevs.c"
  Revert "mtd: allow to unload the mtdtrans module if its block devices
    aren't open"

 drivers/mtd/mtd_blkdevs.c | 77 +++++++++++++++------------------------
 1 file changed, 30 insertions(+), 47 deletions(-)

-- 
2.31.1

[PATCH 0/6] Fix deadlock in ftl formating on mtd

[Zhihao Cheng]

There is an AA-deadlock problem while formating mtd device to generate
a ftl device. Fix it by reverting "mtd: allow to unload the mtdtrans
module if its block devices aren't open" recommended in [1].

[<0>] blktrans_open+0x47/0x340            LOCK(mtd_table_mutex)
[<0>] __blkdev_get+0x5b/0x3e0
[<0>] blkdev_get_by_dev+0x18f/0x370
[<0>] __device_add_disk+0x2db/0x700
[<0>] device_add_disk+0x17/0x20
[<0>] add_mtd_blktrans_dev+0x39e/0x6d0
[<0>] ftl_add_mtd+0x792/0x908 [ftl]
[<0>] register_mtd_blktrans+0xfb/0x170    LOCK(mtd_table_mutex)
[<0>] ftl_tr_init+0x18/0x1000 [ftl]
[<0>] do_one_initcall+0x71/0x330
[<0>] do_init_module+0xa6/0x350

[1] http://lists.infradead.org/pipermail/linux-mtd/2017-March/072899.html

Zhihao Cheng (6):
  Revert "mtd: blkdevs: fix potential deadlock + lockdep warnings"
  Revert "mtd: fix: avoid race condition when accessing mtd->usecount"
  Revert "mtd: mtd_blkdevs: don't increase 'open' count on error path"
  Revert "mtd: mtd_blkdevs: fix error path in blktrans_open"
  Revert "mtd: Remove redundant mutex from mtd_blkdevs.c"
  Revert "mtd: allow to unload the mtdtrans module if its block devices
    aren't open"

 drivers/mtd/mtd_blkdevs.c | 77 +++++++++++++++------------------------
 1 file changed, 30 insertions(+), 47 deletions(-)

-- 
2.31.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 1/6] Revert "mtd: blkdevs: fix potential deadlock + lockdep warnings"

[Zhihao Cheng]

This reverts commit f3c63795e90f0c6238306883b6c72f14d5355721.
---
 drivers/mtd/mtd_blkdevs.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index fb8e12d590a1..16ccce3e7fdb 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -209,8 +209,8 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 	if (!dev)
 		return -ERESTARTSYS; /* FIXME: busy loop! -arnd*/
 
-	mutex_lock(&mtd_table_mutex);
 	mutex_lock(&dev->lock);
+	mutex_lock(&mtd_table_mutex);
 
 	if (dev->open)
 		goto unlock;
@@ -234,8 +234,8 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 
 unlock:
 	dev->open++;
-	mutex_unlock(&dev->lock);
 	mutex_unlock(&mtd_table_mutex);
+	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
 	return ret;
 
@@ -245,8 +245,8 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 error_put:
 	module_put(dev->tr->owner);
 	kref_put(&dev->ref, blktrans_dev_release);
-	mutex_unlock(&dev->lock);
 	mutex_unlock(&mtd_table_mutex);
+	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
 	return ret;
 }
@@ -258,8 +258,8 @@ static void blktrans_release(struct gendisk *disk, fmode_t mode)
 	if (!dev)
 		return;
 
-	mutex_lock(&mtd_table_mutex);
 	mutex_lock(&dev->lock);
+	mutex_lock(&mtd_table_mutex);
 
 	if (--dev->open)
 		goto unlock;
@@ -273,8 +273,8 @@ static void blktrans_release(struct gendisk *disk, fmode_t mode)
 		__put_mtd_device(dev->mtd);
 	}
 unlock:
-	mutex_unlock(&dev->lock);
 	mutex_unlock(&mtd_table_mutex);
+	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
 }
 
-- 
2.31.1

[PATCH 1/6] Revert "mtd: blkdevs: fix potential deadlock + lockdep warnings"

[Zhihao Cheng]

This reverts commit f3c63795e90f0c6238306883b6c72f14d5355721.
---
 drivers/mtd/mtd_blkdevs.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index fb8e12d590a1..16ccce3e7fdb 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -209,8 +209,8 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 	if (!dev)
 		return -ERESTARTSYS; /* FIXME: busy loop! -arnd*/
 
-	mutex_lock(&mtd_table_mutex);
 	mutex_lock(&dev->lock);
+	mutex_lock(&mtd_table_mutex);
 
 	if (dev->open)
 		goto unlock;
@@ -234,8 +234,8 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 
 unlock:
 	dev->open++;
-	mutex_unlock(&dev->lock);
 	mutex_unlock(&mtd_table_mutex);
+	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
 	return ret;
 
@@ -245,8 +245,8 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 error_put:
 	module_put(dev->tr->owner);
 	kref_put(&dev->ref, blktrans_dev_release);
-	mutex_unlock(&dev->lock);
 	mutex_unlock(&mtd_table_mutex);
+	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
 	return ret;
 }
@@ -258,8 +258,8 @@ static void blktrans_release(struct gendisk *disk, fmode_t mode)
 	if (!dev)
 		return;
 
-	mutex_lock(&mtd_table_mutex);
 	mutex_lock(&dev->lock);
+	mutex_lock(&mtd_table_mutex);
 
 	if (--dev->open)
 		goto unlock;
@@ -273,8 +273,8 @@ static void blktrans_release(struct gendisk *disk, fmode_t mode)
 		__put_mtd_device(dev->mtd);
 	}
 unlock:
-	mutex_unlock(&dev->lock);
 	mutex_unlock(&mtd_table_mutex);
+	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
 }
 
-- 
2.31.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 2/6] Revert "mtd: fix: avoid race condition when accessing mtd->usecount"

[Zhihao Cheng]

This reverts commit 073db4a51ee43ccb827f54a4261c0583b028d5ab.
---
 drivers/mtd/mtd_blkdevs.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 16ccce3e7fdb..00b13f71ccf3 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -210,7 +210,6 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 		return -ERESTARTSYS; /* FIXME: busy loop! -arnd*/
 
 	mutex_lock(&dev->lock);
-	mutex_lock(&mtd_table_mutex);
 
 	if (dev->open)
 		goto unlock;
@@ -234,7 +233,6 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 
 unlock:
 	dev->open++;
-	mutex_unlock(&mtd_table_mutex);
 	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
 	return ret;
@@ -245,7 +243,6 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 error_put:
 	module_put(dev->tr->owner);
 	kref_put(&dev->ref, blktrans_dev_release);
-	mutex_unlock(&mtd_table_mutex);
 	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
 	return ret;
@@ -259,7 +256,6 @@ static void blktrans_release(struct gendisk *disk, fmode_t mode)
 		return;
 
 	mutex_lock(&dev->lock);
-	mutex_lock(&mtd_table_mutex);
 
 	if (--dev->open)
 		goto unlock;
@@ -273,7 +269,6 @@ static void blktrans_release(struct gendisk *disk, fmode_t mode)
 		__put_mtd_device(dev->mtd);
 	}
 unlock:
-	mutex_unlock(&mtd_table_mutex);
 	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
 }
-- 
2.31.1

[PATCH 2/6] Revert "mtd: fix: avoid race condition when accessing mtd->usecount"

[Zhihao Cheng]

This reverts commit 073db4a51ee43ccb827f54a4261c0583b028d5ab.
---
 drivers/mtd/mtd_blkdevs.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 16ccce3e7fdb..00b13f71ccf3 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -210,7 +210,6 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 		return -ERESTARTSYS; /* FIXME: busy loop! -arnd*/
 
 	mutex_lock(&dev->lock);
-	mutex_lock(&mtd_table_mutex);
 
 	if (dev->open)
 		goto unlock;
@@ -234,7 +233,6 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 
 unlock:
 	dev->open++;
-	mutex_unlock(&mtd_table_mutex);
 	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
 	return ret;
@@ -245,7 +243,6 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 error_put:
 	module_put(dev->tr->owner);
 	kref_put(&dev->ref, blktrans_dev_release);
-	mutex_unlock(&mtd_table_mutex);
 	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
 	return ret;
@@ -259,7 +256,6 @@ static void blktrans_release(struct gendisk *disk, fmode_t mode)
 		return;
 
 	mutex_lock(&dev->lock);
-	mutex_lock(&mtd_table_mutex);
 
 	if (--dev->open)
 		goto unlock;
@@ -273,7 +269,6 @@ static void blktrans_release(struct gendisk *disk, fmode_t mode)
 		__put_mtd_device(dev->mtd);
 	}
 unlock:
-	mutex_unlock(&mtd_table_mutex);
 	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
 }
-- 
2.31.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 3/6] Revert "mtd: mtd_blkdevs: don't increase 'open' count on error path"

[Zhihao Cheng]

This reverts commit 342ff28f5a2e5aa3236617bd2bddf6c749677ef2.
---
 drivers/mtd/mtd_blkdevs.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 00b13f71ccf3..100a3c96760c 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -211,7 +211,7 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 
 	mutex_lock(&dev->lock);
 
-	if (dev->open)
+	if (dev->open++)
 		goto unlock;
 
 	kref_get(&dev->ref);
@@ -232,7 +232,6 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 	dev->file_mode = mode;
 
 unlock:
-	dev->open++;
 	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
 	return ret;
-- 
2.31.1

[PATCH 3/6] Revert "mtd: mtd_blkdevs: don't increase 'open' count on error path"

[Zhihao Cheng]

This reverts commit 342ff28f5a2e5aa3236617bd2bddf6c749677ef2.
---
 drivers/mtd/mtd_blkdevs.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 00b13f71ccf3..100a3c96760c 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -211,7 +211,7 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 
 	mutex_lock(&dev->lock);
 
-	if (dev->open)
+	if (dev->open++)
 		goto unlock;
 
 	kref_get(&dev->ref);
@@ -232,7 +232,6 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 	dev->file_mode = mode;
 
 unlock:
-	dev->open++;
 	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
 	return ret;
-- 
2.31.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 4/6] Revert "mtd: mtd_blkdevs: fix error path in blktrans_open"

[Zhihao Cheng]

This reverts commit 94735ec4044a6d318b83ad3c5794e931ed168d10.
---
 drivers/mtd/mtd_blkdevs.c | 23 +++--------------------
 1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 100a3c96760c..41dd3b955136 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -217,34 +217,17 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 	kref_get(&dev->ref);
 	__module_get(dev->tr->owner);
 
-	if (!dev->mtd)
-		goto unlock;
-
-	if (dev->tr->open) {
-		ret = dev->tr->open(dev);
-		if (ret)
-			goto error_put;
+	if (dev->mtd) {
+		ret = dev->tr->open ? dev->tr->open(dev) : 0;
+		__get_mtd_device(dev->mtd);
 	}
 
-	ret = __get_mtd_device(dev->mtd);
-	if (ret)
-		goto error_release;
 	dev->file_mode = mode;
 
 unlock:
 	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
 	return ret;
-
-error_release:
-	if (dev->tr->release)
-		dev->tr->release(dev);
-error_put:
-	module_put(dev->tr->owner);
-	kref_put(&dev->ref, blktrans_dev_release);
-	mutex_unlock(&dev->lock);
-	blktrans_dev_put(dev);
-	return ret;
 }
 
 static void blktrans_release(struct gendisk *disk, fmode_t mode)
-- 
2.31.1

[PATCH 4/6] Revert "mtd: mtd_blkdevs: fix error path in blktrans_open"

[Zhihao Cheng]

This reverts commit 94735ec4044a6d318b83ad3c5794e931ed168d10.
---
 drivers/mtd/mtd_blkdevs.c | 23 +++--------------------
 1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 100a3c96760c..41dd3b955136 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -217,34 +217,17 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 	kref_get(&dev->ref);
 	__module_get(dev->tr->owner);
 
-	if (!dev->mtd)
-		goto unlock;
-
-	if (dev->tr->open) {
-		ret = dev->tr->open(dev);
-		if (ret)
-			goto error_put;
+	if (dev->mtd) {
+		ret = dev->tr->open ? dev->tr->open(dev) : 0;
+		__get_mtd_device(dev->mtd);
 	}
 
-	ret = __get_mtd_device(dev->mtd);
-	if (ret)
-		goto error_release;
 	dev->file_mode = mode;
 
 unlock:
 	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
 	return ret;
-
-error_release:
-	if (dev->tr->release)
-		dev->tr->release(dev);
-error_put:
-	module_put(dev->tr->owner);
-	kref_put(&dev->ref, blktrans_dev_release);
-	mutex_unlock(&dev->lock);
-	blktrans_dev_put(dev);
-	return ret;
 }
 
 static void blktrans_release(struct gendisk *disk, fmode_t mode)
-- 
2.31.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 5/6] Revert "mtd: Remove redundant mutex from mtd_blkdevs.c"

[Zhihao Cheng]

This reverts commit 8cc9de3ebdf654601db51a516267dff89c49acc3.

Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 drivers/mtd/mtd_blkdevs.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 41dd3b955136..e8a77f10ae24 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -22,6 +22,7 @@
 
 #include "mtdcore.h"
 
+static DEFINE_MUTEX(mtd_blkdevs_mutex);
 static LIST_HEAD(blktrans_majors);
 static DEFINE_MUTEX(blktrans_ref_mutex);
 
@@ -209,6 +210,7 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 	if (!dev)
 		return -ERESTARTSYS; /* FIXME: busy loop! -arnd*/
 
+	mutex_lock(&mtd_blkdevs_mutex);
 	mutex_lock(&dev->lock);
 
 	if (dev->open++)
@@ -227,6 +229,7 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 unlock:
 	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
+	mutex_unlock(&mtd_blkdevs_mutex);
 	return ret;
 }
 
@@ -237,6 +240,7 @@ static void blktrans_release(struct gendisk *disk, fmode_t mode)
 	if (!dev)
 		return;
 
+	mutex_lock(&mtd_blkdevs_mutex);
 	mutex_lock(&dev->lock);
 
 	if (--dev->open)
@@ -253,6 +257,7 @@ static void blktrans_release(struct gendisk *disk, fmode_t mode)
 unlock:
 	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
+	mutex_unlock(&mtd_blkdevs_mutex);
 }
 
 static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo)
-- 
2.31.1

[PATCH 5/6] Revert "mtd: Remove redundant mutex from mtd_blkdevs.c"

[Zhihao Cheng]

This reverts commit 8cc9de3ebdf654601db51a516267dff89c49acc3.

Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 drivers/mtd/mtd_blkdevs.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 41dd3b955136..e8a77f10ae24 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -22,6 +22,7 @@
 
 #include "mtdcore.h"
 
+static DEFINE_MUTEX(mtd_blkdevs_mutex);
 static LIST_HEAD(blktrans_majors);
 static DEFINE_MUTEX(blktrans_ref_mutex);
 
@@ -209,6 +210,7 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 	if (!dev)
 		return -ERESTARTSYS; /* FIXME: busy loop! -arnd*/
 
+	mutex_lock(&mtd_blkdevs_mutex);
 	mutex_lock(&dev->lock);
 
 	if (dev->open++)
@@ -227,6 +229,7 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 unlock:
 	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
+	mutex_unlock(&mtd_blkdevs_mutex);
 	return ret;
 }
 
@@ -237,6 +240,7 @@ static void blktrans_release(struct gendisk *disk, fmode_t mode)
 	if (!dev)
 		return;
 
+	mutex_lock(&mtd_blkdevs_mutex);
 	mutex_lock(&dev->lock);
 
 	if (--dev->open)
@@ -253,6 +257,7 @@ static void blktrans_release(struct gendisk *disk, fmode_t mode)
 unlock:
 	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
+	mutex_unlock(&mtd_blkdevs_mutex);
 }
 
 static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo)
-- 
2.31.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 6/6] Revert "mtd: allow to unload the mtdtrans module if its block devices aren't open"

[Zhihao Cheng]

This reverts commit 008c751ec78587dd9b48bb62d4b10d616554fea2.

Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 drivers/mtd/mtd_blkdevs.c | 49 ++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index e8a77f10ae24..4d386617fd8a 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -205,7 +205,7 @@ static blk_status_t mtd_queue_rq(struct blk_mq_hw_ctx *hctx,
 static int blktrans_open(struct block_device *bdev, fmode_t mode)
 {
 	struct mtd_blktrans_dev *dev = blktrans_dev_get(bdev->bd_disk);
-	int ret = 0;
+	int ret;
 
 	if (!dev)
 		return -ERESTARTSYS; /* FIXME: busy loop! -arnd*/
@@ -213,17 +213,17 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 	mutex_lock(&mtd_blkdevs_mutex);
 	mutex_lock(&dev->lock);
 
-	if (dev->open++)
+	if (!dev->mtd) {
+		ret = -ENXIO;
 		goto unlock;
-
-	kref_get(&dev->ref);
-	__module_get(dev->tr->owner);
-
-	if (dev->mtd) {
-		ret = dev->tr->open ? dev->tr->open(dev) : 0;
-		__get_mtd_device(dev->mtd);
 	}
 
+	ret = !dev->open++ && dev->tr->open ? dev->tr->open(dev) : 0;
+	/* Take another reference on the device so it won't go away till
+	 * last release */
+	if (!ret)
+		kref_get(&dev->ref);
+
 	dev->file_mode = mode;
 
 unlock:
@@ -243,17 +243,14 @@ static void blktrans_release(struct gendisk *disk, fmode_t mode)
 	mutex_lock(&mtd_blkdevs_mutex);
 	mutex_lock(&dev->lock);
 
-	if (--dev->open)
-		goto unlock;
-
+	/* Release one reference, we sure its not the last one here*/
 	kref_put(&dev->ref, blktrans_dev_release);
-	module_put(dev->tr->owner);
 
-	if (dev->mtd) {
-		if (dev->tr->release)
-			dev->tr->release(dev);
-		__put_mtd_device(dev->mtd);
-	}
+	if (!dev->mtd)
+		goto unlock;
+
+	if (!--dev->open && dev->tr->release)
+		dev->tr->release(dev);
 unlock:
 	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
@@ -408,6 +405,9 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
 
 	gd->queue = new->rq;
 
+	__get_mtd_device(new->mtd);
+	__module_get(tr->owner);
+
 	if (new->readonly)
 		set_disk_ro(gd, 1);
 
@@ -456,15 +456,16 @@ int del_mtd_blktrans_dev(struct mtd_blktrans_dev *old)
 	blk_mq_unquiesce_queue(old->rq);
 	blk_mq_unfreeze_queue(old->rq);
 
-	/* If the device is currently open, tell trans driver to close it,
-		then put mtd device, and don't touch it again */
+	/* Ask trans driver for release to the mtd device */
 	mutex_lock(&old->lock);
-	if (old->open) {
-		if (old->tr->release)
-			old->tr->release(old);
-		__put_mtd_device(old->mtd);
+	if (old->open && old->tr->release) {
+		old->tr->release(old);
+		old->open = 0;
 	}
+	__put_mtd_device(old->mtd);
+	module_put(old->tr->owner);
 
+	/* At that point, we don't touch the mtd anymore */
 	old->mtd = NULL;
 
 	mutex_unlock(&old->lock);
-- 
2.31.1

[PATCH 6/6] Revert "mtd: allow to unload the mtdtrans module if its block devices aren't open"

[Zhihao Cheng]

This reverts commit 008c751ec78587dd9b48bb62d4b10d616554fea2.

Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 drivers/mtd/mtd_blkdevs.c | 49 ++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index e8a77f10ae24..4d386617fd8a 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -205,7 +205,7 @@ static blk_status_t mtd_queue_rq(struct blk_mq_hw_ctx *hctx,
 static int blktrans_open(struct block_device *bdev, fmode_t mode)
 {
 	struct mtd_blktrans_dev *dev = blktrans_dev_get(bdev->bd_disk);
-	int ret = 0;
+	int ret;
 
 	if (!dev)
 		return -ERESTARTSYS; /* FIXME: busy loop! -arnd*/
@@ -213,17 +213,17 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 	mutex_lock(&mtd_blkdevs_mutex);
 	mutex_lock(&dev->lock);
 
-	if (dev->open++)
+	if (!dev->mtd) {
+		ret = -ENXIO;
 		goto unlock;
-
-	kref_get(&dev->ref);
-	__module_get(dev->tr->owner);
-
-	if (dev->mtd) {
-		ret = dev->tr->open ? dev->tr->open(dev) : 0;
-		__get_mtd_device(dev->mtd);
 	}
 
+	ret = !dev->open++ && dev->tr->open ? dev->tr->open(dev) : 0;
+	/* Take another reference on the device so it won't go away till
+	 * last release */
+	if (!ret)
+		kref_get(&dev->ref);
+
 	dev->file_mode = mode;
 
 unlock:
@@ -243,17 +243,14 @@ static void blktrans_release(struct gendisk *disk, fmode_t mode)
 	mutex_lock(&mtd_blkdevs_mutex);
 	mutex_lock(&dev->lock);
 
-	if (--dev->open)
-		goto unlock;
-
+	/* Release one reference, we sure its not the last one here*/
 	kref_put(&dev->ref, blktrans_dev_release);
-	module_put(dev->tr->owner);
 
-	if (dev->mtd) {
-		if (dev->tr->release)
-			dev->tr->release(dev);
-		__put_mtd_device(dev->mtd);
-	}
+	if (!dev->mtd)
+		goto unlock;
+
+	if (!--dev->open && dev->tr->release)
+		dev->tr->release(dev);
 unlock:
 	mutex_unlock(&dev->lock);
 	blktrans_dev_put(dev);
@@ -408,6 +405,9 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
 
 	gd->queue = new->rq;
 
+	__get_mtd_device(new->mtd);
+	__module_get(tr->owner);
+
 	if (new->readonly)
 		set_disk_ro(gd, 1);
 
@@ -456,15 +456,16 @@ int del_mtd_blktrans_dev(struct mtd_blktrans_dev *old)
 	blk_mq_unquiesce_queue(old->rq);
 	blk_mq_unfreeze_queue(old->rq);
 
-	/* If the device is currently open, tell trans driver to close it,
-		then put mtd device, and don't touch it again */
+	/* Ask trans driver for release to the mtd device */
 	mutex_lock(&old->lock);
-	if (old->open) {
-		if (old->tr->release)
-			old->tr->release(old);
-		__put_mtd_device(old->mtd);
+	if (old->open && old->tr->release) {
+		old->tr->release(old);
+		old->open = 0;
 	}
+	__put_mtd_device(old->mtd);
+	module_put(old->tr->owner);
 
+	/* At that point, we don't touch the mtd anymore */
 	old->mtd = NULL;
 
 	mutex_unlock(&old->lock);
-- 
2.31.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 0/6] Fix deadlock in ftl formating on mtd

[Miquel Raynal]

Hi Zhihao,

Zhihao Cheng <chengzhihao1@huawei.com> wrote on Sun, 13 Jun 2021
19:30:29 +0800:

> There is an AA-deadlock problem while formating mtd device to generate
> a ftl device. Fix it by reverting "mtd: allow to unload the mtdtrans
> module if its block devices aren't open" recommended in [1].
> 
> [<0>] blktrans_open+0x47/0x340            LOCK(mtd_table_mutex)
> [<0>] __blkdev_get+0x5b/0x3e0
> [<0>] blkdev_get_by_dev+0x18f/0x370
> [<0>] __device_add_disk+0x2db/0x700
> [<0>] device_add_disk+0x17/0x20
> [<0>] add_mtd_blktrans_dev+0x39e/0x6d0
> [<0>] ftl_add_mtd+0x792/0x908 [ftl]
> [<0>] register_mtd_blktrans+0xfb/0x170    LOCK(mtd_table_mutex)
> [<0>] ftl_tr_init+0x18/0x1000 [ftl]
> [<0>] do_one_initcall+0x71/0x330
> [<0>] do_init_module+0xa6/0x350
> 
> [1] http://lists.infradead.org/pipermail/linux-mtd/2017-March/072899.html
> 
> Zhihao Cheng (6):
>   Revert "mtd: blkdevs: fix potential deadlock + lockdep warnings"
>   Revert "mtd: fix: avoid race condition when accessing mtd->usecount"
>   Revert "mtd: mtd_blkdevs: don't increase 'open' count on error path"
>   Revert "mtd: mtd_blkdevs: fix error path in blktrans_open"
>   Revert "mtd: Remove redundant mutex from mtd_blkdevs.c"
>   Revert "mtd: allow to unload the mtdtrans module if its block devices
>     aren't open"

I understand that some fixes need to revert a couple of patches, but
isn't reverting 6 commits from the v2.6.32 kernel a bit odd for that?
At least, can you justify this choice? Are all these commits useless
and buggy?

> 
>  drivers/mtd/mtd_blkdevs.c | 77 +++++++++++++++------------------------
>  1 file changed, 30 insertions(+), 47 deletions(-)
> 

Thanks,
Miquèl

Re: [PATCH 0/6] Fix deadlock in ftl formating on mtd

[Miquel Raynal]

Hi Zhihao,

Zhihao Cheng <chengzhihao1@huawei.com> wrote on Sun, 13 Jun 2021
19:30:29 +0800:

> There is an AA-deadlock problem while formating mtd device to generate
> a ftl device. Fix it by reverting "mtd: allow to unload the mtdtrans
> module if its block devices aren't open" recommended in [1].
> 
> [<0>] blktrans_open+0x47/0x340            LOCK(mtd_table_mutex)
> [<0>] __blkdev_get+0x5b/0x3e0
> [<0>] blkdev_get_by_dev+0x18f/0x370
> [<0>] __device_add_disk+0x2db/0x700
> [<0>] device_add_disk+0x17/0x20
> [<0>] add_mtd_blktrans_dev+0x39e/0x6d0
> [<0>] ftl_add_mtd+0x792/0x908 [ftl]
> [<0>] register_mtd_blktrans+0xfb/0x170    LOCK(mtd_table_mutex)
> [<0>] ftl_tr_init+0x18/0x1000 [ftl]
> [<0>] do_one_initcall+0x71/0x330
> [<0>] do_init_module+0xa6/0x350
> 
> [1] http://lists.infradead.org/pipermail/linux-mtd/2017-March/072899.html
> 
> Zhihao Cheng (6):
>   Revert "mtd: blkdevs: fix potential deadlock + lockdep warnings"
>   Revert "mtd: fix: avoid race condition when accessing mtd->usecount"
>   Revert "mtd: mtd_blkdevs: don't increase 'open' count on error path"
>   Revert "mtd: mtd_blkdevs: fix error path in blktrans_open"
>   Revert "mtd: Remove redundant mutex from mtd_blkdevs.c"
>   Revert "mtd: allow to unload the mtdtrans module if its block devices
>     aren't open"

I understand that some fixes need to revert a couple of patches, but
isn't reverting 6 commits from the v2.6.32 kernel a bit odd for that?
At least, can you justify this choice? Are all these commits useless
and buggy?

> 
>  drivers/mtd/mtd_blkdevs.c | 77 +++++++++++++++------------------------
>  1 file changed, 30 insertions(+), 47 deletions(-)
> 

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 0/6] Fix deadlock in ftl formating on mtd

[Zhihao Cheng]

在 2021/6/13 23:12, Miquel Raynal 写道:
> Hi Zhihao,
>
> Zhihao Cheng <chengzhihao1@huawei.com> wrote on Sun, 13 Jun 2021
> 19:30:29 +0800:
>
>> There is an AA-deadlock problem while formating mtd device to generate
>> a ftl device. Fix it by reverting "mtd: allow to unload the mtdtrans
>> module if its block devices aren't open" recommended in [1].
>>
> I understand that some fixes need to revert a couple of patches, but
> isn't reverting 6 commits from the v2.6.32 kernel a bit odd for that?
> At least, can you justify this choice? Are all these commits useless
> and buggy?
>
Hi Miquel,

This series of revert patches came from history commits which related 
with "mtd: allow to unload the mtdtrans module if its block devices 
aren't open". I admin it's a weird method and let's forget it.
After some local tests, I think we'd better reserve "mtd: allow to 
unload the mtdtrans module if its block devices aren't open" to support 
removing mtdblock module if no one takes it. But we may move get|put 
mtd_device into add|del_mtd_blktrans to solve deadlock problem, maybe we 
don't have to get_mtd_device each time open mtd block device in 
blktrans_open if we have called get_mtd_device() in 
add_mtd_blktrans_dev(). See v2 "mtd: mtd_blkdevs: Get|Put mtd_device in 
add|del_mtd_blktrans".

> Thanks,
> Miquèl
> .

Re: [PATCH 0/6] Fix deadlock in ftl formating on mtd

[Zhihao Cheng]

在 2021/6/13 23:12, Miquel Raynal 写道:
> Hi Zhihao,
>
> Zhihao Cheng <chengzhihao1@huawei.com> wrote on Sun, 13 Jun 2021
> 19:30:29 +0800:
>
>> There is an AA-deadlock problem while formating mtd device to generate
>> a ftl device. Fix it by reverting "mtd: allow to unload the mtdtrans
>> module if its block devices aren't open" recommended in [1].
>>
> I understand that some fixes need to revert a couple of patches, but
> isn't reverting 6 commits from the v2.6.32 kernel a bit odd for that?
> At least, can you justify this choice? Are all these commits useless
> and buggy?
>
Hi Miquel,

This series of revert patches came from history commits which related 
with "mtd: allow to unload the mtdtrans module if its block devices 
aren't open". I admin it's a weird method and let's forget it.
After some local tests, I think we'd better reserve "mtd: allow to 
unload the mtdtrans module if its block devices aren't open" to support 
removing mtdblock module if no one takes it. But we may move get|put 
mtd_device into add|del_mtd_blktrans to solve deadlock problem, maybe we 
don't have to get_mtd_device each time open mtd block device in 
blktrans_open if we have called get_mtd_device() in 
add_mtd_blktrans_dev(). See v2 "mtd: mtd_blkdevs: Get|Put mtd_device in 
add|del_mtd_blktrans".

> Thanks,
> Miquèl
> .



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 0/6] Fix deadlock in ftl formating on mtd

[Zhihao Cheng]

在 2021/6/13 23:12, Miquel Raynal 写道:
> Hi Zhihao,
> 
> Zhihao Cheng <chengzhihao1@huawei.com> wrote on Sun, 13 Jun 2021
> 19:30:29 +0800:
> 

> 
> I understand that some fixes need to revert a couple of patches, but
> isn't reverting 6 commits from the v2.6.32 kernel a bit odd for that?
> At least, can you justify this choice? Are all these commits useless
> and buggy?

Hi Miquel,

This series of revert patches came from history commits which related 
with "mtd: allow to unload the mtdtrans module if its block devices 
aren't open". I admin it's a weird method and let's forget it.
After some local tests, I think we'd better reserve "mtd: allow to 
unload the mtdtrans module if its block devices aren't open" to support 
removing mtdblock module if no one takes it. But we may move get|put 
mtd_device into add|del_mtd_blktrans to solve deadlock problem, maybe we 
don't have to get_mtd_device each time open mtd block device in 
blktrans_open if we have called get_mtd_device() in 
add_mtd_blktrans_dev(). See v2 "mtd: mtd_blkdevs: Get|Put mtd_device in 
add|del_mtd_blktrans".

>>
> 
> Thanks,
> Miquèl
> .
> 

Re: [PATCH 0/6] Fix deadlock in ftl formating on mtd

[Zhihao Cheng]

在 2021/6/13 23:12, Miquel Raynal 写道:
> Hi Zhihao,
> 
> Zhihao Cheng <chengzhihao1@huawei.com> wrote on Sun, 13 Jun 2021
> 19:30:29 +0800:
> 

> 
> I understand that some fixes need to revert a couple of patches, but
> isn't reverting 6 commits from the v2.6.32 kernel a bit odd for that?
> At least, can you justify this choice? Are all these commits useless
> and buggy?

Hi Miquel,

This series of revert patches came from history commits which related 
with "mtd: allow to unload the mtdtrans module if its block devices 
aren't open". I admin it's a weird method and let's forget it.
After some local tests, I think we'd better reserve "mtd: allow to 
unload the mtdtrans module if its block devices aren't open" to support 
removing mtdblock module if no one takes it. But we may move get|put 
mtd_device into add|del_mtd_blktrans to solve deadlock problem, maybe we 
don't have to get_mtd_device each time open mtd block device in 
blktrans_open if we have called get_mtd_device() in 
add_mtd_blktrans_dev(). See v2 "mtd: mtd_blkdevs: Get|Put mtd_device in 
add|del_mtd_blktrans".

>>
> 
> Thanks,
> Miquèl
> .
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/