* [PATCH] regulator: Fix recursive mutex lockdep warning
@ 2012-07-03 2:21 Stephen Boyd
2012-07-03 19:26 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Boyd @ 2012-07-03 2:21 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown; +Cc: linux-kernel
A recursive lockdep warning occurs if you call
regulator_set_optimum_mode() on a regulator with a supply because
there is no nesting annotation for the rdev->mutex. To avoid this
warning, get the supply's load before locking the regulator's
mutex to avoid grabbing the same class of lock twice.
=============================================
[ INFO: possible recursive locking detected ]
3.4.0 #3257 Tainted: G W
---------------------------------------------
swapper/0/1 is trying to acquire lock:
(&rdev->mutex){+.+.+.}, at: [<c036e9e0>] regulator_get_voltage+0x18/0x38
but task is already holding lock:
(&rdev->mutex){+.+.+.}, at: [<c036ef38>] regulator_set_optimum_mode+0x24/0x224
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(&rdev->mutex);
lock(&rdev->mutex);
*** DEADLOCK ***
May be due to missing lock nesting notation
3 locks held by swapper/0/1:
#0: (&__lockdep_no_validate__){......}, at: [<c03dbb48>] __driver_attach+0x40/0x8c
#1: (&__lockdep_no_validate__){......}, at: [<c03dbb58>] __driver_attach+0x50/0x8c
#2: (&rdev->mutex){+.+.+.}, at: [<c036ef38>] regulator_set_optimum_mode+0x24/0x224
stack backtrace:
[<c001521c>] (unwind_backtrace+0x0/0x12c) from [<c00cc4d4>] (validate_chain+0x760/0x1080)
[<c00cc4d4>] (validate_chain+0x760/0x1080) from [<c00cd744>] (__lock_acquire+0x950/0xa10)
[<c00cd744>] (__lock_acquire+0x950/0xa10) from [<c00cd990>] (lock_acquire+0x18c/0x1e8)
[<c00cd990>] (lock_acquire+0x18c/0x1e8) from [<c080c248>] (mutex_lock_nested+0x68/0x3c4)
[<c080c248>] (mutex_lock_nested+0x68/0x3c4) from [<c036e9e0>] (regulator_get_voltage+0x18/0x38)
[<c036e9e0>] (regulator_get_voltage+0x18/0x38) from [<c036efb8>] (regulator_set_optimum_mode+0xa4/0x224)
...
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
I'm not sure if this is the preferred solution. I also believe there is a
similar warning in regulator_enable()/disable() when drms_uA_update() is
called.
Is anything wrong with regulator_get_voltage() never taking the mutex?
If we did that then this lockdep warning would go away in addition to the
one in drms_uA_update().
drivers/regulator/core.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index ef07b62..db119f3 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2544,9 +2544,12 @@ int regulator_set_optimum_mode(struct regulator *regulator, int uA_load)
{
struct regulator_dev *rdev = regulator->rdev;
struct regulator *consumer;
- int ret, output_uV, input_uV, total_uA_load = 0;
+ int ret, output_uV, input_uV = 0, total_uA_load = 0;
unsigned int mode;
+ if (rdev->supply)
+ input_uV = regulator_get_voltage(rdev->supply);
+
mutex_lock(&rdev->mutex);
/*
@@ -2579,10 +2582,7 @@ int regulator_set_optimum_mode(struct regulator *regulator, int uA_load)
goto out;
}
- /* get input voltage */
- input_uV = 0;
- if (rdev->supply)
- input_uV = regulator_get_voltage(rdev->supply);
+ /* No supply? Use constraint voltage */
if (input_uV <= 0)
input_uV = rdev->constraints->input_uV;
if (input_uV <= 0) {
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] regulator: Fix recursive mutex lockdep warning
2012-07-03 2:21 [PATCH] regulator: Fix recursive mutex lockdep warning Stephen Boyd
@ 2012-07-03 19:26 ` Mark Brown
2012-07-03 20:08 ` Stephen Boyd
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2012-07-03 19:26 UTC (permalink / raw)
To: Stephen Boyd; +Cc: Liam Girdwood, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 495 bytes --]
On Mon, Jul 02, 2012 at 07:21:06PM -0700, Stephen Boyd wrote:
> A recursive lockdep warning occurs if you call
> regulator_set_optimum_mode() on a regulator with a supply because
> there is no nesting annotation for the rdev->mutex. To avoid this
> warning, get the supply's load before locking the regulator's
> mutex to avoid grabbing the same class of lock twice.
Applied, thanks. Though we should probably just remove the optimal mode
stuff on account of the total absence of any users...
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] regulator: Fix recursive mutex lockdep warning
2012-07-03 19:26 ` Mark Brown
@ 2012-07-03 20:08 ` Stephen Boyd
2012-07-03 22:43 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Boyd @ 2012-07-03 20:08 UTC (permalink / raw)
To: Mark Brown; +Cc: Liam Girdwood, linux-kernel
On 07/03/12 12:26, Mark Brown wrote:
> On Mon, Jul 02, 2012 at 07:21:06PM -0700, Stephen Boyd wrote:
>> A recursive lockdep warning occurs if you call
>> regulator_set_optimum_mode() on a regulator with a supply because
>> there is no nesting annotation for the rdev->mutex. To avoid this
>> warning, get the supply's load before locking the regulator's
>> mutex to avoid grabbing the same class of lock twice.
> Applied, thanks. Though we should probably just remove the optimal mode
> stuff on account of the total absence of any users...
Please don't remove it. We use the optimal stuff fairly often
internally, plus our usb driver is upstream and using the API.
What do you think about removing the locking in get_voltage? Is that better?
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] regulator: Fix recursive mutex lockdep warning
2012-07-03 20:08 ` Stephen Boyd
@ 2012-07-03 22:43 ` Mark Brown
2012-07-03 22:47 ` Stephen Boyd
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2012-07-03 22:43 UTC (permalink / raw)
To: Stephen Boyd; +Cc: Liam Girdwood, linux-kernel
On Tue, Jul 03, 2012 at 01:08:46PM -0700, Stephen Boyd wrote:
> On 07/03/12 12:26, Mark Brown wrote:
> > Applied, thanks. Though we should probably just remove the optimal mode
> > stuff on account of the total absence of any users...
> Please don't remove it. We use the optimal stuff fairly often
> internally, plus our usb driver is upstream and using the API.
>From what I remember there's a fair bit of non-upstream code you guys
have for regulators... your usage is certainly very unusual.
> What do you think about removing the locking in get_voltage? Is that better?
There's already an unlocked version of get_voltage()?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] regulator: Fix recursive mutex lockdep warning
2012-07-03 22:43 ` Mark Brown
@ 2012-07-03 22:47 ` Stephen Boyd
2012-07-03 23:06 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Boyd @ 2012-07-03 22:47 UTC (permalink / raw)
To: Mark Brown; +Cc: Liam Girdwood, linux-kernel
On 07/03/12 15:43, Mark Brown wrote:
>
>> What do you think about removing the locking in get_voltage? Is that better?
> There's already an unlocked version of get_voltage()?
Yes, I'm suggesting we just use that unlocked version for all of the
code that gets the input voltage.
index ef07b62..4c63336 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -628,7 +628,7 @@ static void drms_uA_update(struct regulator_dev *rdev)
/* get input voltage */
input_uV = 0;
if (rdev->supply)
- input_uV = regulator_get_voltage(rdev->supply);
+ input_uV = _regulator_get_voltage(rdev->supply->rdev);
if (input_uV <= 0)
input_uV = rdev->constraints->input_uV;
if (input_uV <= 0)
@@ -2582,7 +2582,7 @@ int regulator_set_optimum_mode(struct regulator *regulator, int uA_load)
/* get input voltage */
input_uV = 0;
if (rdev->supply)
- input_uV = regulator_get_voltage(rdev->supply);
+ input_uV = _regulator_get_voltage(rdev->supply->rdev);
if (input_uV <= 0)
input_uV = rdev->constraints->input_uV;
if (input_uV <= 0) {
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] regulator: Fix recursive mutex lockdep warning
2012-07-03 22:47 ` Stephen Boyd
@ 2012-07-03 23:06 ` Mark Brown
0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2012-07-03 23:06 UTC (permalink / raw)
To: Stephen Boyd; +Cc: Liam Girdwood, linux-kernel
On Tue, Jul 03, 2012 at 03:47:54PM -0700, Stephen Boyd wrote:
> On 07/03/12 15:43, Mark Brown wrote:
> >> What do you think about removing the locking in get_voltage? Is that better?
> > There's already an unlocked version of get_voltage()?
> Yes, I'm suggesting we just use that unlocked version for all of the
> code that gets the input voltage.
Either way works.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-07-03 23:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-03 2:21 [PATCH] regulator: Fix recursive mutex lockdep warning Stephen Boyd
2012-07-03 19:26 ` Mark Brown
2012-07-03 20:08 ` Stephen Boyd
2012-07-03 22:43 ` Mark Brown
2012-07-03 22:47 ` Stephen Boyd
2012-07-03 23:06 ` Mark Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).