Messages in this thread Patch in this message | | | Date | Sat, 14 Oct 2006 11:23:34 +0200 (CEST) | From | Jiri Kosina <> | Subject | [PATCH] I2C && DVB: fix recursive locking lockped warning in i2c_transfer() |
| |
I am getting the following lockdep warning (with 2.6.19-rc1-mm1, but that probably doesn't matter too much) during initialization of my AVerMedia AverTV DVB-T USB 2.0 (A800) DVB tuner:
============================================= [ INFO: possible recursive locking detected ] 2.6.19-rc1-mm1 #4 --------------------------------------------- khubd/451 is trying to acquire lock: (&adap->bus_lock){--..}, at: [<f8902177>] i2c_transfer+0x23/0x40 [i2c_core]
but task is already holding lock: (&adap->bus_lock){--..}, at: [<f8902177>] i2c_transfer+0x23/0x40 [i2c_core]
other info that might help us debug this: 1 lock held by khubd/451: #0: (&adap->bus_lock){--..}, at: [<f8902177>] i2c_transfer+0x23/0x40 [i2c_core]
stack backtrace: [<c0103b69>] dump_trace+0x65/0x1a2 [<c0103cb6>] show_trace_log_lvl+0x10/0x20 [<c0103f84>] show_trace+0xa/0xc [<c0103f99>] dump_stack+0x13/0x15 [<c0132ea4>] __lock_acquire+0x7bd/0xa05 [<c01333c1>] lock_acquire+0x5c/0x7b [<c034b683>] __mutex_lock_slowpath+0xab/0x1de [<f8902177>] i2c_transfer+0x23/0x40 [i2c_core] [<f88fa1bf>] dibx000_i2c_gated_tuner_xfer+0x166/0x185 [dibx000_common] [<f8902183>] i2c_transfer+0x2f/0x40 [i2c_core] [<f891f04b>] mt2060_readreg+0x4b/0x69 [mt2060] [<f891f45e>] mt2060_attach+0x40/0x1ea [mt2060] [<f895f468>] dibusb_dib3000mc_tuner_attach+0x126/0x16c [ ... ]
etc (the USB subsystem stacktrace stripped).
The obvious reason is that dibusb_dib3000mc is calling i2c_transfer() recursively - first call is issued for the DVB frontend and second call is issued for the actual adapter (or whatever the correct DVB terminology is).
RFC: I have fixed this by adding the 'depth' field to the struct i2c_adapter, and setting this field to represent 'logical' nesting of i2c adapters (i.e. to represent the ordering in which they have to lock when recursing). In this case, depth == 0 for the DVB frontend and depth == 1 for the adapter). This could probably be useful for other devices using I2C, but I have not yet checked. Would this be acceptable by the I2C people?
Note: when the lockdep_set_subclass() patch makes it into mainline (currently only in input tree, but Dmitry told me that it's on its way up), we could get rid of mutex_lock_nested() in the i2c_transfer(), and set the lock class during initialization of the lock in i2c_add_adapter(), if desired.
Signed-off-by: Jiri Kosina <jikos@jikos.cz>
---
[PATCH] I2C && DVB: fix recursive locking lockped warning in i2c_transfer()
The following callchain can happen and is OK: dibusb_dib3000mc_tuner_attach -> mt2060_attach -> mt2060_readreg -> i2c_transfer -> dibx000_i2c_gated_tuner_xfer -> i2c_transfer. Teach lockdep that recursive locking of adap->bus_lock inside i2c_transfer() is OK in such case.
drivers/i2c/i2c-core.c | 2 +- drivers/media/dvb/dvb-usb/dvb-usb-i2c.c | 2 ++ drivers/media/dvb/frontends/dibx000_common.c | 2 ++ include/linux/i2c.h | 1 + 4 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index 7ca81f4..1f9262d 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -603,7 +603,7 @@ #ifdef DEBUG } #endif - mutex_lock(&adap->bus_lock); + mutex_lock_nested(&adap->bus_lock, adap->depth); ret = adap->algo->master_xfer(adap,msgs,num); mutex_unlock(&adap->bus_lock); diff --git a/drivers/media/dvb/dvb-usb/dvb-usb-i2c.c b/drivers/media/dvb/dvb-usb/dvb-usb-i2c.c index 55ba020..0d380f8 100644 --- a/drivers/media/dvb/dvb-usb/dvb-usb-i2c.c +++ b/drivers/media/dvb/dvb-usb/dvb-usb-i2c.c @@ -27,6 +27,8 @@ #else #endif d->i2c_adap.algo = d->props.i2c_algo; d->i2c_adap.algo_data = NULL; + /* DVB adapter locks after DVB frontend */ + d->i2c_adap.depth = 1; i2c_set_adapdata(&d->i2c_adap, d); diff --git a/drivers/media/dvb/frontends/dibx000_common.c b/drivers/media/dvb/frontends/dibx000_common.c index a18c8f4..5397946 100644 --- a/drivers/media/dvb/frontends/dibx000_common.c +++ b/drivers/media/dvb/frontends/dibx000_common.c @@ -111,6 +111,8 @@ static int i2c_adapter_init(struct i2c_a i2c_adap->class = I2C_CLASS_TV_DIGITAL, i2c_adap->algo = algo; i2c_adap->algo_data = NULL; + /* DVB frontend locks before DVB adapter */ + i2c_adap->depth = 0; i2c_set_adapdata(i2c_adap, mst); if (i2c_add_adapter(i2c_adap) < 0) return -ENODEV; diff --git a/include/linux/i2c.h b/include/linux/i2c.h index 9b5d047..0e3a4dc 100644 --- a/include/linux/i2c.h +++ b/include/linux/i2c.h @@ -218,6 +218,7 @@ struct i2c_adapter { /* data fields that are valid for all devices */ struct mutex bus_lock; struct mutex clist_lock; + unsigned short depth; /* "logical" nesting of devices (for lockdep) */ int timeout; int retries;
-- Jiri Kosina - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |