From: Ulf Hansson <ulf.hansson@linaro.org>
To: Krzysztof Kozlowski <krzk@kernel.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
Kevin Hilman <khilman@kernel.org>,
Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 3/8] PM / Domains: Add lockdep asserts for domains list mutex
Date: Tue, 13 Jun 2017 10:34:49 +0200 [thread overview]
Message-ID: <CAPDyKFpG_o_PEoGUvEgV_s2PovW3VsR9NjzUo9SEKv1GGfO=Ew@mail.gmail.com> (raw)
In-Reply-To: <20170613071218.bbml7vsfcdgpirlp@kozik-lap>
On 13 June 2017 at 09:12, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Mon, Jun 12, 2017 at 09:09:59PM +0200, Ulf Hansson wrote:
>> On 12 June 2017 at 17:17, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>> > Add lockdep checks for holding mutex protecting the list of domains.
>> > This might expose misuse even though only file-scope functions use it
>> > for now.
>>
>> I think it seems a bit silly to use these lockdep checks as these
>> functions are as you state above, static functions. Moreover there are
>> called from a quite limited amount of places.
>>
>> Do you really think this add some value?
>
> In ideal world, these would not need lockdeps because we do not make
> mistakes. I agree, that mostly the exposed functions to other kernel
> modules should be protected, as lockdep annotation is a more advanced
> way of documenting the need of locking.
>
> Does it mean that file-scope functions should not be annotated? Even in
> such case function can be misused as the code is getting more and more
> complicated.
>
> What is best example, one of these calls (pm_genpd_present()) was
> already used in wrong way, without locking when iterating over the list.
> Having lockdep would point this early.
Well, I think we found a potential bug because of reviewing the code.
Not because of adding lockdep_assert_held().
Perhaps adding protection with lockdep_assert_held() could be
meaningful also for static functions, but only in cases of complicated
code. I don't think that is the case here.
Anyway, if other people thinks $subject patch is good idea, then I
drop my case. :-)
[...]
Kind regards
Uffe
next prev parent reply other threads:[~2017-06-13 8:34 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-12 15:17 [PATCH v2 0/8] PM / Domains: Bunch of small improvements and fixes Krzysztof Kozlowski
2017-06-12 15:17 ` [PATCH v2 1/8] PM / Domains: Constify genpd pointer Krzysztof Kozlowski
2017-06-13 10:36 ` Ulf Hansson
2017-06-12 15:17 ` [RFT v2 2/8] PM / Domains: Handle safely genpd_syscore_switch() call on non-genpd device Krzysztof Kozlowski
2017-06-13 10:38 ` Ulf Hansson
2017-06-12 15:17 ` [PATCH v2 3/8] PM / Domains: Add lockdep asserts for domains list mutex Krzysztof Kozlowski
2017-06-12 19:09 ` Ulf Hansson
2017-06-13 7:12 ` Krzysztof Kozlowski
2017-06-13 8:34 ` Ulf Hansson [this message]
2017-06-12 15:17 ` [PATCH v2 4/8] PM / Domains: Fix unsafe iteration over modified list of device links Krzysztof Kozlowski
2017-06-13 10:38 ` Ulf Hansson
2017-06-12 15:17 ` [PATCH v2 5/8] PM / Domains: Fix unsafe iteration over modified list of domain providers Krzysztof Kozlowski
2017-06-13 10:38 ` Ulf Hansson
2017-06-12 15:17 ` [PATCH v2 6/8] PM / Domains: Fix unsafe iteration over modified list of domains Krzysztof Kozlowski
2017-06-13 10:39 ` Ulf Hansson
2017-06-12 15:17 ` [PATCH v2 7/8] PM / Domains: Fix missing default_power_down_ok comment Krzysztof Kozlowski
2017-06-13 10:39 ` Ulf Hansson
2017-06-12 15:17 ` [RFC v2 8/8] PM / Domains: Add asserts for PM domain locks Krzysztof Kozlowski
2017-06-27 22:51 ` [PATCH v2 0/8] PM / Domains: Bunch of small improvements and fixes Rafael J. Wysocki
2017-06-28 14:48 ` Krzysztof Kozlowski
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='CAPDyKFpG_o_PEoGUvEgV_s2PovW3VsR9NjzUo9SEKv1GGfO=Ew@mail.gmail.com' \
--to=ulf.hansson@linaro.org \
--cc=gregkh@linuxfoundation.org \
--cc=khilman@kernel.org \
--cc=krzk@kernel.org \
--cc=len.brown@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=pavel@ucw.cz \
--cc=rjw@rjwysocki.net \
/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;
as well as URLs for NNTP newsgroup(s).