Re: [PATCH v2 3/8] PM / Domains: Add lockdep asserts for domains list mutex

[Ulf Hansson]

On 13 June 2017 at 09:12, Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote:
> On Mon, Jun 12, 2017 at 09:09:59PM +0200, Ulf Hansson wrote:
>> On 12 June 2017 at 17:17, Krzysztof Kozlowski <krzk@xxxxxxxxxx> 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