Em Thu, 18 Dec 2014 16:41:28 +0200
Antti Palosaari <crope@xxxxxx> escreveu:
> On 12/18/2014 01:21 PM, Mauro Carvalho Chehab wrote:
> > Em Thu, 18 Dec 2014 12:29:46 +0200
> > Antti Palosaari <crope@xxxxxx> escreveu:
> >
> >> Introduce own lock to silence locdep warning. I suspect lockdep checks
> >> make wrong decision when two similar name (&map->mutex) locks were
> >> taken recursively, even those are different mutexes in a two different
> >> driver. After that patch, functionality remains same, but mutex names
> >> are different.
> >
> > Please do not add a hack just to silence a lockdep warning.
> >
> > Please take a look at: Documentation/locking/lockdep-design.txt
> >
> > There are some documentation there about the usage of nested locks and
> > how to avoid lockdep to complain.
>
> I cannot see any way how those lockdep things could be used on my driver
> as problematic lock itself is located inside RegMap. I think it is
> RegMap which needs some special lockdep things in order to allow nested
> locking of different instances.
Ok. So, do a patch for RegMap and send for its maintainers.
Regards,
Mauro
>
> So I think demod I2C adapter is good place for that kind of hack until
> better solution is find. Defining own RegMap lock here in I2C repeater
> allows all the clients be hack free using default RegMap lock.
>
> regards
> Antti
>
>
> >
> > Regards,
> > Mauro
> >
> >>
> >> =============================================
> >> [ INFO: possible recursive locking detected ]
> >> 3.18.0-rc4+ #4 Tainted: G O
> >> ---------------------------------------------
> >> kdvb-ad-0-fe-0/2814 is trying to acquire lock:
> >> (&map->mutex){+.+.+.}, at: [<ffffffff814ec90f>] regmap_lock_mutex+0x2f/0x40
> >>
> >> but task is already holding lock:
> >> (&map->mutex){+.+.+.}, at: [<ffffffff814ec90f>] regmap_lock_mutex+0x2f/0x40
> >>
> >> other info that might help us debug this:
> >> Possible unsafe locking scenario:
> >> CPU0
> >> ----
> >> lock(&map->mutex);
> >> lock(&map->mutex);
> >>
> >> *** DEADLOCK ***
> >> May be due to missing lock nesting notation
> >> 1 lock held by kdvb-ad-0-fe-0/2814:
> >> #0: (&map->mutex){+.+.+.}, at: [<ffffffff814ec90f>] regmap_lock_mutex+0x2f/0x40
> >>
> >> stack backtrace:
> >> CPU: 3 PID: 2814 Comm: kdvb-ad-0-fe-0 Tainted: G O 3.18.0-rc4+ #4
> >> Hardware name: System manufacturer System Product Name/M5A78L-M/USB3, BIOS 2001 09/11/2014
> >> 0000000000000000 00000000410c8772 ffff880293af3868 ffffffff817a6f82
> >> 0000000000000000 ffff8800b3462be0 ffff880293af3968 ffffffff810e7f94
> >> ffff880293af3888 00000000410c8772 ffffffff82dfee60 ffffffff81ab8f89
> >> Call Trace:
> >> [<ffffffff817a6f82>] dump_stack+0x4e/0x68
> >> [<ffffffff810e7f94>] __lock_acquire+0x1ea4/0x1f50
> >> [<ffffffff810e2a7d>] ? trace_hardirqs_off+0xd/0x10
> >> [<ffffffff817b01f3>] ? _raw_spin_lock_irqsave+0x83/0xa0
> >> [<ffffffff810e13e6>] ? up+0x16/0x50
> >> [<ffffffff810e2a7d>] ? trace_hardirqs_off+0xd/0x10
> >> [<ffffffff817af8bf>] ? _raw_spin_unlock_irqrestore+0x5f/0x70
> >> [<ffffffff810e9069>] lock_acquire+0xc9/0x170
> >> [<ffffffff814ec90f>] ? regmap_lock_mutex+0x2f/0x40
> >> [<ffffffff817ab50e>] mutex_lock_nested+0x7e/0x430
> >> [<ffffffff814ec90f>] ? regmap_lock_mutex+0x2f/0x40
> >> [<ffffffff814ec90f>] ? regmap_lock_mutex+0x2f/0x40
> >> [<ffffffff817a530b>] ? printk+0x70/0x86
> >> [<ffffffff8110d9e8>] ? mod_timer+0x168/0x240
> >> [<ffffffff814ec90f>] regmap_lock_mutex+0x2f/0x40
> >> [<ffffffff814f08d9>] regmap_update_bits+0x29/0x60
> >> [<ffffffffa03e9778>] rtl2832_select+0x38/0x70 [rtl2832]
> >> [<ffffffffa039b03d>] i2c_mux_master_xfer+0x3d/0x90 [i2c_mux]
> >> [<ffffffff815da493>] __i2c_transfer+0x73/0x2e0
> >> [<ffffffff815dbaba>] i2c_transfer+0x5a/0xc0
> >> [<ffffffff815dbb6e>] i2c_master_send+0x4e/0x70
> >> [<ffffffffa03ff25a>] regmap_i2c_write+0x1a/0x50 [regmap_i2c]
> >> [<ffffffff817ab713>] ? mutex_lock_nested+0x283/0x430
> >> [<ffffffff814f06b2>] _regmap_raw_write+0x862/0x880
> >> [<ffffffff814ec90f>] ? regmap_lock_mutex+0x2f/0x40
> >> [<ffffffff814f0744>] _regmap_bus_raw_write+0x74/0xa0
> >> [<ffffffff814ef3d2>] _regmap_write+0x92/0x140
> >> [<ffffffff814f0b7b>] regmap_write+0x4b/0x70
> >> [<ffffffffa032b090>] ? dvb_frontend_release+0x110/0x110 [dvb_core]
> >> [<ffffffffa05141d4>] e4000_init+0x34/0x210 [e4000]
> >> [<ffffffffa032a029>] dvb_frontend_init+0x59/0xc0 [dvb_core]
> >> [<ffffffff810bde30>] ? finish_task_switch+0x80/0x180
> >> [<ffffffff810bddf2>] ? finish_task_switch+0x42/0x180
> >> [<ffffffffa032b116>] dvb_frontend_thread+0x86/0x7b0 [dvb_core]
> >> [<ffffffff817a9203>] ? __schedule+0x343/0x930
> >> [<ffffffffa032b090>] ? dvb_frontend_release+0x110/0x110 [dvb_core]
> >> [<ffffffff810b826b>] kthread+0x10b/0x130
> >> [<ffffffff81020099>] ? sched_clock+0x9/0x10
> >> [<ffffffff810b8160>] ? kthread_create_on_node+0x250/0x250
> >> [<ffffffff817b063c>] ret_from_fork+0x7c/0xb0
> >> [<ffffffff810b8160>] ? kthread_create_on_node+0x250/0x250
> >>
> >> Signed-off-by: Antti Palosaari <crope@xxxxxx>
> >> ---
> >> drivers/media/dvb-frontends/rtl2832.c | 49 +++++++++++++++++++++++-------
> >> drivers/media/dvb-frontends/rtl2832_priv.h | 2 ++
> >> 2 files changed, 40 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/media/dvb-frontends/rtl2832.c b/drivers/media/dvb-frontends/rtl2832.c
> >> index f44dc50..2ee5bcf 100644
> >> --- a/drivers/media/dvb-frontends/rtl2832.c
> >> +++ b/drivers/media/dvb-frontends/rtl2832.c
> >> @@ -1028,6 +1028,31 @@ static int rtl2832_regmap_gather_write(void *context, const void *reg,
> >> return 0;
> >> }
> >>
> >> +/*
> >> + * FIXME: Implement own RegMap locking in order to silence lockdep recursive
> >> + * lock warning. That happens when RegMap I2C client calls I2C mux adapter,
> >> + * which leads demod I2C repeater enable via demod RegMap. Operation takes two
> >> + * RegMap locks recursively - but those are different RegMap instances in a two
> >> + * different I2C drivers, so it should be deadlock.
> >> + */
> >> +static void rtl2832_regmap_lock(void *__dev)
> >> +{
> >> + struct rtl2832_dev *dev = __dev;
> >> + struct i2c_client *client = dev->client;
> >> +
> >> + dev_dbg(&client->dev, "\n");
> >> + mutex_lock(&dev->regmap_mutex);
> >> +}
> >> +
> >> +static void rtl2832_regmap_unlock(void *__dev)
> >> +{
> >> + struct rtl2832_dev *dev = __dev;
> >> + struct i2c_client *client = dev->client;
> >> +
> >> + dev_dbg(&client->dev, "\n");
> >> + mutex_unlock(&dev->regmap_mutex);
> >> +}
> >> +
> >> static struct dvb_frontend *rtl2832_get_dvb_frontend(struct i2c_client *client)
> >> {
> >> struct rtl2832_dev *dev = i2c_get_clientdata(client);
> >> @@ -1186,15 +1211,6 @@ static int rtl2832_probe(struct i2c_client *client,
> >> .range_max = 5 * 0x100,
> >> },
> >> };
> >> - static const struct regmap_config regmap_config = {
> >> - .reg_bits = 8,
> >> - .val_bits = 8,
> >> - .volatile_reg = rtl2832_volatile_reg,
> >> - .max_register = 5 * 0x100,
> >> - .ranges = regmap_range_cfg,
> >> - .num_ranges = ARRAY_SIZE(regmap_range_cfg),
> >> - .cache_type = REGCACHE_RBTREE,
> >> - };
> >>
> >> dev_dbg(&client->dev, "\n");
> >>
> >> @@ -1218,8 +1234,19 @@ static int rtl2832_probe(struct i2c_client *client,
> >> INIT_DELAYED_WORK(&dev->i2c_gate_work, rtl2832_i2c_gate_work);
> >> INIT_DELAYED_WORK(&dev->stat_work, rtl2832_stat_work);
> >> /* create RegMap */
> >> + mutex_init(&dev->regmap_mutex);
> >> + dev->regmap_config.reg_bits = 8,
> >> + dev->regmap_config.val_bits = 8,
> >> + dev->regmap_config.lock = rtl2832_regmap_lock,
> >> + dev->regmap_config.unlock = rtl2832_regmap_unlock,
> >> + dev->regmap_config.lock_arg = dev,
> >> + dev->regmap_config.volatile_reg = rtl2832_volatile_reg,
> >> + dev->regmap_config.max_register = 5 * 0x100,
> >> + dev->regmap_config.ranges = regmap_range_cfg,
> >> + dev->regmap_config.num_ranges = ARRAY_SIZE(regmap_range_cfg),
> >> + dev->regmap_config.cache_type = REGCACHE_RBTREE,
> >> dev->regmap = regmap_init(&client->dev, ®map_bus, client,
> >> - ®map_config);
> >> + &dev->regmap_config);
> >> if (IS_ERR(dev->regmap)) {
> >> ret = PTR_ERR(dev->regmap);
> >> goto err_kfree;
> >> @@ -1232,7 +1259,7 @@ static int rtl2832_probe(struct i2c_client *client,
> >>
> >> /* create muxed i2c adapter for demod tuner bus */
> >> dev->i2c_adapter_tuner = i2c_add_mux_adapter(i2c, &i2c->dev, dev,
> >> - 0, 0, 0, rtl2832_select, rtl2832_deselect);
> >> + 0, 1, 0, rtl2832_select, rtl2832_deselect);
> >> if (dev->i2c_adapter_tuner == NULL) {
> >> ret = -ENODEV;
> >> goto err_regmap_exit;
> >> diff --git a/drivers/media/dvb-frontends/rtl2832_priv.h b/drivers/media/dvb-frontends/rtl2832_priv.h
> >> index 9ff4f65..c3a922c 100644
> >> --- a/drivers/media/dvb-frontends/rtl2832_priv.h
> >> +++ b/drivers/media/dvb-frontends/rtl2832_priv.h
> >> @@ -33,6 +33,8 @@
> >> struct rtl2832_dev {
> >> struct rtl2832_platform_data *pdata;
> >> struct i2c_client *client;
> >> + struct mutex regmap_mutex;
> >> + struct regmap_config regmap_config;
> >> struct regmap *regmap;
> >> struct i2c_adapter *i2c_adapter_tuner;
> >> struct dvb_frontend fe;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
