Web lists-archives.com

[PATCH] mtd:avoid blktrans_open/release race and avoid insmod ftl.ko deadlock




this modification can fix the is issue in commit
857814ee65dbc942b18b2dc713124ffff043035e "mtd: fix: avoid race condition
when accessing mtd->usecount ",also can solve the issue about it happen
dealock when ismod ftl.ko. the original process is as follows:
init_ftl
register_mtd_blktrans
mutex_lock(&mtd_table_mutex) //mtd_table_mutex locked
ftl_add_mtd
add_mtd_blktrans_dev
device_add_disk
register_disk
blkdev_get
__blkdev_get
blktrans_open
mutex_lock(&mtd_table_mutex) //dead lock

this patch can prevent some mtd_table_mutex lock race undiscovered.

Signed-off-by: Gu Zheng <guzheng1@xxxxxxxxxx>
---
 drivers/mtd/mtd_blkdevs.c | 31 ++++++++------------
 drivers/mtd/mtdcore.c     | 74 +++++++++++++++++++++++++++++++++++------------
 drivers/mtd/mtdcore.h     |  4 ++-
 3 files changed, 71 insertions(+), 38 deletions(-)

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 6b8d5cd..c194208 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -191,7 +191,7 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 	if (!dev)
 		return -ERESTARTSYS; /* FIXME: busy loop! -arnd*/
 
-	mutex_lock(&mtd_table_mutex);
+	mtd_table_mutex_lock();
 	mutex_lock(&dev->lock);
 
 	if (dev->open)
@@ -217,7 +217,7 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 unlock:
 	dev->open++;
 	mutex_unlock(&dev->lock);
-	mutex_unlock(&mtd_table_mutex);
+	mtd_table_mutex_unlock();
 	blktrans_dev_put(dev);
 	return ret;
 
@@ -228,7 +228,7 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
 	module_put(dev->tr->owner);
 	kref_put(&dev->ref, blktrans_dev_release);
 	mutex_unlock(&dev->lock);
-	mutex_unlock(&mtd_table_mutex);
+	mtd_table_mutex_unlock();
 	blktrans_dev_put(dev);
 	return ret;
 }
@@ -240,7 +240,7 @@ static void blktrans_release(struct gendisk *disk, fmode_t mode)
 	if (!dev)
 		return;
 
-	mutex_lock(&mtd_table_mutex);
+	mtd_table_mutex_lock();
 	mutex_lock(&dev->lock);
 
 	if (--dev->open)
@@ -256,7 +256,7 @@ static void blktrans_release(struct gendisk *disk, fmode_t mode)
 	}
 unlock:
 	mutex_unlock(&dev->lock);
-	mutex_unlock(&mtd_table_mutex);
+	mtd_table_mutex_unlock();
 	blktrans_dev_put(dev);
 }
 
@@ -323,10 +323,7 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
 	struct gendisk *gd;
 	int ret;
 
-	if (mutex_trylock(&mtd_table_mutex)) {
-		mutex_unlock(&mtd_table_mutex);
-		BUG();
-	}
+	mtd_table_assert_mutex_locked();
 
 	mutex_lock(&blktrans_ref_mutex);
 	list_for_each_entry(d, &tr->devs, list) {
@@ -455,11 +452,7 @@ int del_mtd_blktrans_dev(struct mtd_blktrans_dev *old)
 {
 	unsigned long flags;
 
-	if (mutex_trylock(&mtd_table_mutex)) {
-		mutex_unlock(&mtd_table_mutex);
-		BUG();
-	}
-
+	mtd_table_assert_mutex_locked();
 	if (old->disk_attributes)
 		sysfs_remove_group(&disk_to_dev(old->disk)->kobj,
 						old->disk_attributes);
@@ -531,13 +524,13 @@ int register_mtd_blktrans(struct mtd_blktrans_ops *tr)
 		register_mtd_user(&blktrans_notifier);
 
 
-	mutex_lock(&mtd_table_mutex);
+	mtd_table_mutex_lock();
 
 	ret = register_blkdev(tr->major, tr->name);
 	if (ret < 0) {
 		printk(KERN_WARNING "Unable to register %s block device on major %d: %d\n",
 		       tr->name, tr->major, ret);
-		mutex_unlock(&mtd_table_mutex);
+		mtd_table_mutex_unlock();
 		return ret;
 	}
 
@@ -553,7 +546,7 @@ int register_mtd_blktrans(struct mtd_blktrans_ops *tr)
 		if (mtd->type != MTD_ABSENT)
 			tr->add_mtd(tr, mtd);
 
-	mutex_unlock(&mtd_table_mutex);
+	mtd_table_mutex_unlock();
 	return 0;
 }
 
@@ -561,7 +554,7 @@ int deregister_mtd_blktrans(struct mtd_blktrans_ops *tr)
 {
 	struct mtd_blktrans_dev *dev, *next;
 
-	mutex_lock(&mtd_table_mutex);
+	mtd_table_mutex_lock();
 
 	/* Remove it from the list of active majors */
 	list_del(&tr->list);
@@ -570,7 +563,7 @@ int deregister_mtd_blktrans(struct mtd_blktrans_ops *tr)
 		tr->remove_dev(dev);
 
 	unregister_blkdev(tr->major, tr->name);
-	mutex_unlock(&mtd_table_mutex);
+	mtd_table_mutex_unlock();
 
 	BUG_ON(!list_empty(&tr->devs));
 	return 0;
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 66a9ded..f3d5470 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -84,6 +84,8 @@ static DEFINE_IDR(mtd_idr);
    should not use them for _anything_ else */
 DEFINE_MUTEX(mtd_table_mutex);
 EXPORT_SYMBOL_GPL(mtd_table_mutex);
+int mtd_table_mutex_depth;
+struct task_struct *mtd_table_mutex_owner;
 
 struct mtd_info *__mtd_next_device(int i)
 {
@@ -96,6 +98,42 @@ static LIST_HEAD(mtd_notifiers);
 
 #define MTD_DEVT(index) MKDEV(MTD_CHAR_MAJOR, (index)*2)
 
+void mtd_table_mutex_lock(void)
+{
+	if (mtd_table_mutex_owner != current) {
+		mutex_lock(&mtd_table_mutex);
+		mtd_table_mutex_owner =  current;
+	}
+	mtd_table_mutex_depth++;
+
+}
+EXPORT_SYMBOL_GPL(mtd_table_mutex_lock);
+
+
+void mtd_table_mutex_unlock(void)
+{
+	if (mtd_table_mutex_owner != current) {
+		pr_err("MTD:lock_owner is %s, but current is %s\n",
+				mtd_table_mutex_owner->comm, current->comm);
+		BUG();
+	}
+	if (--mtd_table_mutex_depth == 0) {
+		mtd_table_mutex_owner =  NULL;
+		mutex_unlock(&mtd_table_mutex);
+	}
+
+}
+EXPORT_SYMBOL_GPL(mtd_table_mutex_unlock);
+
+void mtd_table_assert_mutex_locked(void)
+{
+	if (mtd_table_mutex_owner != current) {
+		pr_err("MTD:lock_owner is %s, but current is %s\n",
+				mtd_table_mutex_owner->comm, current->comm);
+		BUG();
+	}
+}
+EXPORT_SYMBOL_GPL(mtd_table_assert_mutex_locked);
 /* REVISIT once MTD uses the driver model better, whoever allocates
  * the mtd_info will probably want to use the release() hook...
  */
@@ -502,7 +540,7 @@ int add_mtd_device(struct mtd_info *mtd)
 	mtd->backing_dev_info = mtd_bdi;
 
 	BUG_ON(mtd->writesize == 0);
-	mutex_lock(&mtd_table_mutex);
+	mtd_table_mutex_lock();
 
 	i = idr_alloc(&mtd_idr, mtd, 0, 0, GFP_KERNEL);
 	if (i < 0) {
@@ -563,7 +601,7 @@ int add_mtd_device(struct mtd_info *mtd)
 	list_for_each_entry(not, &mtd_notifiers, list)
 		not->add(mtd);
 
-	mutex_unlock(&mtd_table_mutex);
+	mtd_table_mutex_unlock();
 	/* We _know_ we aren't being removed, because
 	   our caller is still holding us here. So none
 	   of this try_ nonsense, and no bitching about it
@@ -575,7 +613,7 @@ int add_mtd_device(struct mtd_info *mtd)
 	of_node_put(mtd_get_of_node(mtd));
 	idr_remove(&mtd_idr, i);
 fail_locked:
-	mutex_unlock(&mtd_table_mutex);
+	mtd_table_mutex_unlock();
 	return error;
 }
 
@@ -594,7 +632,7 @@ int del_mtd_device(struct mtd_info *mtd)
 	int ret;
 	struct mtd_notifier *not;
 
-	mutex_lock(&mtd_table_mutex);
+	mtd_table_mutex_lock();
 
 	if (idr_find(&mtd_idr, mtd->index) != mtd) {
 		ret = -ENODEV;
@@ -621,7 +659,7 @@ int del_mtd_device(struct mtd_info *mtd)
 	}
 
 out_error:
-	mutex_unlock(&mtd_table_mutex);
+	mtd_table_mutex_unlock();
 	return ret;
 }
 
@@ -782,7 +820,7 @@ void register_mtd_user (struct mtd_notifier *new)
 {
 	struct mtd_info *mtd;
 
-	mutex_lock(&mtd_table_mutex);
+	mtd_table_mutex_lock();
 
 	list_add(&new->list, &mtd_notifiers);
 
@@ -791,7 +829,7 @@ void register_mtd_user (struct mtd_notifier *new)
 	mtd_for_each_device(mtd)
 		new->add(mtd);
 
-	mutex_unlock(&mtd_table_mutex);
+	mtd_table_mutex_unlock();
 }
 EXPORT_SYMBOL_GPL(register_mtd_user);
 
@@ -808,7 +846,7 @@ int unregister_mtd_user (struct mtd_notifier *old)
 {
 	struct mtd_info *mtd;
 
-	mutex_lock(&mtd_table_mutex);
+	mtd_table_mutex_lock();
 
 	module_put(THIS_MODULE);
 
@@ -816,7 +854,7 @@ int unregister_mtd_user (struct mtd_notifier *old)
 		old->remove(mtd);
 
 	list_del(&old->list);
-	mutex_unlock(&mtd_table_mutex);
+	mtd_table_mutex_unlock();
 	return 0;
 }
 EXPORT_SYMBOL_GPL(unregister_mtd_user);
@@ -837,7 +875,7 @@ struct mtd_info *get_mtd_device(struct mtd_info *mtd, int num)
 	struct mtd_info *ret = NULL, *other;
 	int err = -ENODEV;
 
-	mutex_lock(&mtd_table_mutex);
+	mtd_table_mutex_lock();
 
 	if (num == -1) {
 		mtd_for_each_device(other) {
@@ -861,7 +899,7 @@ struct mtd_info *get_mtd_device(struct mtd_info *mtd, int num)
 	if (err)
 		ret = ERR_PTR(err);
 out:
-	mutex_unlock(&mtd_table_mutex);
+	mtd_table_mutex_unlock();
 	return ret;
 }
 EXPORT_SYMBOL_GPL(get_mtd_device);
@@ -900,7 +938,7 @@ struct mtd_info *get_mtd_device_nm(const char *name)
 	int err = -ENODEV;
 	struct mtd_info *mtd = NULL, *other;
 
-	mutex_lock(&mtd_table_mutex);
+	mtd_table_mutex_lock();
 
 	mtd_for_each_device(other) {
 		if (!strcmp(name, other->name)) {
@@ -916,20 +954,20 @@ struct mtd_info *get_mtd_device_nm(const char *name)
 	if (err)
 		goto out_unlock;
 
-	mutex_unlock(&mtd_table_mutex);
+	mtd_table_mutex_unlock();
 	return mtd;
 
 out_unlock:
-	mutex_unlock(&mtd_table_mutex);
+	mtd_table_mutex_unlock();
 	return ERR_PTR(err);
 }
 EXPORT_SYMBOL_GPL(get_mtd_device_nm);
 
 void put_mtd_device(struct mtd_info *mtd)
 {
-	mutex_lock(&mtd_table_mutex);
+	mtd_table_mutex_lock();
 	__put_mtd_device(mtd);
-	mutex_unlock(&mtd_table_mutex);
+	mtd_table_mutex_unlock();
 
 }
 EXPORT_SYMBOL_GPL(put_mtd_device);
@@ -1744,13 +1782,13 @@ static int mtd_proc_show(struct seq_file *m, void *v)
 	struct mtd_info *mtd;
 
 	seq_puts(m, "dev:    size   erasesize  name\n");
-	mutex_lock(&mtd_table_mutex);
+	mtd_table_mutex_lock();
 	mtd_for_each_device(mtd) {
 		seq_printf(m, "mtd%d: %8.8llx %8.8x \"%s\"\n",
 			   mtd->index, (unsigned long long)mtd->size,
 			   mtd->erasesize, mtd->name);
 	}
-	mutex_unlock(&mtd_table_mutex);
+	mtd_table_mutex_unlock();
 	return 0;
 }
 
diff --git a/drivers/mtd/mtdcore.h b/drivers/mtd/mtdcore.h
index 55fdb8e..cb1d8fa 100644
--- a/drivers/mtd/mtdcore.h
+++ b/drivers/mtd/mtdcore.h
@@ -3,7 +3,6 @@
  * You should not use them for _anything_ else.
  */
 
-extern struct mutex mtd_table_mutex;
 
 struct mtd_info *__mtd_next_device(int i);
 int add_mtd_device(struct mtd_info *mtd);
@@ -21,6 +20,9 @@ void mtd_part_parser_cleanup(struct mtd_partitions *parts);
 
 int __init init_mtdchar(void);
 void __exit cleanup_mtdchar(void);
+extern void mtd_table_mutex_lock(void);
+extern void mtd_table_mutex_unlock(void);
+extern void mtd_table_assert_mutex_locked(void);
 
 #define mtd_for_each_device(mtd)			\
 	for ((mtd) = __mtd_next_device(0);		\
-- 
2.5.0