All of lore.kernel.org
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <k.kozlowski@samsung.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] regulator: Add lockdep asserts to help detecting locking misuse
Date: Thu, 20 Aug 2015 09:07:08 +0900	[thread overview]
Message-ID: <55D51A2C.2060904@samsung.com> (raw)
In-Reply-To: <CAMuHMdVSkPFsuCbVuWgpzh_8XOrYoiHbKQ4TqS_vv7_rCup2qw@mail.gmail.com>

On 19.08.2015 22:08, Geert Uytterhoeven wrote:
> Hi Krzysztof,
> 
> On Wed, Jun 10, 2015 at 8:23 AM, Krzysztof Kozlowski
> <k.kozlowski@samsung.com> wrote:
>> Add lockdep_assert_held_once() to functions explicitly mentioning that
>> rdev or regulator_list mutex must be held. Using WARN_ONCE shouldn't
>> pollute the dmesg to much.
>>
>> The patch (if CONFIG_LOCKDEP enabled) will show warnings in certain
>> regulators calling regulator_notifier_call_chain() without rdev->mutex
>> held.
>>
>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>
>> ---
>>
>> Warnings for missing locks when calling regulator_notifier_call_chain()
>> should appear on many regulators except wm8350-regulator.c, e.g.:
>> da9055-regulator.c, da9062-regulator.c, da9063-regulator.c,
>> da9211-regulator.c, wm831x-dcdc.c and few more.
>>
>> The question is whether the lock during that call should be held?
> 
> That was a (so far, not counting the "Applied, thanks!") unanswered question?
> 
> For the first time ever, I got:
> 
>         drivers/regulator/core.c:3480 regulator_notifier_call_chain+0x54/0x80()
> 
> due to da9210_irq_handler() not taking the mutex.
> 
> Drivers calling regulator_notifier_call_chain() from a threaded interrupt
> handler should be OK calling mutex_lock().
> 
> Does anyone have plans to fix all affected drivers?

Question is still unanswered. I don't have plans to fix the drivers
because I don't have necessary hardware. Blindly fixing such minor issue
could do more harm than good. I just polluted the dmesg with WARN hoping
that this will wake up someone :) .

Best regards,
Krzysztof

      parent reply	other threads:[~2015-08-20  0:07 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-10  6:23 [PATCH] regulator: Add lockdep asserts to help detecting locking misuse Krzysztof Kozlowski
2015-06-10 10:09 ` Mark Brown
2015-07-07  9:09 ` Applied "regulator: Add lockdep asserts to help detecting locking misuse" to the regulator tree Mark Brown
2015-08-19 13:08 ` [PATCH] regulator: Add lockdep asserts to help detecting locking misuse Geert Uytterhoeven
2015-08-19 16:28   ` Mark Brown
2015-08-20  0:07   ` Krzysztof Kozlowski [this message]

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=55D51A2C.2060904@samsung.com \
    --to=k.kozlowski@samsung.com \
    --cc=broonie@kernel.org \
    --cc=geert@linux-m68k.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.