Alsa-Devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Nicolas Boichat <drinkcat@chromium.org>
To: Mark Brown <broonie@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>,
	Antti Palosaari <crope@iki.fi>, Ingo Molnar <mingo@redhat.com>,
	Arjan van de Ven <arjan@linux.intel.com>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org, Bard Liao <bardliao@realtek.com>,
	Oder Chiou <oder_chiou@realtek.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.de>,
	alsa-devel@alsa-project.org,
	Anatol Pomozov <anatol.pomozov@gmail.com>
Subject: [RFC PATCH 1/2] regmap: add configurable lock class key for lockdep
Date: Thu, 25 Jun 2015 17:35:03 +0800	[thread overview]
Message-ID: <1435224904-35517-1-git-send-email-drinkcat@chromium.org> (raw)

From: Antti Palosaari <crope@iki.fi>

Lockdep validator complains about recursive locking and deadlock
when two different regmap instances are called in a nested order.
That happens anytime a regmap read/write call needs to access
another regmap.

This is because, for performance reason, lockdep groups all locks
initialized by the same mutex_init() in the same lock class.
Therefore all regmap mutexes are in the same lock class, leading
to lockdep "nested locking" warnings if a regmap accesses another
regmap. However, depending on the specifics of the driver, this
can be perfectly safe (e.g. if there is a clear hierarchy between
a "master" regmap that uses another "slave" regmap). In these
cases, the warning is false and should be silenced.

As a solution, add configuration option to pass custom lock class
key for lockdep validator, to be used in the regmap that needs to
access another regmap. This removes the need for uglier workarounds
in drivers, just to silence this warning (e.g. add custom mutex
lock/unlock functions).

Cc: Lars-Peter Clausen <lars@metafoo.de>
Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Antti Palosaari <crope@iki.fi>
[drinkcat@chromium.org: Reworded the commit message and regmap.h
documentation]
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
---

I'm trying to revive Antti's patch [1], as we are hitting similar issue
with rt5677 driver. I only updated the commit message and documentation,
I kept both Signed-off-by and From lines, with a small note highlighting
my changes, let me know if that's not appropriate.

One issue that was raised by Mark at the time is that a per-regmap_config
lock class might not be enough (Mark mentioned clocks as an example). The
current implementation should be good enough as long as the clock regmaps
do not access each other. If this is a problem, maybe we should consider
replacing lockdep_lock_class_key by a boolean use_own_lock_class that would
allocate/use a per regmap instance lock class, or have devm_regmap_init
take an extra parameter specifying the lock class.

[1] https://www.mail-archive.com/linux-media%40vger.kernel.org/msg83490.html

 drivers/base/regmap/regmap.c | 3 +++
 include/linux/regmap.h       | 7 +++++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 6273ff0..f5d1131 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -560,6 +560,9 @@ struct regmap *regmap_init(struct device *dev,
 			mutex_init(&map->mutex);
 			map->lock = regmap_lock_mutex;
 			map->unlock = regmap_unlock_mutex;
+			if (config->lockdep_lock_class_key)
+				lockdep_set_class(&map->mutex,
+						config->lockdep_lock_class_key);
 		}
 		map->lock_arg = map;
 	}
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 116655d..09aaaf5 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -135,6 +135,12 @@ typedef void (*regmap_unlock)(void *);
  * @lock_arg:	  this field is passed as the only argument of lock/unlock
  *		  functions (ignored in case regular lock/unlock functions
  *		  are not overridden).
+ * @lock_class_key: Custom lock class key for lockdep validator. Use that to
+ *                  silence false lockdep nested locking warning, when this
+ *                  regmap needs to access another regmap during read/write
+ *                  operations (directly in read/write functions, or
+ *                  indirectly, e.g. through bus accesses).
+ *                  Valid only when regmap default mutex locking is used.
  * @reg_read:	  Optional callback that if filled will be used to perform
  *           	  all the reads from the registers. Should only be provided for
  *		  devices whose read operation cannot be represented as a simple
@@ -198,6 +204,7 @@ struct regmap_config {
 	regmap_lock lock;
 	regmap_unlock unlock;
 	void *lock_arg;
+	struct lock_class_key *lockdep_lock_class_key;
 
 	int (*reg_read)(void *context, unsigned int reg, unsigned int *val);
 	int (*reg_write)(void *context, unsigned int reg, unsigned int val);
-- 
2.4.3.573.g4eafbef

             reply	other threads:[~2015-06-25  9:35 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-25  9:35 Nicolas Boichat [this message]
2015-06-25  9:35 ` [RFC PATCH 2/2] ASoC: rt5677: Add lockdep class to silence lockdep warnings Nicolas Boichat
2015-06-25 13:21 ` [RFC PATCH 1/2] regmap: add configurable lock class key for lockdep Arjan van de Ven
2015-06-25 14:29   ` Mark Brown
2015-06-25 15:03   ` Lars-Peter Clausen
2015-06-25 15:33     ` Mark Brown
2015-06-25 15:47       ` Lars-Peter Clausen
2015-06-25 16:08         ` Mark Brown
2015-06-26  3:16           ` Nicolas Boichat
2015-06-29 12:51             ` Nicolas Boichat
2015-06-29 12:59               ` Lars-Peter Clausen
2015-06-29 14:03                 ` Nicolas Boichat
2015-06-29 14:18                   ` Lars-Peter Clausen
2015-06-29 15:34                     ` Mark Brown
2015-06-30  4:56                       ` Nicolas Boichat
2015-06-30 11:02                         ` [alsa-devel] " Lars-Peter Clausen
2015-06-29 14:22                   ` Mark Brown
2015-06-29 14:35                     ` Arjan van de Ven
2015-06-29 15:32                       ` Mark Brown
2015-06-29 15:36                         ` Arjan van de Ven
2015-06-29 16:02                           ` Mark Brown
2015-06-29 14:19                 ` Mark Brown
2015-06-25 14:49 ` Mark Brown
2015-06-25 15:59 ` Lars-Peter Clausen
2015-06-26  2:34   ` Nicolas Boichat
2015-06-26  8:31     ` Lars-Peter Clausen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1435224904-35517-1-git-send-email-drinkcat@chromium.org \
    --to=drinkcat@chromium.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=anatol.pomozov@gmail.com \
    --cc=arjan@linux.intel.com \
    --cc=bardliao@realtek.com \
    --cc=broonie@kernel.org \
    --cc=crope@iki.fi \
    --cc=gregkh@linuxfoundation.org \
    --cc=lars@metafoo.de \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mchehab@osg.samsung.com \
    --cc=mingo@redhat.com \
    --cc=oder_chiou@realtek.com \
    --cc=perex@perex.cz \
    --cc=tiwai@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox