Re: [RFC HACK] rtl2832: implement own lock for RegMap

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

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, &regmap_bus, client,
-				  &regmap_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;

--
http://palosaari.fi/
--
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




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux