* [PATCH 1/8] blktrans: nuke mtd_blkcore_priv and make both thread and disk queue be per device
2010-01-23 0:18 [PATCH 0/8] Cleanups for mtd translation layer (broken up) Maxim Levitsky
@ 2010-01-23 0:19 ` Maxim Levitsky
2010-01-23 0:21 ` [PATCH 2/8] blktrans: track open and close calls Maxim Levitsky
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Maxim Levitsky @ 2010-01-23 0:19 UTC (permalink / raw)
To: Artem Bityutskiy
Cc: Alex Dubov, joern, linux-kernel, linux-mtd, Thomas Gleixner,
David Woodhouse
>From b18c2e01321b68dccbd2ea4ea321667687747173 Mon Sep 17 00:00:00 2001
From: Maxim Levitsky <maximlevitsky@gmail.com>
Date: Sat, 23 Jan 2010 02:00:37 +0200
Subject: [PATCH 1/8] blktrans: nuke mtd_blkcore_priv and make both thread and disk queue be per device
This is the biggest change. To make hotplug possible, and this layer clearer, now
mtd_blktrans_dev contains everything for a single mtd block translation device.
Also removed some very old leftovers
Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
drivers/mtd/mtd_blkdevs.c | 123 ++++++++++++++++++++----------------------
include/linux/mtd/blktrans.h | 10 ++--
2 files changed, 64 insertions(+), 69 deletions(-)
diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index c82e09b..3d9802e 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -26,11 +26,6 @@
static LIST_HEAD(blktrans_majors);
-struct mtd_blkcore_priv {
- struct task_struct *thread;
- struct request_queue *rq;
- spinlock_t queue_lock;
-};
static int do_blktrans_request(struct mtd_blktrans_ops *tr,
struct mtd_blktrans_dev *dev,
@@ -80,14 +75,13 @@ static int do_blktrans_request(struct mtd_blktrans_ops *tr,
static int mtd_blktrans_thread(void *arg)
{
- struct mtd_blktrans_ops *tr = arg;
- struct request_queue *rq = tr->blkcore_priv->rq;
+ struct mtd_blktrans_dev *dev = arg;
+ struct request_queue *rq = dev->rq;
struct request *req = NULL;
spin_lock_irq(rq->queue_lock);
while (!kthread_should_stop()) {
- struct mtd_blktrans_dev *dev;
int res;
if (!req && !(req = blk_fetch_request(rq))) {
@@ -98,13 +92,10 @@ static int mtd_blktrans_thread(void *arg)
continue;
}
- dev = req->rq_disk->private_data;
- tr = dev->tr;
-
spin_unlock_irq(rq->queue_lock);
mutex_lock(&dev->lock);
- res = do_blktrans_request(tr, dev, req);
+ res = do_blktrans_request(dev->tr, dev, req);
mutex_unlock(&dev->lock);
spin_lock_irq(rq->queue_lock);
@@ -123,8 +114,8 @@ static int mtd_blktrans_thread(void *arg)
static void mtd_blktrans_request(struct request_queue *rq)
{
- struct mtd_blktrans_ops *tr = rq->queuedata;
- wake_up_process(tr->blkcore_priv->thread);
+ struct mtd_blktrans_dev *dev = rq->queuedata;
+ wake_up_process(dev->thread);
}
@@ -214,6 +205,7 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
struct mtd_blktrans_dev *d;
int last_devnum = -1;
struct gendisk *gd;
+ int ret;
if (mutex_trylock(&mtd_table_mutex)) {
mutex_unlock(&mtd_table_mutex);
@@ -239,12 +231,13 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
}
last_devnum = d->devnum;
}
+
+ ret = -EBUSY;
if (new->devnum == -1)
new->devnum = last_devnum+1;
- if ((new->devnum << tr->part_bits) > 256) {
- return -EBUSY;
- }
+ if ((new->devnum << tr->part_bits) > 256)
+ goto error1;
list_add_tail(&new->list, &tr->devs);
added:
@@ -252,11 +245,16 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
if (!tr->writesect)
new->readonly = 1;
+
+ /* Create gendisk */
+ ret = -ENOMEM;
gd = alloc_disk(1 << tr->part_bits);
- if (!gd) {
- list_del(&new->list);
- return -ENOMEM;
- }
+
+ if (!gd)
+ goto error2;
+
+ new->disk = gd;
+ gd->private_data = new;
gd->major = tr->major;
gd->first_minor = (new->devnum) << tr->part_bits;
gd->fops = &mtd_blktrans_ops;
@@ -274,13 +272,33 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
snprintf(gd->disk_name, sizeof(gd->disk_name),
"%s%d", tr->name, new->devnum);
- /* 2.5 has capacity in units of 512 bytes while still
- having BLOCK_SIZE_BITS set to 10. Just to keep us amused. */
set_capacity(gd, (new->size * tr->blksize) >> 9);
- gd->private_data = new;
- new->blkcore_priv = gd;
- gd->queue = tr->blkcore_priv->rq;
+
+ /* Create the request queue */
+ spin_lock_init(&new->queue_lock);
+ new->rq = blk_init_queue(mtd_blktrans_request, &new->queue_lock);
+
+ if (!new->rq)
+ goto error3;
+
+ new->rq->queuedata = new;
+ blk_queue_logical_block_size(new->rq, tr->blksize);
+
+ if (tr->discard)
+ queue_flag_set_unlocked(QUEUE_FLAG_DISCARD,
+ new->rq);
+
+ gd->queue = new->rq;
+
+ /* Create processing thread */
+ /* TODO: workqueue ? */
+ new->thread = kthread_run(mtd_blktrans_thread, new,
+ "%s%d", tr->name, new->mtd->index);
+ if (IS_ERR(new->thread)) {
+ ret = PTR_ERR(new->thread);
+ goto error4;
+ }
gd->driverfs_dev = &new->mtd->dev;
if (new->readonly)
@@ -289,6 +307,15 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
add_disk(gd);
return 0;
+error4:
+ blk_cleanup_queue(new->rq);
+error3:
+ put_disk(new->disk);
+error2:
+ list_del(&new->list);
+error1:
+ kfree(new);
+ return ret;
}
int del_mtd_blktrans_dev(struct mtd_blktrans_dev *old)
@@ -300,9 +327,13 @@ int del_mtd_blktrans_dev(struct mtd_blktrans_dev *old)
list_del(&old->list);
- del_gendisk(old->blkcore_priv);
- put_disk(old->blkcore_priv);
+ /* stop new requests to arrive */
+ del_gendisk(old->disk);
+ /* Stop the thread */
+ kthread_stop(old->thread);
+
+ blk_cleanup_queue(old->rq);
return 0;
}
@@ -343,9 +374,6 @@ int register_mtd_blktrans(struct mtd_blktrans_ops *tr)
if (!blktrans_notifier.list.next)
register_mtd_user(&blktrans_notifier);
- tr->blkcore_priv = kzalloc(sizeof(*tr->blkcore_priv), GFP_KERNEL);
- if (!tr->blkcore_priv)
- return -ENOMEM;
mutex_lock(&mtd_table_mutex);
@@ -353,39 +381,12 @@ int register_mtd_blktrans(struct mtd_blktrans_ops *tr)
if (ret) {
printk(KERN_WARNING "Unable to register %s block device on major %d: %d\n",
tr->name, tr->major, ret);
- kfree(tr->blkcore_priv);
mutex_unlock(&mtd_table_mutex);
return ret;
}
- spin_lock_init(&tr->blkcore_priv->queue_lock);
-
- tr->blkcore_priv->rq = blk_init_queue(mtd_blktrans_request, &tr->blkcore_priv->queue_lock);
- if (!tr->blkcore_priv->rq) {
- unregister_blkdev(tr->major, tr->name);
- kfree(tr->blkcore_priv);
- mutex_unlock(&mtd_table_mutex);
- return -ENOMEM;
- }
-
- tr->blkcore_priv->rq->queuedata = tr;
- blk_queue_logical_block_size(tr->blkcore_priv->rq, tr->blksize);
- if (tr->discard)
- queue_flag_set_unlocked(QUEUE_FLAG_DISCARD,
- tr->blkcore_priv->rq);
tr->blkshift = ffs(tr->blksize) - 1;
- tr->blkcore_priv->thread = kthread_run(mtd_blktrans_thread, tr,
- "%sd", tr->name);
- if (IS_ERR(tr->blkcore_priv->thread)) {
- ret = PTR_ERR(tr->blkcore_priv->thread);
- blk_cleanup_queue(tr->blkcore_priv->rq);
- unregister_blkdev(tr->major, tr->name);
- kfree(tr->blkcore_priv);
- mutex_unlock(&mtd_table_mutex);
- return ret;
- }
-
INIT_LIST_HEAD(&tr->devs);
list_add(&tr->list, &blktrans_majors);
@@ -405,8 +406,6 @@ int deregister_mtd_blktrans(struct mtd_blktrans_ops *tr)
mutex_lock(&mtd_table_mutex);
- /* Clean up the kernel thread */
- kthread_stop(tr->blkcore_priv->thread);
/* Remove it from the list of active majors */
list_del(&tr->list);
@@ -414,13 +413,9 @@ int deregister_mtd_blktrans(struct mtd_blktrans_ops *tr)
list_for_each_entry_safe(dev, next, &tr->devs, list)
tr->remove_dev(dev);
- blk_cleanup_queue(tr->blkcore_priv->rq);
unregister_blkdev(tr->major, tr->name);
-
mutex_unlock(&mtd_table_mutex);
- kfree(tr->blkcore_priv);
-
BUG_ON(!list_empty(&tr->devs));
return 0;
}
diff --git a/include/linux/mtd/blktrans.h b/include/linux/mtd/blktrans.h
index 8b4aa05..a4b3928 100644
--- a/include/linux/mtd/blktrans.h
+++ b/include/linux/mtd/blktrans.h
@@ -24,11 +24,13 @@ struct mtd_blktrans_dev {
int devnum;
unsigned long size;
int readonly;
- void *blkcore_priv; /* gendisk in 2.5, devfs_handle in 2.4 */
+ struct gendisk *disk;
+ struct task_struct *thread;
+ struct request_queue *rq;
+ spinlock_t queue_lock;
+ void *priv;
};
-struct blkcore_priv; /* Differs for 2.4 and 2.5 kernels; private */
-
struct mtd_blktrans_ops {
char *name;
int major;
@@ -60,8 +62,6 @@ struct mtd_blktrans_ops {
struct list_head devs;
struct list_head list;
struct module *owner;
-
- struct mtd_blkcore_priv *blkcore_priv;
};
extern int register_mtd_blktrans(struct mtd_blktrans_ops *tr);
--
1.6.3.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/8] blktrans: track open and close calls.
2010-01-23 0:18 [PATCH 0/8] Cleanups for mtd translation layer (broken up) Maxim Levitsky
2010-01-23 0:19 ` [PATCH 1/8] blktrans: nuke mtd_blkcore_priv and make both thread and disk queue be per device Maxim Levitsky
@ 2010-01-23 0:21 ` Maxim Levitsky
2010-01-23 0:23 ` [PATCH 3/8] blktrans: don't free mtd_blktrans_dev, core will do that for you Maxim Levitsky
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Maxim Levitsky @ 2010-01-23 0:21 UTC (permalink / raw)
To: Artem Bityutskiy
Cc: Alex Dubov, joern, linux-kernel, linux-mtd, Thomas Gleixner,
David Woodhouse
>From 1ed89e9f7355284eaebbacb1d083e3bbd54aa3bc Mon Sep 17 00:00:00 2001
From: Maxim Levitsky <maximlevitsky@gmail.com>
Date: Sat, 23 Jan 2010 02:03:13 +0200
Subject: [PATCH 2/8] blktrans: track open and close calls.
This patch adds tracking for open and close calls.
Now trans ->open and ->release are never called twise in a row
->release is also called once before mtd device disappers
Proper locking will be added in follow up patch
Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
drivers/mtd/mtd_blkdevs.c | 19 +++++++++++++++++++
include/linux/mtd/blktrans.h | 1 +
2 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 3d9802e..c2d97a0 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -128,6 +128,9 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
if (!get_mtd_device(NULL, dev->mtd->index))
goto out;
+ if (dev->open++)
+ goto out;
+
if (!try_module_get(tr->owner))
goto out_tr;
@@ -153,6 +156,10 @@ static int blktrans_release(struct gendisk *disk, fmode_t mode)
struct mtd_blktrans_ops *tr = dev->tr;
int ret = 0;
+ dev->open--;
+ if (dev->open)
+ return 0;
+
if (tr->release)
ret = tr->release(dev);
@@ -304,6 +311,7 @@ int add_mtd_blktrans_dev(struct mtd_blktrans_dev *new)
if (new->readonly)
set_disk_ro(gd, 1);
+ new->open = 0;
add_disk(gd);
return 0;
@@ -332,6 +340,17 @@ int del_mtd_blktrans_dev(struct mtd_blktrans_dev *old)
/* Stop the thread */
kthread_stop(old->thread);
+
+ if (old->open) {
+ if (old->tr->release)
+ old->tr->release(old);
+ put_mtd_device(old->mtd);
+ }
+
+ /* From now on, no calls into trans can be made */
+ /* Mtd device will be gone real soon now */
+ old->mtd = NULL;
+
blk_cleanup_queue(old->rq);
return 0;
diff --git a/include/linux/mtd/blktrans.h b/include/linux/mtd/blktrans.h
index a4b3928..507f7b2 100644
--- a/include/linux/mtd/blktrans.h
+++ b/include/linux/mtd/blktrans.h
@@ -24,6 +24,7 @@ struct mtd_blktrans_dev {
int devnum;
unsigned long size;
int readonly;
+ int open;
struct gendisk *disk;
struct task_struct *thread;
struct request_queue *rq;
--
1.6.3.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/8] blktrans: don't free mtd_blktrans_dev, core will do that for you
2010-01-23 0:18 [PATCH 0/8] Cleanups for mtd translation layer (broken up) Maxim Levitsky
2010-01-23 0:19 ` [PATCH 1/8] blktrans: nuke mtd_blkcore_priv and make both thread and disk queue be per device Maxim Levitsky
2010-01-23 0:21 ` [PATCH 2/8] blktrans: track open and close calls Maxim Levitsky
@ 2010-01-23 0:23 ` Maxim Levitsky
2010-01-23 0:23 ` [PATCH 4/8] MTD: create lockless versions of {get,put}_mtd_device Maxim Levitsky
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Maxim Levitsky @ 2010-01-23 0:23 UTC (permalink / raw)
To: Artem Bityutskiy
Cc: Alex Dubov, joern, linux-kernel, linux-mtd, Thomas Gleixner,
David Woodhouse
>From 926e8cf37572691246011945e3266ca140319b41 Mon Sep 17 00:00:00 2001
From: Maxim Levitsky <maximlevitsky@gmail.com>
Date: Sat, 23 Jan 2010 02:04:32 +0200
Subject: [PATCH 3/8] blktrans: don't free mtd_blktrans_dev, core will do that for you
We need that structure till last user exits, and that might be years after
mtd device disappered.
Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
drivers/mtd/ftl.c | 1 -
drivers/mtd/inftlcore.c | 1 -
drivers/mtd/mtd_blkdevs.c | 16 +++++++++++++++-
drivers/mtd/mtdblock.c | 1 -
drivers/mtd/mtdblock_ro.c | 1 -
drivers/mtd/nftlcore.c | 1 -
drivers/mtd/rfd_ftl.c | 1 -
drivers/mtd/ssfdc.c | 1 -
include/linux/mtd/blktrans.h | 1 +
9 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/ftl.c b/drivers/mtd/ftl.c
index e56d6b4..62da9eb 100644
--- a/drivers/mtd/ftl.c
+++ b/drivers/mtd/ftl.c
@@ -1082,7 +1082,6 @@ static void ftl_remove_dev(struct mtd_blktrans_dev *dev)
{
del_mtd_blktrans_dev(dev);
ftl_freepart((partition_t *)dev);
- kfree(dev);
}
static struct mtd_blktrans_ops ftl_tr = {
diff --git a/drivers/mtd/inftlcore.c b/drivers/mtd/inftlcore.c
index 8aca552..015a7fe 100755
--- a/drivers/mtd/inftlcore.c
+++ b/drivers/mtd/inftlcore.c
@@ -139,7 +139,6 @@ static void inftl_remove_dev(struct mtd_blktrans_dev *dev)
kfree(inftl->PUtable);
kfree(inftl->VUtable);
- kfree(inftl);
}
/*
diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index c2d97a0..c95e1ff 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -129,7 +129,11 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
goto out;
if (dev->open++)
- goto out;
+ goto out;
+
+ if (dev->deleted)
+ goto out;
+
if (!try_module_get(tr->owner))
goto out_tr;
@@ -169,6 +173,14 @@ static int blktrans_release(struct gendisk *disk, fmode_t mode)
module_put(tr->owner);
}
+ /* Free the private data */
+ if (dev->deleted) {
+ module_put(tr->owner);
+ mutex_unlock(&dev->lock);
+ kfree(dev);
+ return 0;
+ }
+
return ret;
}
@@ -338,6 +350,8 @@ int del_mtd_blktrans_dev(struct mtd_blktrans_dev *old)
/* stop new requests to arrive */
del_gendisk(old->disk);
+ old->deleted = 1;
+
/* Stop the thread */
kthread_stop(old->thread);
diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c
index 9f41b1a..d8322cc 100644
--- a/drivers/mtd/mtdblock.c
+++ b/drivers/mtd/mtdblock.c
@@ -368,7 +368,6 @@ static void mtdblock_add_mtd(struct mtd_blktrans_ops *tr, struct mtd_info *mtd)
static void mtdblock_remove_dev(struct mtd_blktrans_dev *dev)
{
del_mtd_blktrans_dev(dev);
- kfree(dev);
}
static struct mtd_blktrans_ops mtdblock_tr = {
diff --git a/drivers/mtd/mtdblock_ro.c b/drivers/mtd/mtdblock_ro.c
index 852165f..54ff288 100644
--- a/drivers/mtd/mtdblock_ro.c
+++ b/drivers/mtd/mtdblock_ro.c
@@ -49,7 +49,6 @@ static void mtdblock_add_mtd(struct mtd_blktrans_ops *tr, struct mtd_info *mtd)
static void mtdblock_remove_dev(struct mtd_blktrans_dev *dev)
{
del_mtd_blktrans_dev(dev);
- kfree(dev);
}
static struct mtd_blktrans_ops mtdblock_tr = {
diff --git a/drivers/mtd/nftlcore.c b/drivers/mtd/nftlcore.c
index 1002e18..a4578bf 100644
--- a/drivers/mtd/nftlcore.c
+++ b/drivers/mtd/nftlcore.c
@@ -126,7 +126,6 @@ static void nftl_remove_dev(struct mtd_blktrans_dev *dev)
del_mtd_blktrans_dev(dev);
kfree(nftl->ReplUnitTable);
kfree(nftl->EUNtable);
- kfree(nftl);
}
/*
diff --git a/drivers/mtd/rfd_ftl.c b/drivers/mtd/rfd_ftl.c
index d2aa9c4..63b83c0 100644
--- a/drivers/mtd/rfd_ftl.c
+++ b/drivers/mtd/rfd_ftl.c
@@ -817,7 +817,6 @@ static void rfd_ftl_remove_dev(struct mtd_blktrans_dev *dev)
vfree(part->sector_map);
kfree(part->header_cache);
kfree(part->blocks);
- kfree(part);
}
static struct mtd_blktrans_ops rfd_ftl_tr = {
diff --git a/drivers/mtd/ssfdc.c b/drivers/mtd/ssfdc.c
index 3f67e00..81c4ecd 100644
--- a/drivers/mtd/ssfdc.c
+++ b/drivers/mtd/ssfdc.c
@@ -375,7 +375,6 @@ static void ssfdcr_remove_dev(struct mtd_blktrans_dev *dev)
del_mtd_blktrans_dev(dev);
kfree(ssfdc->logic_block_map);
- kfree(ssfdc);
}
static int ssfdcr_readsect(struct mtd_blktrans_dev *dev,
diff --git a/include/linux/mtd/blktrans.h b/include/linux/mtd/blktrans.h
index 507f7b2..e276aca 100644
--- a/include/linux/mtd/blktrans.h
+++ b/include/linux/mtd/blktrans.h
@@ -24,6 +24,7 @@ struct mtd_blktrans_dev {
int devnum;
unsigned long size;
int readonly;
+ int deleted;
int open;
struct gendisk *disk;
struct task_struct *thread;
--
1.6.3.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 4/8] MTD: create lockless versions of {get,put}_mtd_device
2010-01-23 0:18 [PATCH 0/8] Cleanups for mtd translation layer (broken up) Maxim Levitsky
` (2 preceding siblings ...)
2010-01-23 0:23 ` [PATCH 3/8] blktrans: don't free mtd_blktrans_dev, core will do that for you Maxim Levitsky
@ 2010-01-23 0:23 ` Maxim Levitsky
2010-01-23 0:24 ` [PATCH 5/8] blktrans: add proper locking Maxim Levitsky
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Maxim Levitsky @ 2010-01-23 0:23 UTC (permalink / raw)
To: Artem Bityutskiy
Cc: Alex Dubov, joern, linux-kernel, linux-mtd, Thomas Gleixner,
David Woodhouse
>From b7c27159b5540b0951f6b030ffca895f34a3b3d3 Mon Sep 17 00:00:00 2001
From: Maxim Levitsky <maximlevitsky@gmail.com>
Date: Sat, 23 Jan 2010 02:04:44 +0200
Subject: [PATCH 4/8] MTD: create lockless versions of {get,put}_mtd_device
This will be used to resolve deadlock in block translation layer.
Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
drivers/mtd/mtdcore.c | 60 ++++++++++++++++++++++++++++++----------------
include/linux/mtd/mtd.h | 3 +-
2 files changed, 41 insertions(+), 22 deletions(-)
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index c356c0a..3bdb7e8 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -453,27 +453,38 @@ struct mtd_info *get_mtd_device(struct mtd_info *mtd, int num)
ret = NULL;
}
- if (!ret)
- goto out_unlock;
-
- if (!try_module_get(ret->owner))
- goto out_unlock;
-
- if (ret->get_device) {
- err = ret->get_device(ret);
- if (err)
- goto out_put;
+ if (!ret) {
+ ret = ERR_PTR(err);
+ goto out;
}
- ret->usecount++;
+ err = __get_mtd_device(ret);
+ if (err)
+ ret = ERR_PTR(err);
+out:
mutex_unlock(&mtd_table_mutex);
return ret;
+}
-out_put:
- module_put(ret->owner);
-out_unlock:
- mutex_unlock(&mtd_table_mutex);
- return ERR_PTR(err);
+
+int __get_mtd_device(struct mtd_info *mtd)
+{
+ int err;
+
+ if (!try_module_get(mtd->owner))
+ return -ENODEV;
+
+ if (mtd->get_device) {
+
+ err = mtd->get_device(mtd);
+
+ if (err) {
+ module_put(mtd->owner);
+ return err;
+ }
+ }
+ mtd->usecount++;
+ return 0;
}
/**
@@ -524,14 +535,19 @@ out_unlock:
void put_mtd_device(struct mtd_info *mtd)
{
- int c;
-
mutex_lock(&mtd_table_mutex);
- c = --mtd->usecount;
+ __put_mtd_device(mtd);
+ mutex_unlock(&mtd_table_mutex);
+
+}
+
+void __put_mtd_device(struct mtd_info *mtd)
+{
+ --mtd->usecount;
+ BUG_ON(mtd->usecount < 0);
+
if (mtd->put_device)
mtd->put_device(mtd);
- mutex_unlock(&mtd_table_mutex);
- BUG_ON(c < 0);
module_put(mtd->owner);
}
@@ -569,7 +585,9 @@ EXPORT_SYMBOL_GPL(add_mtd_device);
EXPORT_SYMBOL_GPL(del_mtd_device);
EXPORT_SYMBOL_GPL(get_mtd_device);
EXPORT_SYMBOL_GPL(get_mtd_device_nm);
+EXPORT_SYMBOL_GPL(__get_mtd_device);
EXPORT_SYMBOL_GPL(put_mtd_device);
+EXPORT_SYMBOL_GPL(__put_mtd_device);
EXPORT_SYMBOL_GPL(register_mtd_user);
EXPORT_SYMBOL_GPL(unregister_mtd_user);
EXPORT_SYMBOL_GPL(default_mtd_writev);
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 0f32a9b..662d747 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -290,8 +290,9 @@ extern int add_mtd_device(struct mtd_info *mtd);
extern int del_mtd_device (struct mtd_info *mtd);
extern struct mtd_info *get_mtd_device(struct mtd_info *mtd, int num);
+extern int __get_mtd_device(struct mtd_info *mtd);
+extern void __put_mtd_device(struct mtd_info *mtd);
extern struct mtd_info *get_mtd_device_nm(const char *name);
-
extern void put_mtd_device(struct mtd_info *mtd);
--
1.6.3.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 5/8] blktrans: add proper locking
2010-01-23 0:18 [PATCH 0/8] Cleanups for mtd translation layer (broken up) Maxim Levitsky
` (3 preceding siblings ...)
2010-01-23 0:23 ` [PATCH 4/8] MTD: create lockless versions of {get,put}_mtd_device Maxim Levitsky
@ 2010-01-23 0:24 ` Maxim Levitsky
2010-01-23 0:24 ` [PATCH 6/8] blktrans: flush all requests before we remove the device Maxim Levitsky
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Maxim Levitsky @ 2010-01-23 0:24 UTC (permalink / raw)
To: Artem Bityutskiy
Cc: Alex Dubov, joern, linux-kernel, linux-mtd, Thomas Gleixner,
David Woodhouse
>From 695c50a575ba7e004931e234f4706766be507289 Mon Sep 17 00:00:00 2001
From: Maxim Levitsky <maximlevitsky@gmail.com>
Date: Sat, 23 Jan 2010 02:07:53 +0200
Subject: [PATCH 5/8] blktrans: add proper locking
First, use lockless versions of get/put_mtd_device
We don't care what happened to mtd table, because we have a pointer
to mtd device. It guaranteed to exist till we do final put_mtd_device
Also add locking to prevent all kinds or races
Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
drivers/mtd/mtd_blkdevs.c | 83 ++++++++++++++++++++++++++++----------------
1 files changed, 53 insertions(+), 30 deletions(-)
diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index c95e1ff..f165115 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -123,34 +123,34 @@ static int blktrans_open(struct block_device *bdev, fmode_t mode)
{
struct mtd_blktrans_dev *dev = bdev->bd_disk->private_data;
struct mtd_blktrans_ops *tr = dev->tr;
- int ret = -ENODEV;
-
- if (!get_mtd_device(NULL, dev->mtd->index))
- goto out;
+ int ret = 0;
+ mutex_lock(&dev->lock);
if (dev->open++)
goto out;
if (dev->deleted)
goto out;
-
+ ret = -ENODEV;
if (!try_module_get(tr->owner))
- goto out_tr;
+ goto out;
+
+ if (__get_mtd_device(dev->mtd)) {
+ module_put(tr->owner);
+ goto out;
+ }
- /* FIXME: Locking. A hot pluggable device can go away
- (del_mtd_device can be called for it) without its module
- being unloaded. */
- dev->mtd->usecount++;
ret = 0;
if (tr->open && (ret = tr->open(dev))) {
- dev->mtd->usecount--;
- put_mtd_device(dev->mtd);
- out_tr:
module_put(tr->owner);
+ __put_mtd_device(dev->mtd);
+ goto out;
+
}
out:
+ mutex_unlock(&dev->lock);
return ret;
}
@@ -160,18 +160,11 @@ static int blktrans_release(struct gendisk *disk, fmode_t mode)
struct mtd_blktrans_ops *tr = dev->tr;
int ret = 0;
+ mutex_lock(&dev->lock);
dev->open--;
if (dev->open)
return 0;
- if (tr->release)
- ret = tr->release(dev);
-
- if (!ret) {
- dev->mtd->usecount--;
- put_mtd_device(dev->mtd);
- module_put(tr->owner);
- }
/* Free the private data */
if (dev->deleted) {
@@ -181,33 +174,59 @@ static int blktrans_release(struct gendisk *disk, fmode_t mode)
return 0;
}
+ ret = tr->release ? tr->release(dev) : 0;
+ module_put(tr->owner);
+
+ if (dev->mtd)
+ __put_mtd_device(dev->mtd);
+out:
+ mutex_unlock(&dev->lock);
return ret;
}
static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo)
{
struct mtd_blktrans_dev *dev = bdev->bd_disk->private_data;
+ int error;
+
+ mutex_lock(&dev->lock);
+ error = -ENODEV;
+ if (dev->deleted)
+ goto out;
+
+ error = -ENOTTY;
if (dev->tr->getgeo)
- return dev->tr->getgeo(dev, geo);
- return -ENOTTY;
+ error = dev->tr->getgeo(dev, geo);
+out:
+ mutex_unlock(&dev->lock);
+ return error;
}
static int blktrans_ioctl(struct block_device *bdev, fmode_t mode,
unsigned int cmd, unsigned long arg)
{
struct mtd_blktrans_dev *dev = bdev->bd_disk->private_data;
- struct mtd_blktrans_ops *tr = dev->tr;
+ int error = -ENODEV;
+
+ mutex_lock(&dev->lock);
+
+ if (dev->deleted)
+ goto out;
+
+ error = -ENOTTY;
switch (cmd) {
case BLKFLSBUF:
- if (tr->flush)
- return tr->flush(dev);
- /* The core code did the work, we had nothing to do. */
- return 0;
+ if (dev->tr->flush)
+ error = dev->tr->flush(dev);
+ break;
default:
- return -ENOTTY;
+ break;
}
+out:
+ mutex_unlock(&dev->lock);
+ return error;
}
static const struct block_device_operations mtd_blktrans_ops = {
@@ -355,15 +374,19 @@ int del_mtd_blktrans_dev(struct mtd_blktrans_dev *old)
/* Stop the thread */
kthread_stop(old->thread);
+ /* Tell trans driver to release the device */
+ mutex_lock(&old->lock);
+
if (old->open) {
if (old->tr->release)
old->tr->release(old);
- put_mtd_device(old->mtd);
+ __put_mtd_device(old->mtd);
}
/* From now on, no calls into trans can be made */
/* Mtd device will be gone real soon now */
old->mtd = NULL;
+ mutex_unlock(&old->lock);
blk_cleanup_queue(old->rq);
--
1.6.3.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 6/8] blktrans: flush all requests before we remove the device
2010-01-23 0:18 [PATCH 0/8] Cleanups for mtd translation layer (broken up) Maxim Levitsky
` (4 preceding siblings ...)
2010-01-23 0:24 ` [PATCH 5/8] blktrans: add proper locking Maxim Levitsky
@ 2010-01-23 0:24 ` Maxim Levitsky
2010-01-23 0:25 ` [PATCH 7/8] MTD: call remove notifiers before removing " Maxim Levitsky
2010-01-23 0:26 ` [PATCH 8/8] MTD: make mtdtrans thread freezeable Maxim Levitsky
7 siblings, 0 replies; 9+ messages in thread
From: Maxim Levitsky @ 2010-01-23 0:24 UTC (permalink / raw)
To: Artem Bityutskiy
Cc: Alex Dubov, joern, linux-kernel, linux-mtd, Thomas Gleixner,
David Woodhouse
>From 6d5a70a18bea40bfc354e73d7fde5042465fc534 Mon Sep 17 00:00:00 2001
From: Maxim Levitsky <maximlevitsky@gmail.com>
Date: Sat, 23 Jan 2010 02:09:11 +0200
Subject: [PATCH 6/8] blktrans: flush all requests before we remove the device
Flush all requests, so we can be sure we don't deadlock the system later
when we remove the disk queue.
Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
drivers/mtd/mtd_blkdevs.c | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index f165115..33bdef1 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -115,7 +115,13 @@ static int mtd_blktrans_thread(void *arg)
static void mtd_blktrans_request(struct request_queue *rq)
{
struct mtd_blktrans_dev *dev = rq->queuedata;
- wake_up_process(dev->thread);
+ struct request *req = NULL;
+
+ if (dev->deleted)
+ while ((req = blk_fetch_request(rq)) != NULL)
+ __blk_end_request_all(req, -ENODEV);
+ else
+ wake_up_process(dev->thread);
}
@@ -359,6 +365,8 @@ error1:
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();
@@ -369,7 +377,11 @@ int del_mtd_blktrans_dev(struct mtd_blktrans_dev *old)
/* stop new requests to arrive */
del_gendisk(old->disk);
+ /* flush current requests */
+ spin_lock_irqsave(&old->queue_lock, flags);
old->deleted = 1;
+ blk_start_queue(old->rq);
+ spin_unlock_irqrestore(&old->queue_lock, flags);
/* Stop the thread */
kthread_stop(old->thread);
--
1.6.3.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 7/8] MTD: call remove notifiers before removing the device
2010-01-23 0:18 [PATCH 0/8] Cleanups for mtd translation layer (broken up) Maxim Levitsky
` (5 preceding siblings ...)
2010-01-23 0:24 ` [PATCH 6/8] blktrans: flush all requests before we remove the device Maxim Levitsky
@ 2010-01-23 0:25 ` Maxim Levitsky
2010-01-23 0:26 ` [PATCH 8/8] MTD: make mtdtrans thread freezeable Maxim Levitsky
7 siblings, 0 replies; 9+ messages in thread
From: Maxim Levitsky @ 2010-01-23 0:25 UTC (permalink / raw)
To: Artem Bityutskiy
Cc: Alex Dubov, joern, linux-kernel, linux-mtd, Thomas Gleixner,
David Woodhouse
>From 6ab6770f8a575441dfb42095f72f34d77727fa0c Mon Sep 17 00:00:00 2001
From: Maxim Levitsky <maximlevitsky@gmail.com>
Date: Sat, 23 Jan 2010 02:09:31 +0200
Subject: [PATCH 7/8] MTD: call remove notifiers before removing the device
This is first step in implementing proper hotplug support.
Now all users of the mtd device have a chance to put the mtd device
when they are notified to do so, and they have to do so to make hotplug work.
Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
drivers/mtd/mtdcore.c | 21 +++++++++++----------
1 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 3bdb7e8..0e86208 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -341,31 +341,32 @@ int add_mtd_device(struct mtd_info *mtd)
int del_mtd_device (struct mtd_info *mtd)
{
int ret;
+ struct mtd_notifier *not;
mutex_lock(&mtd_table_mutex);
if (mtd_table[mtd->index] != mtd) {
ret = -ENODEV;
- } else if (mtd->usecount) {
+ goto out_error;
+ }
+
+ /* No need to get a refcount on the module containing
+ the notifier, since we hold the mtd_table_mutex */
+ list_for_each_entry(not, &mtd_notifiers, list)
+ not->remove(mtd);
+
+ if (mtd->usecount) {
printk(KERN_NOTICE "Removing MTD device #%d (%s) with use count %d\n",
mtd->index, mtd->name, mtd->usecount);
ret = -EBUSY;
} else {
- struct mtd_notifier *not;
-
device_unregister(&mtd->dev);
-
- /* No need to get a refcount on the module containing
- the notifier, since we hold the mtd_table_mutex */
- list_for_each_entry(not, &mtd_notifiers, list)
- not->remove(mtd);
-
mtd_table[mtd->index] = NULL;
-
module_put(THIS_MODULE);
ret = 0;
}
+out_error:
mutex_unlock(&mtd_table_mutex);
return ret;
}
--
1.6.3.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 8/8] MTD: make mtdtrans thread freezeable.
2010-01-23 0:18 [PATCH 0/8] Cleanups for mtd translation layer (broken up) Maxim Levitsky
` (6 preceding siblings ...)
2010-01-23 0:25 ` [PATCH 7/8] MTD: call remove notifiers before removing " Maxim Levitsky
@ 2010-01-23 0:26 ` Maxim Levitsky
7 siblings, 0 replies; 9+ messages in thread
From: Maxim Levitsky @ 2010-01-23 0:26 UTC (permalink / raw)
To: Artem Bityutskiy
Cc: Alex Dubov, joern, linux-kernel, linux-mtd, Thomas Gleixner,
David Woodhouse
>From ebc69b9819f963a17574aa42e93f409368acdfaa Mon Sep 17 00:00:00 2001
From: Maxim Levitsky <maximlevitsky@gmail.com>
Date: Sat, 23 Jan 2010 02:09:33 +0200
Subject: [PATCH 8/8] MTD: make mtdtrans thread freezeable.
This makes the mtd blktrans thread enter the freezer in between accesses
to nand device.
This will ensure that we aren't suspening the system in the middle
of page read/write and even worse erase, which is a bad idea.
Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
drivers/mtd/mtd_blkdevs.c | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 33bdef1..ed82396 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -20,6 +20,7 @@
#include <linux/init.h>
#include <linux/mutex.h>
#include <linux/kthread.h>
+#include <linux/freezer.h>
#include <asm/uaccess.h>
#include "mtdcore.h"
@@ -79,36 +80,35 @@ static int mtd_blktrans_thread(void *arg)
struct request_queue *rq = dev->rq;
struct request *req = NULL;
- spin_lock_irq(rq->queue_lock);
+ set_freezable();
while (!kthread_should_stop()) {
int res;
+ try_to_freeze();
+
+ spin_lock_irq(rq->queue_lock);
if (!req && !(req = blk_fetch_request(rq))) {
set_current_state(TASK_INTERRUPTIBLE);
spin_unlock_irq(rq->queue_lock);
schedule();
- spin_lock_irq(rq->queue_lock);
continue;
}
-
spin_unlock_irq(rq->queue_lock);
+
mutex_lock(&dev->lock);
res = do_blktrans_request(dev->tr, dev, req);
mutex_unlock(&dev->lock);
spin_lock_irq(rq->queue_lock);
-
if (!__blk_end_request_cur(req, res))
req = NULL;
+ spin_unlock_irq(rq->queue_lock);
}
if (req)
__blk_end_request_all(req, -EIO);
-
- spin_unlock_irq(rq->queue_lock);
-
return 0;
}
--
1.6.3.3
^ permalink raw reply related [flat|nested] 9+ messages in thread