[PATCH] block2mtd lockdep_init_map warning

Erez Zadok
2008-01-06T07:17:56Z

Hi David,

I've reported before a lockdep warning when block2mtd is modloaded, and a
device is initialized, as in

modprobe block2mtd block2mtd=/dev/loop0

A typical warning looks like this:

BUG: key f88565c0 not in .data!
WARNING: at kernel/lockdep.c:2331 lockdep_init_map()
Pid: 1823, comm: modprobe Not tainted 2.6.24-rc6-unionfs2 #135
[<c02038c0>] show_trace_log_lvl+0x1a/0x2f
[<c02042bb>] show_trace+0x12/0x14
[<c0204a01>] dump_stack+0x6c/0x72
[<c022edf0>] lockdep_init_map+0x94/0x374
[<c022e79d>] debug_mutex_init+0x2c/0x3c
[<c0229bb0>] __mutex_init+0x38/0x40
[<f885520d>] 0xf885520d
[<c0226816>] parse_args+0x121/0x1fb
[<c0237aaf>] sys_init_module+0x10e8/0x1576
[<c0202836>] sysenter_past_esp+0x5f/0xa5
=======================

This is a long-standing problem I've seen in several of the latest stable
kernels. Once lockdep turns itself off, there's no easy way to turn it back
on short of a reboot.

I looked more closely at the mtd code. I believe the problem is that
lockdep doesn't like a mutex_init to be called from a module_init code path,
possibly because the module's symbols aren't all initialized yet. (This
could arguably be considered a limitation of lockdep.)

So I tried to defer the call to module_init until it's absolutely needed. I
couldn't find a clean way to do that via the struct mtd_info ops (there's no
suitable ->init op), so instead I used an int to mark whether the mutex is
initialized or not. Below is a patch. It works, but it's not as clean as
it should be: a better way would be to probably add an mtd_info ->init op or
so.

At least with this patch, lockdep doesn't complain any longer, so I can run
a clean set of regression tests w/ unionfs on top of jffs2 and other file
systems.

In lieu of a better fix, is this patch acceptable?

Thanks,
Erez.

------------------------------------------------------------------------------

block2mtd: defer mutex initialization to avoid a lockdep warning

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>

diff --git a/drivers/mtd/devices/block2mtd.c b/drivers/mtd/devices/block2mtd.c
index be4b994..2c6d3e7 100644
--- a/drivers/mtd/devices/block2mtd.c
+++ b/drivers/mtd/devices/block2mtd.c
@@ -32,6 +32,7 @@ struct block2mtd_dev {
struct list_head list;
struct block_device *blkdev;
struct mtd_info mtd;
+ int mutex_initialized;
struct mutex write_mutex;
};

@@ -85,6 +86,11 @@ static int block2mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
size_t len = instr->len;
int err;

+ if (!dev->mutex_initialized) {
+ mutex_init(&dev->write_mutex);
+ dev->mutex_initialized = 1;
+ }
+
instr->state = MTD_ERASING;
mutex_lock(&dev->write_mutex);
err = _block2mtd_erase(dev, from, len);
@@ -194,6 +200,11 @@ static int block2mtd_write(struct mtd_info *mtd, loff_t to, size_t len,
struct block2mtd_dev *dev = mtd->priv;
int err;

+ if (!dev->mutex_initialized) {
+ mutex_init(&dev->write_mutex);
+ dev->mutex_initialized = 1;
+ }
+
if (!len)
return 0;
if (to >= mtd->size)
@@ -275,8 +286,6 @@ static struct block2mtd_dev *add_device(char *devname, int erase_size)
goto devinit_err;
}

- mutex_init(&dev->write_mutex);
-
/* Setup the MTD structure */
/* make the name contain the block device in */
dev->mtd.name = kmalloc(sizeof("block2mtd: ") + strlen(devname),