| Message ID | 20090714200429.GD18774@darwin |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Tue, 2009-07-14 at 22:04 +0200, Matthias Kaehlcke wrote: > +static struct mutex mtdblks_lock; > + > /* > * Cache stuff... > * > @@ -270,6 +272,8 @@ static int mtdblock_open(struct mtd_blktrans_dev *mbd) > > DEBUG(MTD_DEBUG_LEVEL1,"mtdblock_open\n"); > > + mutex_lock(&mtdblks_lock); > + > if (mtdblks[dev]) { > mtdblks[dev]->count++; > return 0; Now about unlocking the mutex here?
El Wed, Jul 15, 2009 at 10:41:54AM +0300 Artem Bityutskiy ha dit: > On Tue, 2009-07-14 at 22:04 +0200, Matthias Kaehlcke wrote: > > +static struct mutex mtdblks_lock; > > + > > /* > > * Cache stuff... > > * > > @@ -270,6 +272,8 @@ static int mtdblock_open(struct mtd_blktrans_dev *mbd) > > > > DEBUG(MTD_DEBUG_LEVEL1,"mtdblock_open\n"); > > > > + mutex_lock(&mtdblks_lock); > > + > > if (mtdblks[dev]) { > > mtdblks[dev]->count++; > > return 0; > > Now about unlocking the mutex here? i missed this one ... thanks for your review. i'm going fix that and send out an updated version of the patch
On Wed, 2009-07-15 at 10:36 +0200, Matthias Kaehlcke wrote: > El Wed, Jul 15, 2009 at 10:41:54AM +0300 Artem Bityutskiy ha dit: > > > On Tue, 2009-07-14 at 22:04 +0200, Matthias Kaehlcke wrote: > > > +static struct mutex mtdblks_lock; > > > + > > > /* > > > * Cache stuff... > > > * > > > @@ -270,6 +272,8 @@ static int mtdblock_open(struct mtd_blktrans_dev *mbd) > > > > > > DEBUG(MTD_DEBUG_LEVEL1,"mtdblock_open\n"); > > > > > > + mutex_lock(&mtdblks_lock); > > > + > > > if (mtdblks[dev]) { > > > mtdblks[dev]->count++; > > > return 0; > > > > Now about unlocking the mutex here? > > i missed this one ... > > thanks for your review. i'm going fix that and send out an updated > version of the patch Err, I've already amended your patch and put it to my l2-mtd-2.6.git: http://git.infradead.org/users/dedekind/l2-mtd-2.6.git?a=commit;h=2a4d4fdb6393fdcbfea546b2474baf41d434302a
El Wed, Jul 15, 2009 at 11:39:22AM +0300 Artem Bityutskiy ha dit: > On Wed, 2009-07-15 at 10:36 +0200, Matthias Kaehlcke wrote: > > El Wed, Jul 15, 2009 at 10:41:54AM +0300 Artem Bityutskiy ha dit: > > > > > On Tue, 2009-07-14 at 22:04 +0200, Matthias Kaehlcke wrote: > > > > +static struct mutex mtdblks_lock; > > > > + > > > > /* > > > > * Cache stuff... > > > > * > > > > @@ -270,6 +272,8 @@ static int mtdblock_open(struct mtd_blktrans_dev *mbd) > > > > > > > > DEBUG(MTD_DEBUG_LEVEL1,"mtdblock_open\n"); > > > > > > > > + mutex_lock(&mtdblks_lock); > > > > + > > > > if (mtdblks[dev]) { > > > > mtdblks[dev]->count++; > > > > return 0; > > > > > > Now about unlocking the mutex here? > > > > i missed this one ... > > > > thanks for your review. i'm going fix that and send out an updated > > version of the patch > > Err, I've already amended your patch and put it to my l2-mtd-2.6.git: > http://git.infradead.org/users/dedekind/l2-mtd-2.6.git?a=commit;h=2a4d4fdb6393fdcbfea546b2474baf41d434302a ok, thx!
diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c index 208c6fa..ca0c6b8 100644 --- a/drivers/mtd/mtdblock.c +++ b/drivers/mtd/mtdblock.c @@ -29,6 +29,8 @@ static struct mtdblk_dev { enum { STATE_EMPTY, STATE_CLEAN, STATE_DIRTY } cache_state; } *mtdblks[MAX_MTD_DEVICES]; +static struct mutex mtdblks_lock; + /* * Cache stuff... * @@ -270,6 +272,8 @@ static int mtdblock_open(struct mtd_blktrans_dev *mbd) DEBUG(MTD_DEBUG_LEVEL1,"mtdblock_open\n"); + mutex_lock(&mtdblks_lock); + if (mtdblks[dev]) { mtdblks[dev]->count++; return 0; @@ -277,8 +281,10 @@ static int mtdblock_open(struct mtd_blktrans_dev *mbd) /* OK, it's not open. Create cache info for it */ mtdblk = kzalloc(sizeof(struct mtdblk_dev), GFP_KERNEL); - if (!mtdblk) + if (!mtdblk) { + mutex_unlock(&mtdblks_lock); return -ENOMEM; + } mtdblk->count = 1; mtdblk->mtd = mtd; @@ -292,6 +298,8 @@ static int mtdblock_open(struct mtd_blktrans_dev *mbd) mtdblks[dev] = mtdblk; + mutex_unlock(&mtdblks_lock); + DEBUG(MTD_DEBUG_LEVEL1, "ok\n"); return 0; @@ -304,6 +312,8 @@ static int mtdblock_release(struct mtd_blktrans_dev *mbd) DEBUG(MTD_DEBUG_LEVEL1, "mtdblock_release\n"); + mutex_lock(&mtdblks_lock); + mutex_lock(&mtdblk->cache_mutex); write_cached_data(mtdblk); mutex_unlock(&mtdblk->cache_mutex); @@ -316,6 +326,9 @@ static int mtdblock_release(struct mtd_blktrans_dev *mbd) vfree(mtdblk->cache_data); kfree(mtdblk); } + + mutex_unlock(&mtdblks_lock); + DEBUG(MTD_DEBUG_LEVEL1, "ok\n"); return 0; @@ -376,6 +389,8 @@ static struct mtd_blktrans_ops mtdblock_tr = { static int __init init_mtdblock(void) { + mutex_init(&mtdblks_lock); + return register_mtd_blktrans(&mtdblock_tr); }
The mtdblks array and its content are prone to race conditions. Introduce the mutex mtdblks_lock in order to solve this. Signed-off-by: Matthias Kaehlcke <matthias@kaehlcke.net> --- drivers/mtd/mtdblock.c | 17 ++++++++++++++++- 1 files changed, 16 insertions(+), 1 deletions(-)